You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Joe Orton <jo...@redhat.com> on 2005/08/02 13:00:24 UTC

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 -0000, 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/%3c20050722101207.GB17365@redhat.com%3e
http://mail-archives.apache.org/mod_mbox/httpd-dev/200507.mbox/%3c20050722110229.GA20303@redhat.com%3e
http://mail-archives.apache.org/mod_mbox/httpd-dev/200507.mbox/%3c20050722121821.GB19040@redhat.com%3e

joe

Re: [PATCH] Mixed-cased SSLRequire operators in mod_ssl ?

Posted by David Reid <da...@jetnet.co.uk>.
Joe Orton wrote:
> On Mon, Sep 19, 2005 at 11:40:24AM +0200, Martin Kraemer wrote:
> 
>>On Tue, Aug 02, 2005 at 07:14:10PM +0200, Martin Kraemer wrote:
>>
>>>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?
>>
>>I saw no discussion about this question. What does everybody think --
>>should I apply something like the appended patch to allow for a more
>>liberal syntax, e.g. using "AND", "oR" or "In" in the example
> 
> 
> I'm pretty much indifferent, lacking a strong motivation to change it 
> I'd leave it as-is...

While I can see this making writing the config easier, I'm not sure it's
something we should change... -0 from me as well.

david

Re: [PATCH] Mixed-cased SSLRequire operators in mod_ssl ?

Posted by Joe Orton <jo...@redhat.com>.
On Mon, Sep 19, 2005 at 11:40:24AM +0200, Martin Kraemer wrote:
> On Tue, Aug 02, 2005 at 07:14:10PM +0200, Martin Kraemer wrote:
> > 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?
> 
> I saw no discussion about this question. What does everybody think --
> should I apply something like the appended patch to allow for a more
> liberal syntax, e.g. using "AND", "oR" or "In" in the example

I'm pretty much indifferent, lacking a strong motivation to change it 
I'd leave it as-is...

Regards,

joe

[PATCH] Mixed-cased SSLRequire operators in mod_ssl ?

Posted by Martin Kraemer <ma...@apache.org>.
On Tue, Aug 02, 2005 at 07:14:10PM +0200, Martin Kraemer wrote:
> 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?

I saw no discussion about this question. What does everybody think --
should I apply something like the appended patch to allow for a more
liberal syntax, e.g. using "AND", "oR" or "In" in the example
directive from the docs:
  SSLRequire (    %{SSL_CIPHER} !~ m/^(EXP|NULL)-/ \
            and %{SSL_CLIENT_S_DN_O} eq "Snake Oil, Ltd." \
            and %{SSL_CLIENT_S_DN_OU} in {"Staff", "CA", "Dev"} \
            and %{TIME_WDAY} >= 1 and %{TIME_WDAY} <= 5 \
            and %{TIME_HOUR} >= 8 and %{TIME_HOUR} <= 20       ) \
           or %{REMOTE_ADDR} =~ m/^192\.76\.162\.[0-9]+$/

WDYT?
  Martin
-- 
<Ma...@Fujitsu-Siemens.com>         |     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

Posted by Joe Orton <jo...@redhat.com>.
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
     </Directory>
+
+    <Directory @SERVERROOT@/htdocs/modules/setenvif/ssl>
+        Options +Includes
+        AllowOverride All
+    </Directory>
 </IfModule>
 
 ##
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:<!--#echo var="NETSCAPE_COMMENT"-->

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

Posted by David Reid <da...@jetnet.co.uk>.
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

Posted by Joe Orton <jo...@redhat.com>.
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

Posted by Ben Laurie <be...@algroup.co.uk>.
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

Posted by David Reid <da...@jetnet.co.uk>.
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

Posted by Mads Toftum <ma...@toftum.dk>.
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 </peanutgallery>

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

Posted by Joe Orton <jo...@redhat.com>.
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

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

Posted by Martin Kraemer <ma...@apache.org>.
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
-- 
<Ma...@Fujitsu-Siemens.com>         |     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

Posted by Martin Kraemer <ma...@apache.org>.
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 "<expr> in {value,value}", and
> > because "{value,value}" is effectively an array, I chose to implement
> > the OID() "function" as a special case of the "<expr> 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
-- 
<Ma...@Fujitsu-Siemens.com>         |     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

Posted by Joe Orton <jo...@redhat.com>.
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 "<expr> in {value,value}", and
> because "{value,value}" is effectively an array, I chose to implement
> the OID() "function" as a special case of the "<expr> 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 <<mod_setenvif.c.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 <<ssl_peerext.patch>> 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

Posted by Martin Kraemer <ma...@apache.org>.
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 -0000, 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/%3c20050722101207.GB17365@redhat.com%3e
> http://mail-archives.apache.org/mod_mbox/httpd-dev/200507.mbox/%3c20050722110229.GA20303@redhat.com%3e
> http://mail-archives.apache.org/mod_mbox/httpd-dev/200507.mbox/%3c20050722121821.GB19040@redhat.com%3e

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 "<expr> in {value,value}", and
because "{value,value}" is effectively an array, I chose to implement
the OID() "function" as a special case of the "<expr> 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 <<mod_setenvif.c.patch>> attached.

In <<ssl_peerext.patch>> there is a patch which changes OID()
to SSLPeerExt() for mod_ssl.

  Martin
-- 
<Ma...@Fujitsu-Siemens.com>         |     Fujitsu Siemens
Fon: +49-89-636-46021, FAX: +49-89-636-48332 | 81730  Munich,  Germany