Re: svn commit: r1499351 - in /httpd/httpd/trunk: include/ap_mmn.h include/mod_auth.h modules/aaa/mod_auth_basic.c modules/aaa/mod_auth_digest.c
Hi,
while looking at backport candidate to synch 2.4 and trunk, I found this
commit.
It seems harmless to me, so could be a good candidate.
Actually, it should be no use in module provided with apache, because
none seems to use AUTH_HANDLED.
So I assume that restoring this behavior was dedicated to 3rd party module.
However, I've looked at 2.2 (i.e. 2.2.x dev branch) but did not find any
AUTH_HANDLED in the code.
Even "->ststus" is not part of mod_auth_basic.c and mod_auth_digest.c.
So I don't see how it "Restore support for the AUTH_HANDLED return code
in AUTHN providers, like in 2.2"
Could you please elaborate?
Was it done elsewhere or another way?
CJ
Le 03/07/2013 14:13, cove...@apache.org a écrit :
Author: covener
Date: Wed Jul 3 12:13:50 2013
New Revision: 1499351
URL: http://svn.apache.org/r1499351
Log:
Restore support for the AUTH_HANDLED return code in AUTHN providers,
like in 2.2, which allows authn provider to return their own status
in r->status (custom error code, or return a redirect)
Modified:
httpd/httpd/trunk/include/ap_mmn.h
httpd/httpd/trunk/include/mod_auth.h
httpd/httpd/trunk/modules/aaa/mod_auth_basic.c
httpd/httpd/trunk/modules/aaa/mod_auth_digest.c
Modified: httpd/httpd/trunk/include/ap_mmn.h
URL:
http://svn.apache.org/viewvc/httpd/httpd/trunk/include/ap_mmn.h?rev=1499351&r1=1499350&r2=1499351&view=diff
==
--- httpd/httpd/trunk/include/ap_mmn.h (original)
+++ httpd/httpd/trunk/include/ap_mmn.h Wed Jul 3 12:13:50 2013
@@ -435,6 +435,7 @@
* 20121222.15 (2.5.0-dev) Add allow/decode_encoded_slashes_set to
core_dir_config
* 20121222.16 (2.5.0-dev) AP_DEFAULT_HANDLER_NAME/AP_IS_DEAULT_HANDLER_NAME
* 20130702.0 (2.5.0-dev) Remove pre_htaccess hook, add open_htaccess hook.
+ * 20130702.1 (2.5.0-dev) Restore AUTH_HANDLED to mod_auth.h
*/
#define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */
@@ -442,7 +443,7 @@
#ifndef MODULE_MAGIC_NUMBER_MAJOR
#define MODULE_MAGIC_NUMBER_MAJOR 20130702
#endif
-#define MODULE_MAGIC_NUMBER_MINOR 0 /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 1 /* 0...n */
/**
* Determine if the server's current MODULE_MAGIC_NUMBER is at least a
Modified: httpd/httpd/trunk/include/mod_auth.h
URL:
http://svn.apache.org/viewvc/httpd/httpd/trunk/include/mod_auth.h?rev=1499351&r1=1499350&r2=1499351&view=diff
==
--- httpd/httpd/trunk/include/mod_auth.h (original)
+++ httpd/httpd/trunk/include/mod_auth.h Wed Jul 3 12:13:50 2013
@@ -66,7 +66,8 @@ typedef enum {
AUTH_GRANTED,
AUTH_USER_FOUND,
AUTH_USER_NOT_FOUND,
-AUTH_GENERAL_ERROR
+AUTH_GENERAL_ERROR,
+AUTH_HANDLED
} authn_status;
typedef enum {
Modified: httpd/httpd/trunk/modules/aaa/mod_auth_basic.c
URL:
http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/aaa/mod_auth_basic.c?rev=1499351&r1=1499350&r2=1499351&view=diff
==
--- httpd/httpd/trunk/modules/aaa/mod_auth_basic.c (original)
+++ httpd/httpd/trunk/modules/aaa/mod_auth_basic.c Wed Jul 3 12:13:50 2013
@@ -359,6 +359,9 @@ static int authenticate_basic_user(reque
"user %s not found: %s", sent_user, r->uri);
return_code = HTTP_UNAUTHORIZED;
break;
+case AUTH_HANDLED:
+return_code = r->status;
+break;
case AUTH_GENERAL_ERROR:
default:
/* We'll assume that the module has already said what its error
Modified: httpd/httpd/trunk/modules/aaa/mod_auth_digest.c
URL:
http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/aaa/mod_auth_digest.c?rev=1499351&r1=1499350&r2=1499351&view=diff
==
--- httpd/httpd/trunk/modules/aaa/mod_auth_digest.c (original)
+++ httpd/httpd/trunk/modules/aaa/mod_auth_digest.c Wed Jul 3 12:13:50 2013
@@ -1728,6 +1728,9 @@ static int authenticate_digest_user(requ
note_digest_auth_failure(r, conf, resp, 0);
return HTTP_UNAUTHORIZED;
}
+else if (return_code == AUTH_HANDLED) {
+return r->status;
+}
else {
/* AUTH_GENERAL_ERROR (or worse)
* We'll assume that the module has already said what its error
Re: yet another mod_ssl temp DH handling tweak
On Thu, Jun 19, 2014 at 04:29:17PM +0100, Joe Orton wrote:
> One more tweak here, proposed by Hubert from our QA team who has been
> reviewing all this stuff. Hubert argued we should be erring on the side
> of stronger not weaker here, particularly because of the possibility of
> 2048-bit keys being identified as 2047 in rare cases.
I was reminded that there was a request to use the larger key sizes as
well. So with apologies, I over-engineered this even more:
Any comments/objections/hatemail?
Index: ssl_engine_init.c
===
--- ssl_engine_init.c (revision 1603915)
+++ ssl_engine_init.c (working copy)
@@ -67,29 +67,39 @@
return dh;
}
-static DH *dhparam1024, *dhparam2048, *dhparam3072, *dhparam4096;
+/* Storage and initialization for DH parameters. */
+static struct dhparam {
+BIGNUM *(*const prime)(BIGNUM *); /* function to generate... */
+DH *dh; /* ...this, used for keys */
+const unsigned int min; /* ...of length >= this. */
+} dhparams[] = {
+{ get_rfc3526_prime_8192, NULL, 6145 },
+{ get_rfc3526_prime_6144, NULL, 4097 },
+{ get_rfc3526_prime_4096, NULL, 3073 },
+{ get_rfc3526_prime_3072, NULL, 2049 },
+{ get_rfc3526_prime_2048, NULL, 1025 },
+{ get_rfc2409_prime_1024, NULL, 0 }
+};
static void init_dh_params(void)
{
-/*
- * Prepare DH parameters from 1024 to 4096 bits, in 1024-bit increments
- */
-dhparam1024 = make_dh_params(get_rfc2409_prime_1024, "2");
-dhparam2048 = make_dh_params(get_rfc3526_prime_2048, "2");
-dhparam3072 = make_dh_params(get_rfc3526_prime_3072, "2");
-dhparam4096 = make_dh_params(get_rfc3526_prime_4096, "2");
+unsigned n;
+
+for (n = 0; n < sizeof(dhparams)/sizeof(dhparams[0]); n++)
+dhparams[n].dh = make_dh_params(dhparams[n].prime, "2");
}
static void free_dh_params(void)
{
+unsigned n;
+
/* DH_free() is a noop for a NULL parameter, so these are harmless
* in the (unexpected) case where these variables are already
* NULL. */
-DH_free(dhparam1024);
-DH_free(dhparam2048);
-DH_free(dhparam3072);
-DH_free(dhparam4096);
-dhparam1024 = dhparam2048 = dhparam3072 = dhparam4096 = NULL;
+for (n = 0; n < sizeof(dhparams)/sizeof(dhparams[0]); n++) {
+DH_free(dhparams[n].dh);
+dhparams[n].dh = NULL;
+}
}
/* Hand out the same DH structure though once generated as we leak
@@ -101,14 +111,13 @@
* to our copy. */
DH *modssl_get_dh_params(unsigned keylen)
{
-if (keylen >= 4096)
-return dhparam4096;
-else if (keylen >= 3072)
-return dhparam3072;
-else if (keylen >= 2048)
-return dhparam2048;
-else
-return dhparam1024;
+unsigned n;
+
+for (n = 0; n < sizeof(dhparams)/sizeof(dhparams[0]); n++)
+if (keylen >= dhparams[n].min)
+return dhparams[n].dh;
+
+return NULL; /* impossible to reach. */
}
static void ssl_add_version_components(apr_pool_t *p,
Re: svn commit: r1602989 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h modules/proxy/mod_proxy.h modules/proxy/mod_proxy_fdpass.c modules/proxy/proxy_util.c
On Thu, Jun 19, 2014 at 1:39 PM, Yann Ylavic wrote: > On Thu, Jun 19, 2014 at 6:36 PM, Jeff Trawick wrote: > > On Mon, Jun 16, 2014 at 4:26 PM, wrote: > >> > >> Author: ylavic > >> Date: Mon Jun 16 20:26:24 2014 > >> New Revision: 1602989 > >> > >> URL: http://svn.apache.org/r1602989 > >> Log: > >> mod_proxy: Don't limit the size of the connectable Unix Domain Socket > >> paths. > >> Since connect() to UDS path is used at several places, introduce > >> ap_proxy_connect_uds() in proxy_util. > >> > > > > mod_authnz_fcgi could use something like this when adding Unix socket > > support... (so maybe a core API?) Maybe I'll get a chance to experiment > > before this gets merged to the 2.4.x branch. > > > > It seems weird that proxy_fdpass's original implementation of the > function > > had the timeout handling logic, but the module did not set a timeout for > the > > socket (bad). I guess that should be fixed in proxy_fdpass. > > I first wanted to add some core helpers and use them in mod_cgid too > (didn't notice mod_authnz_fcgi was concerned with its APR_UNSPEC). > But the timeout handling difference made me limit that to > ap_proxy_connect_uds() only (and as a result put the connect in the > function too)... > > The ideal would be to do that in APR though (where the code looks > buggy too), and then use the apr_sockaddr_t where needed in httpd. > Actually I'm working on a that APR patch, however what if APR is > "fixed" on that point, do we still need the workaround in httpd for > the ones using an older APR lib that used to work (eg. with mod_cgid)? > IMO 2.4.x should keep compatibility with APR 1.5.x for quite a while. (Event already requires 1.5.x.) Hopefully there will be a period of time which isn't so short when a lot of people that want to try 2.4.latest can use distro-provided support libraries. -- Born in Roswell... married an alien... http://emptyhammock.com/ http://edjective.org/
Re: svn commit: r1602989 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h modules/proxy/mod_proxy.h modules/proxy/mod_proxy_fdpass.c modules/proxy/proxy_util.c
On Thu, Jun 19, 2014 at 6:36 PM, Jeff Trawick wrote: > On Mon, Jun 16, 2014 at 4:26 PM, wrote: >> >> Author: ylavic >> Date: Mon Jun 16 20:26:24 2014 >> New Revision: 1602989 >> >> URL: http://svn.apache.org/r1602989 >> Log: >> mod_proxy: Don't limit the size of the connectable Unix Domain Socket >> paths. >> Since connect() to UDS path is used at several places, introduce >> ap_proxy_connect_uds() in proxy_util. >> > > mod_authnz_fcgi could use something like this when adding Unix socket > support... (so maybe a core API?) Maybe I'll get a chance to experiment > before this gets merged to the 2.4.x branch. > > It seems weird that proxy_fdpass's original implementation of the function > had the timeout handling logic, but the module did not set a timeout for the > socket (bad). I guess that should be fixed in proxy_fdpass. I first wanted to add some core helpers and use them in mod_cgid too (didn't notice mod_authnz_fcgi was concerned with its APR_UNSPEC). But the timeout handling difference made me limit that to ap_proxy_connect_uds() only (and as a result put the connect in the function too)... The ideal would be to do that in APR though (where the code looks buggy too), and then use the apr_sockaddr_t where needed in httpd. Actually I'm working on a that APR patch, however what if APR is "fixed" on that point, do we still need the workaround in httpd for the ones using an older APR lib that used to work (eg. with mod_cgid)?
Re: svn commit: r1602989 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h modules/proxy/mod_proxy.h modules/proxy/mod_proxy_fdpass.c modules/proxy/proxy_util.c
On Mon, Jun 16, 2014 at 4:26 PM, wrote: > Author: ylavic > Date: Mon Jun 16 20:26:24 2014 > New Revision: 1602989 > > URL: http://svn.apache.org/r1602989 > Log: > mod_proxy: Don't limit the size of the connectable Unix Domain Socket > paths. > Since connect() to UDS path is used at several places, introduce > ap_proxy_connect_uds() in proxy_util. > > mod_authnz_fcgi could use something like this when adding Unix socket support... (so maybe a core API?) Maybe I'll get a chance to experiment before this gets merged to the 2.4.x branch. It seems weird that proxy_fdpass's original implementation of the function had the timeout handling logic, but the module did not set a timeout for the socket (bad). I guess that should be fixed in proxy_fdpass. > Modified: > httpd/httpd/trunk/CHANGES > httpd/httpd/trunk/include/ap_mmn.h > httpd/httpd/trunk/modules/proxy/mod_proxy.h > httpd/httpd/trunk/modules/proxy/mod_proxy_fdpass.c > httpd/httpd/trunk/modules/proxy/proxy_util.c > > Modified: httpd/httpd/trunk/CHANGES > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1602989&r1=1602988&r2=1602989&view=diff > > == > --- httpd/httpd/trunk/CHANGES [utf-8] (original) > +++ httpd/httpd/trunk/CHANGES [utf-8] Mon Jun 16 20:26:24 2014 > @@ -1,6 +1,9 @@ > -*- coding: > utf-8 -*- > Changes with Apache 2.5.0 > > + *) mod_proxy: Don't limit the size of the connectable Unix Domain Socket > + paths. [Christophe Jaillet, Yann Ylavic] > + >*) mod_ssl: dump SSL IO/state for the write side of the connection(s), > like reads (level TRACE4). [Yann Ylavic] > > @@ -31,7 +34,7 @@ Changes with Apache 2.5.0 > PR 56286 [Micha Lenk ] > >*) mod_proxy_fdpass: Fix computation of the size of 'struct sockaddr_un' > - when passed to 'connec()'. > + when passed to 'connect()'. > [Graham Dumpleton ] > >*) mod_socache_shmcb: Correct counting of expirations for status > display. > > Modified: httpd/httpd/trunk/include/ap_mmn.h > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/include/ap_mmn.h?rev=1602989&r1=1602988&r2=1602989&view=diff > > == > --- httpd/httpd/trunk/include/ap_mmn.h (original) > +++ httpd/httpd/trunk/include/ap_mmn.h Mon Jun 16 20:26:24 2014 > @@ -461,6 +461,7 @@ > Changes 3rd argument's type of > ap_mpm_register_socket_callback and > ap_mpm_register_socket_callback_timeout. > + * 20140611.1 (2.5.0-dev) Add ap_proxy_connect_uds(). > */ > > #define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */ > @@ -468,7 +469,7 @@ > #ifndef MODULE_MAGIC_NUMBER_MAJOR > #define MODULE_MAGIC_NUMBER_MAJOR 20140611 > #endif > -#define MODULE_MAGIC_NUMBER_MINOR 0 /* 0...n */ > +#define MODULE_MAGIC_NUMBER_MINOR 1 /* 0...n */ > > /** > * Determine if the server's current MODULE_MAGIC_NUMBER is at least a > > Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.h > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.h?rev=1602989&r1=1602988&r2=1602989&view=diff > > == > --- httpd/httpd/trunk/modules/proxy/mod_proxy.h (original) > +++ httpd/httpd/trunk/modules/proxy/mod_proxy.h Mon Jun 16 20:26:24 2014 > @@ -874,6 +874,17 @@ PROXY_DECLARE(int) ap_proxy_connect_back > proxy_conn_rec *conn, > proxy_worker *worker, > server_rec *s); > + > +/** > + * Make a connection to a Unix Domain Socket (UDS) path > + * @param sock UDS to connect > + * @param uds_path UDS path to connect to > + * @param ppool to make the sock addr > + * @return APR_SUCCESS or error status > + */ > +PROXY_DECLARE(apr_status_t) ap_proxy_connect_uds(apr_socket_t *sock, > + const char *uds_path, > + apr_pool_t *p); > /** > * Make a connection record for backend connection > * @param proxy_function calling proxy scheme (http, ajp, ...) > > Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_fdpass.c > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_fdpass.c?rev=1602989&r1=1602988&r2=1602989&view=diff > > == > --- httpd/httpd/trunk/modules/proxy/mod_proxy_fdpass.c (original) > +++ httpd/httpd/trunk/modules/proxy/mod_proxy_fdpass.c Mon Jun 16 20:26:24 > 2014 > @@ -24,12 +24,6 @@ > #error This module only works on unix platforms with the correct OS > support > #endif > > -#include "apr_version.h" > -#if APR_MAJOR_VERSION < 2 >
yet another mod_ssl temp DH handling tweak
One more tweak here, proposed by Hubert from our QA team who has been
reviewing all this stuff. Hubert argued we should be erring on the side
of stronger not weaker here, particularly because of the possibility of
2048-bit keys being identified as 2047 in rare cases.
Is this reasonable?
Index: ssl_engine_init.c
===
--- ssl_engine_init.c (revision 1603915)
+++ ssl_engine_init.c (working copy)
@@ -101,11 +101,11 @@
* to our copy. */
DH *modssl_get_dh_params(unsigned keylen)
{
-if (keylen >= 4096)
+if (keylen > 3072)
return dhparam4096;
-else if (keylen >= 3072)
+else if (keylen > 2048)
return dhparam3072;
-else if (keylen >= 2048)
+else if (keylen > 1024)
return dhparam2048;
else
return dhparam1024;
Re: Memory leak in mod_ssl ssl_callback_TmpDH
On Sat, Jun 14, 2014 at 10:35:28AM +0200, Kaspar Brand wrote: > Just a quick reminder... I think it's important to backport > r1597349/r1598107 + the additional adjustment for 2.4.10 (happy to vote > once it's settled on trunk). Sorry guys. http://svn.apache.org/viewvc?view=revision&revision=1603915
Re: svn commit: r1603863 - in /httpd/httpd/trunk/modules: aaa/mod_auth_basic.c http/http_filters.c
On Thu, Jun 19, 2014 at 2:59 PM, Jim Jagielski wrote:
>
> On Jun 19, 2014, at 8:43 AM, yla...@apache.org wrote:
>
>> Author: ylavic
>> Date: Thu Jun 19 12:43:05 2014
>> New Revision: 1603863
>>
>> URL: http://svn.apache.org/r1603863
>> Log:
>> Use unsigned bit flags (otherwise the non-zero value to be used is -1).
>>
>
> I don't understand that at all... A bit is either 1 or 0.
>
Well, it depends on what you are doing with that bit...
#include
int main(int argc, const char *argv[])
{
struct {
int s:1;
} bitfield;
bitfield.s = 1;
if (bitfield.s > 0) {
printf("positive\n");
}
else {
printf("negative or nul\n");
}
if (bitfield.s == 1) {
printf("one\n");
}
else {
printf("not one\n");
}
return 0;
}
$ ./bitfield
negative or nul
not one
Re: svn commit: r1603863 - in /httpd/httpd/trunk/modules: aaa/mod_auth_basic.c http/http_filters.c
On Jun 19, 2014, at 8:43 AM, yla...@apache.org wrote: > Author: ylavic > Date: Thu Jun 19 12:43:05 2014 > New Revision: 1603863 > > URL: http://svn.apache.org/r1603863 > Log: > Use unsigned bit flags (otherwise the non-zero value to be used is -1). > I don't understand that at all... A bit is either 1 or 0.
mod_ssl FakeBasicAuth, the colon problem (PR 52644)
I've had a user hit this: with FakeBasicAuth the client DN gets
translated into a Basic auth blob of base64("username:password"), which
then fails when the username part contains a ":" colon character.
At minimum mod_ssl could/should catch and fail auth under FakeBasicAuth
when DN is seen with a ":", that's easy enough. We *could* also try
escaping the colon, but that introduces an inevitable ambiguity since
there is no escaping standard.
One approach would be to escape any colon in the DN by replacing with
some unusual character sequence ("" or whatever) and then only fail
for unescaped DNs which contain that sequence to avoid ambiguity
problems.
Any opinions before I hack something up?
Probably the "correct" way to approach this problem is using Graham's
nice hacks in the trunk to allow users to construct an appropriate
username:password blog based on expressions:
http://svn.apache.org/viewvc?view=revision&revision=r1457471
but even that does not actually protect against the "colon problem".
Regards, Joe
