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/[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 "<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
--
<[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 == NULL)
return NULL;
- /* Determine the oid we are looking for */
- if ((oid = OBJ_txt2obj(oidstr, 1)) == NULL) {
+ /* Determine the peer extension we are looking for */
+ if ((peerext = OBJ_txt2obj(oidstr, 1)) == NULL) {
ERR_clear_error();
return NULL;
}
@@ -230,7 +230,7 @@
for (j = 0; j < count; j++) {
X509_EXTENSION *ext = X509_get_ext(xs, j);
- if (OBJ_cmp(ext->object, oid) == 0) {
+ if (OBJ_cmp(ext->object, peerext) == 0) {
BIO *bio = BIO_new(BIO_s_mem());
if (X509V3_EXT_print(bio, ext, 0, 0) == 1) {
Index: ssl_expr_scan.l
===================================================================
--- ssl_expr_scan.l (Revision 226989)
+++ ssl_expr_scan.l (Arbeitskopie)
@@ -152,39 +152,39 @@
/*
* Operators
*/
-"eq" { return T_OP_EQ; }
+[Ee][Qq] { return T_OP_EQ; }
"==" { return T_OP_EQ; }
-"ne" { return T_OP_NE; }
+[Nn][Ee] { return T_OP_NE; }
"!=" { return T_OP_NE; }
-"lt" { return T_OP_LT; }
+[Ll][Tt] { return T_OP_LT; }
"<" { return T_OP_LT; }
-"le" { return T_OP_LE; }
+[Ll][Ee] { return T_OP_LE; }
"<=" { return T_OP_LE; }
-"gt" { return T_OP_GT; }
+[Gg][Tt] { return T_OP_GT; }
">" { return T_OP_GT; }
-"ge" { return T_OP_GE; }
+[Gg][Ee] { return T_OP_GE; }
">=" { return T_OP_GE; }
"=~" { return T_OP_REG; }
"!~" { return T_OP_NRE; }
-"and" { return T_OP_AND; }
+[Aa][Nn][Dd] { return T_OP_AND; }
"&&" { return T_OP_AND; }
-"or" { return T_OP_OR; }
+[Oo][Rr] { return T_OP_OR; }
"||" { return T_OP_OR; }
-"not" { return T_OP_NOT; }
+[Nn][Oo][Tt] { return T_OP_NOT; }
"!" { return T_OP_NOT; }
-"in" { return T_OP_IN; }
-[Oo][Ii][Dd] { return T_OP_OID; }
+[Ii][Nn] { return T_OP_IN; }
+[Ss][Ss][Ll][Pp][Ee][Ee][Rr][Ee][Xx][Tt] { return T_OP_PEEREXT; }
/*
* Functions
*/
-"file" { return T_FUNC_FILE; }
+[Ff][Ii][Ll][Ee] { return T_FUNC_FILE; }
/*
* Specials
*/
-"true" { return T_TRUE; }
-"false" { return T_FALSE; }
+[Tt][Rr][Uu][Ee] { return T_TRUE; }
+[Ff][Aa][Ll][Ss][Ee] { return T_FALSE; }
/*
* Digits
Index: ssl_expr_parse.y
===================================================================
--- ssl_expr_parse.y (Revision 226989)
+++ ssl_expr_parse.y (Arbeitskopie)
@@ -61,7 +61,7 @@
%token T_OP_REG
%token T_OP_NRE
%token T_OP_IN
-%token T_OP_OID
+%token T_OP_PEEREXT
%token T_OP_OR
%token T_OP_AND
@@ -104,7 +104,7 @@
| word T_OP_NRE regex { $$ = ssl_expr_make(op_NRE, $1, $3);
}
;
-wordlist : T_OP_OID '(' word ')' { $$ =
ssl_expr_make(op_OidListElement, $3, NULL); }
+wordlist : T_OP_PEEREXT '(' word ')' { $$ =
ssl_expr_make(op_PeerExtElement, $3, NULL); }
| '{' words '}' { $$ = $2 ; }
;
Index: mod_setenvif.c
===================================================================
--- mod_setenvif.c (Revision 226913)
+++ mod_setenvif.c (Arbeitskopie)
@@ -104,7 +104,7 @@
SPECIAL_REQUEST_METHOD,
SPECIAL_REQUEST_PROTOCOL,
SPECIAL_SERVER_ADDR,
- SPECIAL_OID_VALUE
+ SPECIAL_SSLPEEREXT_VALUE
};
typedef struct {
char *name; /* header name */
@@ -349,28 +349,28 @@
else if (!strcasecmp(fname, "server_addr")) {
new->special_type = SPECIAL_SERVER_ADDR;
}
- else if (!strncasecmp(fname, "oid(",4)) {
+ else if (!strncasecmp(fname, "SSLPeerExt(",11)) {
ap_regmatch_t match[AP_MAX_REG_MATCH];
- new->special_type = SPECIAL_OID_VALUE;
+ new->special_type = SPECIAL_SSLPEEREXT_VALUE;
- /* Syntax check and extraction of the OID as a regex: */
+ /* Syntax check and extraction of the SSLPeerExt as a regex: */
new->pnamereg = ap_pregcomp(cmd->pool,
- "^oid\\(\"?([0-9.]+)\"?\\)$",
+ "^SSLPeerExt\\(\"?([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);
+ "SSLPeerExt regex could not be
compiled.", NULL);
*/
if (ap_regexec(new->pnamereg, fname, AP_MAX_REG_MATCH, match, 0)
== AP_REG_NOMATCH) {
return apr_pstrcat(cmd->pool, cmd->cmd->name,
- "OID syntax is: oid(\"1.2.3.4.5\");
error in: ",
+ "SSLPeerExt syntax is:
SSLPeerExt(\"1.2.3.4.5\"); error in: ",
fname, NULL);
}
new->pnamereg = NULL;
- /* The name field is used for the stripped oid string */
+ /* The name field is used for the stripped PeerExt string */
new->name = fname = apr_pstrdup(cmd->pool, fname+match[1].rm_so);
fname[match[1].rm_eo - match[1].rm_so] = '\0';
}
@@ -504,7 +504,7 @@
* 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
+ * In the case of SPECIAL_SSLPEEREXT_VALUE values, each PeerExt string
is
* dynamically allocated, thus there are no duplicates.
*/
if (b->name != last_name) {
@@ -529,29 +529,29 @@
case SPECIAL_REQUEST_PROTOCOL:
val = r->protocol;
break;
- case SPECIAL_OID_VALUE:
+ case SPECIAL_SSLPEEREXT_VALUE:
/* If mod_ssl is not loaded, the accessor function is NULL */
if (ssl_extlist_by_oid_func != NULL)
{
- apr_array_header_t *oid_array;
- char **oid_value;
+ apr_array_header_t *peerext_array;
+ char **peerext_value;
int j, len = 0;
char *retval = NULL;
- /* The given oid can occur multiple times. Concatenate the
values */
- if ((oid_array = ssl_extlist_by_oid_func(r, b->name)) !=
NULL) {
- oid_value = (char **) oid_array->elts;
+ /* The given PeerExt can occur multiple times. Concatenate
the values */
+ if ((peerext_array = ssl_extlist_by_oid_func(r, b->name))
!= NULL) {
+ peerext_value = (char **) peerext_array->elts;
/* pass 1: determine the size of the string */
- for (len=j=0; j < oid_array->nelts; j++) {
- len += strlen(oid_value[j]) + 1; /* +1 for ',' or
terminating NIL */
+ for (len=j=0; j < peerext_array->nelts; j++) {
+ len += strlen(peerext_value[j]) + 1; /* +1 for ','
or terminating NIL */
}
retval = apr_palloc(r->pool, len);
/* pass 2: fill the string */
- for (j=0; j < oid_array->nelts; j++) {
+ for (j=0; j < peerext_array->nelts; j++) {
if (j > 0) {
strcat(retval, ",");
}
- strcat(retval, oid_value[j]);
+ strcat(retval, peerext_value[j]);
}
}
val = retval;
pgpRIFXZYkRr0.pgp
Description: PGP signature
