Re: [PATCH] Dynamic locking upcalls in mod_ssl
On Aug 20, 2008, at 11:56 AM, Plüm, Rüdiger, VF-Group wrote: You should set dynlockpool to NULL here as well. In case it is used afterwards things segfault and are easier to detected than when an invalid pointer is used. This should basicly address your question regarding the reference on a pool in global variable as well. Thanks Rüdiger, that's great feedback. With that, and a test run on my Linux box where it responded successfully to a restart, a graceful and a stop interspersed with ab runs that raised the number of worker children, I'll commit. Where the whole thing will of course be up for further review and consideration. S. -- Sander Temme [EMAIL PROTECTED] PGP FP: 51B4 8727 466A 0BC3 69F4 B7B8 B2BE BC40 1529 24AF smime.p7s Description: S/MIME cryptographic signature
Re: SNI in 2.2.x (Re: Time for 2.2.10?)
On Aug 20, 2008, at 7:10 AM, Nick Kew wrote: It might be worth a --with-SNI configuration option, which would label it as an experimental feature. +1, given that it'd be off by default. Anyone care to craft some autofoo? S. -- Sander Temme [EMAIL PROTECTED] PGP FP: 51B4 8727 466A 0BC3 69F4 B7B8 B2BE BC40 1529 24AF smime.p7s Description: S/MIME cryptographic signature
Re: Dropping mod_sed into /trunk/ ?
Nick Kew wrote: A little while ago, Basant Kukreja published mod_sed under the Apache license. He's now also written a blog entry that could become the basis for a tutorial into how mod_sed is much more than a mere string-or-regexp search-and-replace filter: http://blogs.sun.com/basant/entry/using_mod_sed_to_filter I happen to know that Basant and Sun will be happy for us to adopt mod_sed, and I think that with that blog entry reworked into a howto doc, it'll add real value to httpd. Any thoughts on dropping it in to trunk, with a view to including it as standard in 2.4 in due course? I have no objection. "mod_sed is much more than a mere string-or-regexp search-and-replace" It's nice to see my objections to an earlier module name reinforced by a practical example of why only mod_sed deserves the "sed" designation :)
Dropping mod_sed into /trunk/ ?
A little while ago, Basant Kukreja published mod_sed under the Apache license. He's now also written a blog entry that could become the basis for a tutorial into how mod_sed is much more than a mere string-or-regexp search-and-replace filter: http://blogs.sun.com/basant/entry/using_mod_sed_to_filter I happen to know that Basant and Sun will be happy for us to adopt mod_sed, and I think that with that blog entry reworked into a howto doc, it'll add real value to httpd. Any thoughts on dropping it in to trunk, with a view to including it as standard in 2.4 in due course? -- Nick Kew Application Development with Apache - the Apache Modules Book http://www.apachetutor.org/
Re: [PATCH] Dynamic locking upcalls in mod_ssl
> -Ursprüngliche Nachricht- > Von: Sander Temme > Gesendet: Mittwoch, 20. August 2008 16:37 > An: dev@httpd.apache.org > Cc: Joe Orton > Betreff: Re: [PATCH] Dynamic locking upcalls in mod_ssl > > > Index: modules/ssl/ssl_util.c > === > --- modules/ssl/ssl_util.c(revision 687227) > +++ modules/ssl/ssl_util.c(working copy) > @@ -351,6 +351,106 @@ > } > } > > +/* Global reference to the pool passed into > ssl_util_thread_setup() */ > +apr_pool_t *dynlockpool; Should be initialized with NULL. > + > +/* > + * Dynamic lock creation callback > + */ > +static struct CRYPTO_dynlock_value *ssl_dyn_create_function(const > char *file, > + int line) > +{ > +struct CRYPTO_dynlock_value *value; > +apr_pool_t *p; > +apr_status_t rv; > + > +/* > + * We need a pool to allocate our mutex. Since we can't clear > + * allocated memory from a pool, create a subpool that > we can blow > + * away in the destruction callback. > + */ > +rv = apr_pool_create(&p, dynlockpool); > +if (rv != APR_SUCCESS) { > +ap_log_perror(file, line, APLOG_ERR, rv, dynlockpool, > + "Failed to create subpool for dynamic lock"); > +return NULL; > +} > + > +ap_log_perror(file, line, APLOG_DEBUG, 0, p, > + "Creating dynamic lock"); > + > +value = (struct CRYPTO_dynlock_value *)apr_palloc(p, > + sizeof(struct > CRYPTO_dynlock_value)); > +if (!value) { > +ap_log_perror(file, line, APLOG_ERR, 0, p, > + "Failed to allocate dynamic lock structure"); > +return NULL; > +} > + > +value->pool = p; > +/* Keep our own copy of the place from which we were created, > + using our own pool. */ > +value->file = apr_psprintf(p, "%s", file); Hm, why not doing an apr_pstrdup here? This seems to be more efficient than an apr_psprintf in this situation. > +value->line = line; > +rv = apr_thread_mutex_create(&(value->mutex), > APR_THREAD_MUTEX_DEFAULT, > +p); > +if (rv != APR_SUCCESS) { > +ap_log_perror(file, line, APLOG_ERR, rv, p, > + "Failed to create thread mutex for dynamic > lock"); > +apr_pool_destroy(p); > +return NULL; > +} > +return value; > +} > + > +/* > + * Dynamic locking and unlocking function > + */ > + > +static void ssl_dyn_lock_function(int mode, struct > CRYPTO_dynlock_value *l, > + const char *file, int line) > +{ > +apr_status_t rv; > + > +if (mode & CRYPTO_LOCK) { > +ap_log_perror(file, line, APLOG_DEBUG, 0, l->pool, > + "Acquiring mutex %s:%d", l->file, l->line); > +rv = apr_thread_mutex_lock(l->mutex); > +ap_log_perror(file, line, APLOG_DEBUG, rv, l->pool, > + "Mutex %s:%d acquired!", l->file, l->line); > +} > +else { > +ap_log_perror(file, line, APLOG_DEBUG, 0, l->pool, > + "Releasing mutex %s:%d", l->file, l->line); > +rv = apr_thread_mutex_unlock(l->mutex); > +ap_log_perror(file, line, APLOG_DEBUG, rv, l->pool, > + "Mutex %s:%d released!", l->file, l->line); > +} > +} > + > +/* > + * Dynamic lock destruction callback > + */ > +static void ssl_dyn_destroy_function(struct CRYPTO_dynlock_value *l, > + const char *file, int line) > +{ > +apr_status_t rv; > + > +ap_log_perror(file, line, APLOG_DEBUG, 0, l->pool, > + "Destroying dynamic lock %s:%d", l->file, l->line); > +rv = apr_thread_mutex_destroy(l->mutex); > +if (rv != APR_SUCCESS) { > +ap_log_perror(file, line, APLOG_ERR, rv, l->pool, > + "Failed to destroy mutex for dynamic > lock %s:%d", > + l->file, l->line); > +} > + > +/* Trust that whomever owned the CRYPTO_dynlock_value we were > + * passed has no future use for it... > + */ > +apr_pool_destroy(l->pool); > +} > + > static unsigned long ssl_util_thr_id(void) > { > /* OpenSSL needs this to return an unsigned long. On OS/390, > the pthread > @@ -373,6 +473,10 @@ > { > CRYPTO_set_locking_callback(NULL); > CRYPTO_set_id_callback(NULL); > + > +CRYPTO_set_dynlock_create_callback(NULL); > +CRYPTO_set_dynlock_lock_callback(NULL); > +CRYPTO_set_dynlock_destroy_callback(NULL); You should set dynlockpool to NULL here as well. In case it is used afterwards things segfault and are easier to detected than when an invalid pointer is used. This should basicly address your question regarding the reference on a pool in global variable as well. Regards Rüdiger
Re: [PATCH] Dynamic locking upcalls in mod_ssl
On Aug 18, 2008, at 5:18 AM, Joe Orton wrote: So generally pconf is the right pool to use, along with a cleanup registered against that pool which sets the callbacks to NULL. Yes, with the cleanup it no longer hangs. What about stashing a pool reference in a global, is that a red flag? The patch now looks something like this, with the rv change Rüdiger suggested and some tweaks to get the types in line and make the compiler shut up: Index: modules/ssl/ssl_private.h === --- modules/ssl/ssl_private.h (revision 687227) +++ modules/ssl/ssl_private.h (working copy) @@ -463,6 +463,16 @@ } SSLDirConfigRec; /** + * Dynamic lock structure + */ +struct CRYPTO_dynlock_value { +apr_pool_t *pool; +const char* file; +int line; +apr_thread_mutex_t *mutex; +}; + +/** * function prototypes */ Index: modules/ssl/ssl_engine_init.c === --- modules/ssl/ssl_engine_init.c (revision 687227) +++ modules/ssl/ssl_engine_init.c (working copy) @@ -321,6 +321,9 @@ ssl_log_ssl_error(APLOG_MARK, APLOG_ERR, s); ssl_die(); } +ap_log_error(APLOG_MARK, APLOG_INFO, 0, s, + "Init: loaded Crypto Device API `%s'", + mc->szCryptoDevice); ENGINE_free(e); } Index: modules/ssl/ssl_util.c === --- modules/ssl/ssl_util.c (revision 687227) +++ modules/ssl/ssl_util.c (working copy) @@ -351,6 +351,106 @@ } } +/* Global reference to the pool passed into ssl_util_thread_setup() */ +apr_pool_t *dynlockpool; + +/* + * Dynamic lock creation callback + */ +static struct CRYPTO_dynlock_value *ssl_dyn_create_function(const char *file, + int line) +{ +struct CRYPTO_dynlock_value *value; +apr_pool_t *p; +apr_status_t rv; + +/* + * We need a pool to allocate our mutex. Since we can't clear + * allocated memory from a pool, create a subpool that we can blow + * away in the destruction callback. + */ +rv = apr_pool_create(&p, dynlockpool); +if (rv != APR_SUCCESS) { +ap_log_perror(file, line, APLOG_ERR, rv, dynlockpool, + "Failed to create subpool for dynamic lock"); +return NULL; +} + +ap_log_perror(file, line, APLOG_DEBUG, 0, p, + "Creating dynamic lock"); + +value = (struct CRYPTO_dynlock_value *)apr_palloc(p, + sizeof(struct CRYPTO_dynlock_value)); +if (!value) { +ap_log_perror(file, line, APLOG_ERR, 0, p, + "Failed to allocate dynamic lock structure"); +return NULL; +} + +value->pool = p; +/* Keep our own copy of the place from which we were created, + using our own pool. */ +value->file = apr_psprintf(p, "%s", file); +value->line = line; +rv = apr_thread_mutex_create(&(value->mutex), APR_THREAD_MUTEX_DEFAULT, +p); +if (rv != APR_SUCCESS) { +ap_log_perror(file, line, APLOG_ERR, rv, p, + "Failed to create thread mutex for dynamic lock"); +apr_pool_destroy(p); +return NULL; +} +return value; +} + +/* + * Dynamic locking and unlocking function + */ + +static void ssl_dyn_lock_function(int mode, struct CRYPTO_dynlock_value *l, + const char *file, int line) +{ +apr_status_t rv; + +if (mode & CRYPTO_LOCK) { +ap_log_perror(file, line, APLOG_DEBUG, 0, l->pool, + "Acquiring mutex %s:%d", l->file, l->line); +rv = apr_thread_mutex_lock(l->mutex); +ap_log_perror(file, line, APLOG_DEBUG, rv, l->pool, + "Mutex %s:%d acquired!", l->file, l->line); +} +else { +ap_log_perror(file, line, APLOG_DEBUG, 0, l->pool, + "Releasing mutex %s:%d", l->file, l->line); +rv = apr_thread_mutex_unlock(l->mutex); +ap_log_perror(file, line, APLOG_DEBUG, rv, l->pool, + "Mutex %s:%d released!", l->file, l->line); +} +} + +/* + * Dynamic lock destruction callback + */ +static void ssl_dyn_destroy_function(struct CRYPTO_dynlock_value *l, + const char *file, int line) +{ +apr_status_t rv; + +ap_log_perror(file, line, APLOG_DEBUG, 0, l->pool, + "Destroying dynamic lock %s:%d", l->file, l->line); +rv = apr_thread_mutex_destroy(l->mutex); +if (rv != APR_SUCCESS) { +ap_log_perror(file, line, APLOG_ERR, rv, l->pool, + "Failed to destroy mutex for dynamic lock %s:%d", + l->file, l->line); +} + +/* Trust that whomever owned the CRYPTO_dynlock_value we were + * passed has no futur
Re: SNI in 2.2.x (Re: Time for 2.2.10?)
On Wed, Aug 20, 2008 at 02:08:02PM +0200, Jorge Schrauwen wrote: > I like the idea of using --with-SNI and labeling it as experimental. Yeah, good way to move forward. > Maybe leave it of by default though? > absolutely. It would seem rather odd to turn on experimental by default. vh Mads Toftum -- http://soulfood.dk
Re: SNI in 2.2.x (Re: Time for 2.2.10?)
I like the idea of using --with-SNI and labeling it as experimental. Maybe leave it of by default though? ~ Jorge On Wed, Aug 20, 2008 at 1:10 PM, Nick Kew <[EMAIL PROTECTED]> wrote: > On Wed, 20 Aug 2008 12:06:33 +0200 > Oden Eriksson <[EMAIL PROTECTED]> wrote: > > > FYI: This patch is applied in Mandriva Linux. > > Any feedback? Bug reports coming from their users? > > If you'd said Debuntu or Deadrat+family, we might infer a user > community big enough to rely on (FSreasonableVO rely). > Not sure about Mandriva, but it's surely better than nothing. > > It seems clear that users *really* want it. I'd say that adds > weight to the argument for including it and taking the risk. > It might be worth a --with-SNI configuration option, which > would label it as an experimental feature. > > -- > Nick Kew >
Re: SNI in 2.2.x (Re: Time for 2.2.10?)
On Wed, 20 Aug 2008 12:06:33 +0200 Oden Eriksson <[EMAIL PROTECTED]> wrote: > FYI: This patch is applied in Mandriva Linux. Any feedback? Bug reports coming from their users? If you'd said Debuntu or Deadrat+family, we might infer a user community big enough to rely on (FSreasonableVO rely). Not sure about Mandriva, but it's surely better than nothing. It seems clear that users *really* want it. I'd say that adds weight to the argument for including it and taking the risk. It might be worth a --with-SNI configuration option, which would label it as an experimental feature. -- Nick Kew
Re: SNI in 2.2.x (Re: Time for 2.2.10?)
Den Tuesday 19 August 2008 08:16:08 skrev Kaspar Brand: > Ruediger Pluem wrote: > > At the moment we have 9 entries in the CHANGES file for 2.2.10 and > > there are 5 more proposals in the STATUS file that are missing only > > one vote. I think if get these done we also have enough stuff from > > pure httpd point of view that warrants a release. WDYT? > > May I use this occasion to ask if there's still a chance of getting a > backport of SNI accepted for 2.2.x? Following suggestions from Joe, I > went through all relevant SSL* config directives and posted my findings > in two parts on 9th/18th June: > > http://mail-archives.apache.org/mod_mbox/httpd-dev/200806.mbox/%3c484CBAA6. >[EMAIL PROTECTED] > http://mail-archives.apache.org/mod_mbox/httpd-dev/200806.mbox/%3c48592955. >[EMAIL PROTECTED] > > The patch I posted on 18 June introduced a regression, however, so I > posted an updated version on 26th June: > > http://mail-archives.apache.org/mod_mbox/httpd-dev/200806.mbox/%3c4863C04C. >[EMAIL PROTECTED] > > That's the version I still consider suitable for check-in to trunk > (attached again for convenience). A backport to 2.2.x is available at > > http://sni.velox.ch/httpd-2.2.x-sni.diff > > If, on the other hand, people think that SNI isn't important enough for > 2.2.x, then I'd be glad to hear that as well (it doesn't make sense to > repeatedly nag the list about that topic, I think). > > Thanks, > Kaspar FYI: This patch is applied in Mandriva Linux.
Re: dev Digest 20 Aug 2008 06:49:31 -0000 Issue 2669
Paul wrote: It is important enough, the problem is we don't want to a back port to cause regression or other sidee effects, and to me that is the scariest thing about the SNI patch(es). There might be a compromise position here: As long as the SNI patch causes no problem to other services, this could be ok. If later on SNI users had to put up with some fast fixes, this wouldn't be so much of an issue. Serious users of TLS (e.g., banks) won't be using SNI for a while. It is the grass roots people, the LAMPS vhost people, that we want to get into TLS. Right now they are not. Getting them into TLS/SNI even with a few potential scary events, is still a huge win. (Indeed, if TLS/SNI is thought to be wobbly by the mainstreet TLS operators for a while, that's fine.) Normally, this aggressive approach might not be so good. But in the case of real live attacks, if we delay a year for a perfect secure implementation, then we lose a year, over-all, and the victims lose another billion. Those numbers are much scarier than a few hiccups over SNI. iang smime.p7s Description: S/MIME Cryptographic Signature
Re: SNI in 2.2.x (Re: Time for 2.2.10?)
[EMAIL PROTECTED] wrote: May I use this occasion to ask if there's still a chance of getting a backport of SNI accepted for 2.2.x? For me, +1. For the LAMPs guys, +1m. For the phishing victims, +10m. Ok, the numbers are fingers in the air, but the essence is right. We need to move much much more http services into secured sites, and the *only* efficient way to do this is via TLS/SNI. thanks for good work so far! If, on the other hand, people think that SNI isn't important enough for 2.2.x, then I'd be glad to hear that as well (it doesn't make sense to repeatedly nag the list about that topic, I think). It is IMHO the most important change in the last 10 years. It makes TLS in Apache's HTTPD product work like virtual hosts. It means all those LAMPs guys that share servers can now use TLS to provide site authentication. It is the only issue in TLS that contributes to an active, dynamic, attacker. The losses to direct phishing (lack of proper site authentication) were around a billion, and the same attacker is now doing around 3 billion a year. Also, see the current DNS issues. We can't do routine boring LAMPs-level end-to-end authentication of the site without TLS/SNI. (So we don't.) iang smime.p7s Description: S/MIME Cryptographic Signature
[PATCH] Add directive to skip authentication when using client certificates
Hello, The following patch against trunk adds a directive "AuthBasicUserFromSSL" (On/Off) to mod_auth_basic. Setting this to "On" would skip authentication if r->user is set by mod_ssl. This is needed when using client certificates for authentication, because in this case you don't get any password from the user, which you can use to authenticate. Well, there is FakeBasicAuth, but setting the password to "password" for every user in a directory is definitely no solution. Would be nice if we could include this in 2.2.x too. The affected code is basically similar. See also discussion at http://mail-archives.apache.org/mod_mbox/httpd-dev/200807.mbox/[EMAIL PROTECTED] Configuration may look like this: SSLUserName SSL_CLIENT_S_DN_CN SSLVerifyClient require AuthTypeBasic AuthName"Test" AuthBasicUserFromSSLOn AuthBasicProvider ldap AuthLDAPUrl ldap://myldapserver.company.com:389/ou=Users,o=COMPANY,c=COM?uid?sub AuthLDAPBindDN cn=myUser,ou=users,o=COMPANY,c=COM AuthLDAPBindPasswordmyPassword require ldap-group cn=mygroup,ou=Groups,o=COMPANY,c=COM Greetings, Johannes Müller mod_auth_basic.patch Description: mod_auth_basic.patch