Re: svn commit: r220307 - in /httpd/httpd/trunk/modules: metadata/mod_setenvif.c ssl/mod_ssl.c ssl/mod_ssl.h ssl/ssl_expr_eval.c
David Reid wrote: Joe Orton wrote: On Fri, Aug 05, 2005 at 08:00:01PM +0200, Martin Kraemer wrote: On Tue, Aug 02, 2005 at 07:14:10PM +0200, Martin Kraemer wrote: I wanted something like SSLRequire "committers" in SSLPeerExtList("1.3.6.1.4.1.18060.1"); to mean "at least one extension with an OID of 1.3.6.1.4.1.18060.1 with a value of 'committers' exists in the client cert". I'll be on vacation next week, and will send in another patch after that. OK, hope you had a good holiday. I wasn't trying to argue about the semantics just to nitpick the naming. Having "SSL" in the SSLRequire function is redundant, but not in the context of mod_setenvif. So, my preference is: SSLRequire "committers" in PeerExtList("1.3.6.1.4.1.18060.1"); SetEnvIf SSL_PeerExtList("etc") ... +1 on the revised naming. Patch looks OK as well. Late to the party here, but giving the same function two different names sucks! Or am I misunderstanding? Cheers, Ben. -- http://www.apache-ssl.org/ben.html http://www.thebunker.net/ "There is no limit to what a man can do or how far he can go if he doesn't mind who gets the credit." - Robert Woodruff
Re: svn commit: r220307 - in /httpd/httpd/trunk/modules: metadata/mod_setenvif.c ssl/mod_ssl.c ssl/mod_ssl.h ssl/ssl_expr_eval.c
On Tue, Aug 16, 2005 at 04:45:41PM +0100, David Reid wrote: > Joe Orton wrote: > > On Mon, Aug 15, 2005 at 02:36:18PM +0100, Joe Orton wrote: > > > >>I just went to write a test case for the SetEnvIf function, and there > >>seems to be a rather annoying fundamental problem: the match_headers > >>hooks runs too early to be useful for this when doing per-dir client > >>cert negotiation. > > > > > > I can't see any simple way round this, and I don't think this feature > > should go in 2.2 unless this can be solved. Any ideas? > > I've not looked at it in detail, so would have to dig through the code. > Care to post your test case? Well, try testing it in any configuration with "SSLVerifyClient require" in Directory or Location context rather than in the vhost context. httpd-test test case: mkdir t/htdocs/modules/setenvif/ssl + applying Index: t/conf/extra.conf.in === --- t/conf/extra.conf.in(revision 231019) +++ t/conf/extra.conf.in(working copy) @@ -358,6 +358,11 @@ Options +Includes AllowOverride All + + +Options +Includes +AllowOverride All + ## Index: t/htdocs/modules/setenvif/ssl/.htaccess === --- t/htdocs/modules/setenvif/ssl/.htaccess (revision 0) +++ t/htdocs/modules/setenvif/ssl/.htaccess (revision 0) @@ -0,0 +1 @@ +SetEnvIf SSL_PeerExtList("2.16.840.1.113730.1.13") "(.*)" NETSCAPE_COMMENT=$1 Property changes on: t/htdocs/modules/setenvif/ssl/.htaccess ___ Name: svn:eol-style + native Index: t/htdocs/modules/setenvif/ssl/peerextlist.shtml === --- t/htdocs/modules/setenvif/ssl/peerextlist.shtml (revision 0) +++ t/htdocs/modules/setenvif/ssl/peerextlist.shtml (revision 0) @@ -0,0 +1 @@ +0: Property changes on: t/htdocs/modules/setenvif/ssl/peerextlist.shtml ___ Name: svn:eol-style + native Index: t/ssl/setenvif.t === --- t/ssl/setenvif.t(revision 0) +++ t/ssl/setenvif.t(revision 0) @@ -0,0 +1,21 @@ +use strict; +use warnings FATAL => 'all'; + +use Apache::Test; +use Apache::TestRequest; +use Apache::TestUtil; + +plan tests => 2, need 'setenvif', need_min_apache_version("2.1.6"); + +Apache::TestRequest::scheme("https"); + +my $r = GET("/require/asf/modules/setenvif/ssl/peerextlist.shtml", cert => 'client_ok'); + +ok t_cmp($r->code, 200, "fetched page works"); + +my $c = $r->content; + +chomp $c; + +ok t_cmp($c, "0:This Is A Comment", "Retrieve nsComment extension"); +
Re: svn commit: r220307 - in /httpd/httpd/trunk/modules: metadata/mod_setenvif.c ssl/mod_ssl.c ssl/mod_ssl.h ssl/ssl_expr_eval.c
Joe Orton wrote: > On Mon, Aug 15, 2005 at 02:36:18PM +0100, Joe Orton wrote: > >>I just went to write a test case for the SetEnvIf function, and there >>seems to be a rather annoying fundamental problem: the match_headers >>hooks runs too early to be useful for this when doing per-dir client >>cert negotiation. > > > I can't see any simple way round this, and I don't think this feature > should go in 2.2 unless this can be solved. Any ideas? I've not looked at it in detail, so would have to dig through the code. Care to post your test case? david
Re: svn commit: r220307 - in /httpd/httpd/trunk/modules: metadata/mod_setenvif.c ssl/mod_ssl.c ssl/mod_ssl.h ssl/ssl_expr_eval.c
On Mon, Aug 15, 2005 at 02:36:18PM +0100, Joe Orton wrote: > I just went to write a test case for the SetEnvIf function, and there > seems to be a rather annoying fundamental problem: the match_headers > hooks runs too early to be useful for this when doing per-dir client > cert negotiation. I can't see any simple way round this, and I don't think this feature should go in 2.2 unless this can be solved. Any ideas? joe
Re: svn commit: r220307 - in /httpd/httpd/trunk/modules: metadata/mod_setenvif.c ssl/mod_ssl.c ssl/mod_ssl.h ssl/ssl_expr_eval.c
On Mon, Aug 15, 2005 at 02:36:18PM +0100, Joe Orton wrote: > OK, hope you had a good holiday. I wasn't trying to argue about the > semantics just to nitpick the naming. Having "SSL" in the SSLRequire > function is redundant, but not in the context of mod_setenvif. So, my > preference is: > > SSLRequire "committers" in PeerExtList("1.3.6.1.4.1.18060.1"); > > SetEnvIf SSL_PeerExtList("etc") ... > +1 on concept vh Mads Toftum -- `Darn it, who spiked my coffee with water?!' - lwall
Re: svn commit: r220307 - in /httpd/httpd/trunk/modules: metadata/mod_setenvif.c ssl/mod_ssl.c ssl/mod_ssl.h ssl/ssl_expr_eval.c
Joe Orton wrote: > On Fri, Aug 05, 2005 at 08:00:01PM +0200, Martin Kraemer wrote: > >>On Tue, Aug 02, 2005 at 07:14:10PM +0200, Martin Kraemer wrote: >> >>>I wanted something like >>> >>> SSLRequire "committers" in SSLPeerExtList("1.3.6.1.4.1.18060.1"); >>> >>>to mean "at least one extension with an OID of >>>1.3.6.1.4.1.18060.1 with a value of 'committers' exists in the >>>client cert". >> >>I'll be on vacation next week, and will send in another patch after >>that. > > > OK, hope you had a good holiday. I wasn't trying to argue about the > semantics just to nitpick the naming. Having "SSL" in the SSLRequire > function is redundant, but not in the context of mod_setenvif. So, my > preference is: > > SSLRequire "committers" in PeerExtList("1.3.6.1.4.1.18060.1"); > > SetEnvIf SSL_PeerExtList("etc") ... +1 on the revised naming. Patch looks OK as well. david
Re: svn commit: r220307 - in /httpd/httpd/trunk/modules: metadata/mod_setenvif.c ssl/mod_ssl.c ssl/mod_ssl.h ssl/ssl_expr_eval.c
On Fri, Aug 05, 2005 at 08:00:01PM +0200, Martin Kraemer wrote: > On Tue, Aug 02, 2005 at 07:14:10PM +0200, Martin Kraemer wrote: > > I wanted something like > > > > SSLRequire "committers" in SSLPeerExtList("1.3.6.1.4.1.18060.1"); > > > > to mean "at least one extension with an OID of > > 1.3.6.1.4.1.18060.1 with a value of 'committers' exists in the > > client cert". > > I'll be on vacation next week, and will send in another patch after > that. OK, hope you had a good holiday. I wasn't trying to argue about the semantics just to nitpick the naming. Having "SSL" in the SSLRequire function is redundant, but not in the context of mod_setenvif. So, my preference is: SSLRequire "committers" in PeerExtList("1.3.6.1.4.1.18060.1"); SetEnvIf SSL_PeerExtList("etc") ... I just went to write a test case for the SetEnvIf function, and there seems to be a rather annoying fundamental problem: the match_headers hooks runs too early to be useful for this when doing per-dir client cert negotiation. Attached the patch I have for mod_setenvif to clean it up and adopt the naming above; untested so far as I'm blocked by the fact that it doesn't work for per-dir negotiation. joe Index: modules/metadata/mod_setenvif.c === --- modules/metadata/mod_setenvif.c (revision 232271) +++ modules/metadata/mod_setenvif.c (working copy) @@ -104,7 +104,7 @@ SPECIAL_REQUEST_METHOD, SPECIAL_REQUEST_PROTOCOL, SPECIAL_SERVER_ADDR, -SPECIAL_OID_VALUE +SPECIAL_SSL_PEEREXTLIST }; typedef struct { char *name; /* header name */ @@ -349,30 +349,30 @@ else if (!strcasecmp(fname, "server_addr")) { new->special_type = SPECIAL_SERVER_ADDR; } -else if (!strncasecmp(fname, "oid(",4)) { -ap_regmatch_t match[AP_MAX_REG_MATCH]; +else if (!strncasecmp(fname, "ssl_peerextlist(", + sizeof("ssl_peerextlist(") - 1)) { +const char *oid, *oidend; -new->special_type = SPECIAL_OID_VALUE; +new->special_type = SPECIAL_SSL_PEEREXTLIST; -/* Syntax check and extraction of the OID as a regex: */ -new->pnamereg = ap_pregcomp(cmd->pool, -"^oid\\(\"?([0-9.]+)\"?\\)$", -(AP_REG_EXTENDED // | AP_REG_NOSUB - | AP_REG_ICASE)); -/* this can never happen, as long as pcre works: - if (new->pnamereg == NULL) -return apr_pstrcat(cmd->pool, cmd->cmd->name, - "OID regex could not be compiled.", NULL); - */ -if (ap_regexec(new->pnamereg, fname, AP_MAX_REG_MATCH, match, 0) == AP_REG_NOMATCH) { +oid = fname + strlen("ssl_peerextlist("); +oidend = ap_strchr_c(oid, ')'); + +/* skip over quotes if present */ +if (oid[0] == '"') { +oid++; +} +if (oidend && oidend[-1] == '"') { +oidend--; +} + +if (!oidend || oidend <= oid) { return apr_pstrcat(cmd->pool, cmd->cmd->name, - "OID syntax is: oid(\"1.2.3.4.5\"); error in: ", - fname, NULL); + "invalid ssl_peerextlist() syntax in: ", + fname, NULL); } -new->pnamereg = NULL; -/* The name field is used for the stripped oid string */ -new->name = fname = apr_pstrdup(cmd->pool, fname+match[1].rm_so); -fname[match[1].rm_eo - match[1].rm_so] = '\0'; + +new->name = apr_pstrmemdup(cmd->pool, oid, oidend - oid); } else { new->special_type = SPECIAL_NOT; @@ -504,8 +504,10 @@ * same header. Remember we don't need to strcmp the two header * names because we made sure the pointers were equal during * configuration. - * In the case of SPECIAL_OID_VALUE values, each oid string is - * dynamically allocated, thus there are no duplicates. + * + * In the case of SPECIAL_SSL_PEEREXTLIST values, each + * extension list is dynamically allocated, thus there are no + * duplicates. */ if (b->name != last_name) { last_name = b->name; @@ -529,32 +531,19 @@ case SPECIAL_REQUEST_PROTOCOL: val = r->protocol; break; -case SPECIAL_OID_VALUE: +case SPECIAL_SSL_PEEREXTLIST: /* If mod_ssl is not loaded, the accessor function is NULL */ -if (ssl_extlist_by_oid_func != NULL) -{ -apr_array_header_t *oid_array; -cha
Re: svn commit: r220307 - in /httpd/httpd/trunk/modules: metadata/mod_setenvif.c ssl/mod_ssl.c ssl/mod_ssl.h ssl/ssl_expr_eval.c
On Tue, Aug 02, 2005 at 07:14:10PM +0200, Martin Kraemer wrote: > I wanted something like > > SSLRequire "committers" in SSLPeerExtList("1.3.6.1.4.1.18060.1"); > > to mean "at least one extension with an OID of > 1.3.6.1.4.1.18060.1 with a value of 'committers' exists in the > client cert". I'll be on vacation next week, and will send in another patch after that. L8er, Martin -- <[EMAIL PROTECTED]> | Fujitsu Siemens Fon: +49-89-636-46021, FAX: +49-89-636-48332 | 81730 Munich, Germany
Re: svn commit: r220307 - in /httpd/httpd/trunk/modules: metadata/mod_setenvif.c ssl/mod_ssl.c ssl/mod_ssl.h ssl/ssl_expr_eval.c
On Tue, Aug 02, 2005 at 03:03:45PM +0100, Joe Orton wrote: > On Tue, Aug 02, 2005 at 03:23:44PM +0200, Martin Kraemer wrote: > > The problem with the OID() "function" is that it where file() (or > > another file() like function) return a single value, what OID() > > stands for is an "array of zero or more values". But there is no > > syntax to deal with arrays in place of expressions. I tried to > > implement it as function first, but noticed that it would break when > > an OID was specified more than once. In the ASF scenario, the > > intention is to add multiple extensions with this OID, each one > > containing as value a group name of which the client is member. > > > > Because of the pre-existing syntax " in {value,value}", and > > because "{value,value}" is effectively an array, I chose to implement > > the OID() "function" as a special case of the " in" operator. > > No, OK, that makes sense. Perhaps a more general implementation would > be to have a "peerextlist()" which evalutes to a "wordlist" array like > the current function, along with a "peerext()" which evaluates to a > "word" (just the first extension with given name). Hm. I'm not totally happy with this, or maybe I misunderstand what you want to achieve. I wanted something like SSLRequire "committers" in SSLPeerExtList("1.3.6.1.4.1.18060.1"); to mean "at least one extension with an OID of 1.3.6.1.4.1.18060.1 with a value of 'committers' exists in the client cert". How should SSLPeerExts() be used then? Maybe... SSLRequire SSLPeerExts("1.3.6.1.4.1.18060.1") =~ m/(^|,)(committers|administrators)(,|$)/ meaning: if in the ','-collapsed string (unclear; I am not happy with the idea to just take the 1st occurrence) of all extension values with an OID of 1.3.6.1.4.1.18060.1, does the word "committers" or "administrator" start at the beginning or after a comma, and does it end at the end or at another comma? (That would be possible without further ado, because the left-hand-side expression can also be an environment variable. And with the ','-collapsed string of all extension values available from mod_setenvif, used like this: SetEnvIf SSLPeerExts("1.3.6.1.4.1.18060.1") "(.*)" SSL_PEER_EXT_ASF=$1 SSLRequire %{ENV:SSL_PEER_EXT_ASF} =~ m/(^|,)(committers|administrators)(,|$)/ > I'd prefer peerexts() or peerextlist() given the above, to emphasize the > the fact that it returns a list, and to allow a peerext() which doesn't > do that, in the future. I think that when similar functionality is offered, the naming should be similar as well. If I use "SSLPeerExts" in mod_setenvif, then of course it should be called SSLPeerExts or SSLPeerExtList in mod_ssl too (both offer access to the SSL certificate extension). > Also note that apr_array_pstrcat(pool, array, ',') can be used instead > of all that code to flatten the array to a string. Ah, yes, thanks. > Other than the choice of name, this looks fine. Please split out the > unrelated change to match tokens case-insensitively to a separate > commit. Of course. BTW: do you think case insensitivity for the keywords is a good idea? I do, but I don't know if it would cause misinterpretation for some existing config files. Like, when someone was looking for a string "EQ", will the parser now bail out because it becomes a keyword? Martin -- <[EMAIL PROTECTED]> | Fujitsu Siemens Fon: +49-89-636-46021, FAX: +49-89-636-48332 | 81730 Munich, Germany
Re: svn commit: r220307 - in /httpd/httpd/trunk/modules: metadata/mod_setenvif.c ssl/mod_ssl.c ssl/mod_ssl.h ssl/ssl_expr_eval.c
On Tue, Aug 02, 2005 at 03:23:44PM +0200, Martin Kraemer wrote: > On Tue, Aug 02, 2005 at 12:00:24PM +0100, Joe Orton wrote: > >> 1) this is a pretty specific to way to code it. Is there no way to make > >> it more general so that OID() is just a function like file() and can be > >> used e.g. in regex matches too? > > The problem with the OID() "function" is that it where file() (or > another file() like function) return a single value, what OID() > stands for is an "array of zero or more values". But there is no > syntax to deal with arrays in place of expressions. I tried to > implement it as function first, but noticed that it would break when > an OID was specified more than once. In the ASF scenario, the > intention is to add multiple extensions with this OID, each one > containing as value a group name of which the client is member. > > Because of the pre-existing syntax " in {value,value}", and > because "{value,value}" is effectively an array, I chose to implement > the OID() "function" as a special case of the " in" operator. > > Do you have a good idea how to use a function-like syntax, and still > maintain the "is an array" property? No, OK, that makes sense. Perhaps a more general implementation would be to have a "peerextlist()" which evalutes to a "wordlist" array like the current function, along with a "peerext()" which evaluates to a "word" (just the first extension with given name). > >> 3) oid() is a terrible name for this; it's simply the type of the > >> parameter. It would be like calling malloc() "size()". The function > >> expands (conceptually) to the values of an extension in the peer's > >> certificate, identified by OID; so call it peerext() or something > >> meaningful like that. > > Good point - Thanks a lot -- that is a *very* good idea, and (if > nobody objects) I'd want to follow this suggestion. I had been a > little unhappy with OID() myself. peerext() is especially good > because it also documents where the certificate came from. I'd prefer peerexts() or peerextlist() given the above, to emphasize the the fact that it returns a list, and to allow a peerext() which doesn't do that, in the future. > >> > SetEnvIf OID("2.16.840.1.113730.1.13") "(.*) Generated (Certificate)" > >> > ca=$1 > >> > >> -1 on the naming since OID is completely entirely meaningless in this > >> context. > > In the context of mod_setenvif, I'd even use "SSLPeerExt()" because > it makes it clear that we are dealing with an SSL-related thing. > > Patch <> attached. Likewise on naming; sslpeerexts() or sslpeerextlist() seems better although it's perhaps getting a little long. Also note that apr_array_pstrcat(pool, array, ',') can be used instead of all that code to flatten the array to a string. > In <> there is a patch which changes OID() > to SSLPeerExt() for mod_ssl. Other than the choice of name, this looks fine. Please split out the unrelated change to match tokens case-insensitively to a separate commit. Regards, joe
Re: svn commit: r220307 - in /httpd/httpd/trunk/modules: metadata/mod_setenvif.c ssl/mod_ssl.c ssl/mod_ssl.h ssl/ssl_expr_eval.c
On Tue, Aug 02, 2005 at 12:00:24PM +0100, Joe Orton wrote: > On Fri, Jul 22, 2005 at 02:24:50PM +0200, Sander Striker wrote: > > Joe Orton wrote: > > >On Fri, Jul 22, 2005 at 12:11:56PM -, Martin Kraemer wrote: > > > > > >>Author: martin > > >>Date: Fri Jul 22 05:11:55 2005 > > >>New Revision: 220307 > > >> > > >>URL: http://svn.apache.org/viewcvs?rev=220307&view=rev > > >>Log: > > >>Allow extraction of the values of SSL certificate extensions into > > >>environment variables, so that their value can be used by any > > >>module that is aware of environment variables, as in: > > > > > > > > >So what is the point in posting patches for review if you don't actually > > >wait for anyone to review them before committing? > > > > That would be my fault. We're here at ApacheCon and when Martin said > > he posted to the list first I asked him why he didn't commit to trunk > > directly, since that is C-T-R. It's a reasonable smallish patch, with > > little impact on existing functionality; hence the nudge. > > C-T-R is a good way to accumulate a codebase full of unfinished changes > if the R bit is ignored by the committer. Ping Martin. > > http://mail-archives.apache.org/mod_mbox/httpd-dev/200507.mbox/[EMAIL > PROTECTED] > http://mail-archives.apache.org/mod_mbox/httpd-dev/200507.mbox/[EMAIL > PROTECTED] > http://mail-archives.apache.org/mod_mbox/httpd-dev/200507.mbox/[EMAIL > PROTECTED] Oops, sorry. Thanks for pinging. >> 1) this is a pretty specific to way to code it. Is there no way to make >> it more general so that OID() is just a function like file() and can be >> used e.g. in regex matches too? The problem with the OID() "function" is that it where file() (or another file() like function) return a single value, what OID() stands for is an "array of zero or more values". But there is no syntax to deal with arrays in place of expressions. I tried to implement it as function first, but noticed that it would break when an OID was specified more than once. In the ASF scenario, the intention is to add multiple extensions with this OID, each one containing as value a group name of which the client is member. Because of the pre-existing syntax " in {value,value}", and because "{value,value}" is effectively an array, I chose to implement the OID() "function" as a special case of the " in" operator. Do you have a good idea how to use a function-like syntax, and still maintain the "is an array" property? >> 2) you must always check in the regenerated generated scanner source >> along with changes to the lex file. My bad, sorry for missing that. Committed right now. >> 3) oid() is a terrible name for this; it's simply the type of the >> parameter. It would be like calling malloc() "size()". The function >> expands (conceptually) to the values of an extension in the peer's >> certificate, identified by OID; so call it peerext() or something >> meaningful like that. Good point - Thanks a lot -- that is a *very* good idea, and (if nobody objects) I'd want to follow this suggestion. I had been a little unhappy with OID() myself. peerext() is especially good because it also documents where the certificate came from. >> > SetEnvIf OID("2.16.840.1.113730.1.13") "(.*) Generated (Certificate)" >> > ca=$1 >> >> -1 on the naming since OID is completely entirely meaningless in this >> context. In the context of mod_setenvif, I'd even use "SSLPeerExt()" because it makes it clear that we are dealing with an SSL-related thing. Patch <> attached. In <> there is a patch which changes OID() to SSLPeerExt() for mod_ssl. Martin -- <[EMAIL PROTECTED]> | Fujitsu Siemens Fon: +49-89-636-46021, FAX: +49-89-636-48332 | 81730 Munich, Germany Index: ssl_expr.h === --- ssl_expr.h (Revision 226989) +++ ssl_expr.h (Arbeitskopie) @@ -61,7 +61,7 @@ #endif typedef enum { -op_NOP, op_ListElement, op_OidListElement, +op_NOP, op_ListElement, op_PeerExtElement, op_True, op_False, op_Not, op_Or, op_And, op_Comp, op_EQ, op_NE, op_LT, op_LE, op_GT, op_GE, op_IN, op_REG, op_NRE, op_Digit, op_String, op_Regex, op_Var, op_Func Index: ssl_expr_eval.c === --- ssl_expr_eval.c (Revision 226989) +++ ssl_expr_eval.c (Arbeitskopie) @@ -118,7 +118,7 @@ e3 = (ssl_expr *)e2->node_arg1; e2 = (ssl_expr *)e2->node_arg2; -if (op == op_OidListElement) { +if (op == op_PeerExtElement) { char *w3 = ssl_expr_eval_word(r, e3); found = ssl_expr_eval_oid(r, w1, w3); @@ -204,7 +204,7 @@ { int count = 0, j; X509 *xs = NULL; -ASN1_OBJECT *oid; +ASN1_OBJECT *peerext; apr_array_header_t *val_array; SSLConnRec *sslconn = myConnConfig(r->connection); @@ -212,8 +212,8 @@ if (oidstr == NULL || sslconn == NULL || sslconn->ssl == NUL
Re: svn commit: r220307 - in /httpd/httpd/trunk/modules: metadata/mod_setenvif.c ssl/mod_ssl.c ssl/mod_ssl.h ssl/ssl_expr_eval.c
On Fri, Jul 22, 2005 at 02:24:50PM +0200, Sander Striker wrote: > Joe Orton wrote: > >On Fri, Jul 22, 2005 at 12:11:56PM -, Martin Kraemer wrote: > > > >>Author: martin > >>Date: Fri Jul 22 05:11:55 2005 > >>New Revision: 220307 > >> > >>URL: http://svn.apache.org/viewcvs?rev=220307&view=rev > >>Log: > >>Allow extraction of the values of SSL certificate extensions into > >>environment variables, so that their value can be used by any > >>module that is aware of environment variables, as in: > > > > > >So what is the point in posting patches for review if you don't actually > >wait for anyone to review them before committing? > > That would be my fault. We're here at ApacheCon and when Martin said > he posted to the list first I asked him why he didn't commit to trunk > directly, since that is C-T-R. It's a reasonable smallish patch, with > little impact on existing functionality; hence the nudge. C-T-R is a good way to accumulate a codebase full of unfinished changes if the R bit is ignored by the committer. Ping Martin. http://mail-archives.apache.org/mod_mbox/httpd-dev/200507.mbox/[EMAIL PROTECTED] http://mail-archives.apache.org/mod_mbox/httpd-dev/200507.mbox/[EMAIL PROTECTED] http://mail-archives.apache.org/mod_mbox/httpd-dev/200507.mbox/[EMAIL PROTECTED] joe
Re: svn commit: r220307 - in /httpd/httpd/trunk/modules: metadata/mod_setenvif.c ssl/mod_ssl.c ssl/mod_ssl.h ssl/ssl_expr_eval.c
Joe Orton wrote: On Fri, Jul 22, 2005 at 12:11:56PM -, Martin Kraemer wrote: Author: martin Date: Fri Jul 22 05:11:55 2005 New Revision: 220307 URL: http://svn.apache.org/viewcvs?rev=220307&view=rev Log: Allow extraction of the values of SSL certificate extensions into environment variables, so that their value can be used by any module that is aware of environment variables, as in: So what is the point in posting patches for review if you don't actually wait for anyone to review them before committing? That would be my fault. We're here at ApacheCon and when Martin said he posted to the list first I asked him why he didn't commit to trunk directly, since that is C-T-R. It's a reasonable smallish patch, with little impact on existing functionality; hence the nudge. Sander
Re: svn commit: r220307 - in /httpd/httpd/trunk/modules: metadata/mod_setenvif.c ssl/mod_ssl.c ssl/mod_ssl.h ssl/ssl_expr_eval.c
On Fri, Jul 22, 2005 at 12:11:56PM -, Martin Kraemer wrote: > Author: martin > Date: Fri Jul 22 05:11:55 2005 > New Revision: 220307 > > URL: http://svn.apache.org/viewcvs?rev=220307&view=rev > Log: > Allow extraction of the values of SSL certificate extensions into > environment variables, so that their value can be used by any > module that is aware of environment variables, as in: So what is the point in posting patches for review if you don't actually wait for anyone to review them before committing? > SetEnvIf OID("2.16.840.1.113730.1.13") "(.*) Generated (Certificate)" ca=$1 -1 on the naming since OID is completely entirely meaningless in this context. mod_setenvif.c: > module AP_MODULE_DECLARE_DATA setenvif_module; > +#if (MODULE_MAGIC_NUMBER_MAJOR > 20020903) > +#include "mod_ssl.h" unnecessary for trunk code to care about the MMN, it can always rely on mod_ssl.h being available. mod_ssl.h: > +extern apr_array_header_t *ssl_extlist_by_oid(request_rec *r, const char > *oidstr); and don't export the function as well as the optional function.