Bugs found (and fixed) in certificateExactmatch code.

[Mark Ruijter <openldap@siennax.com>]

The certificateExactIndexer routine is not called when a userCertifcate
is modified.
This meens that you won't be able to find an entry with
certificateExactMatch after modification.

When you remove the whole DN and reinsert it then the
certificateExactIndexer is triggered and everything works fine.

I've been using the ldbm backend and the problem here is found in
index.c
For now we use the following patch:
--- openldap-2.1.22/servers/slapd/back-ldbm/index.c     Sun Feb  9
17:31:38 2003
+++ /usr/local/src/openldap-2.1.22/servers/slapd/back-ldbm/index.c
Sun Sep  7 01:18:40 2003
@@ -72,6 +72,16 @@
        char *dbname;
        struct berval prefix;

+        if ( desc->ad_type->sat_equality != NULL )
+            if ( 0 ==
strcmp("2.5.4.36",desc->ad_type->sat_atype.at_oid))
+               if ( 0 ==
strcmp("1.2.826.0.1.3344810.7.1",desc->ad_type->sat_equality->smr_syntax->ssyn_syn.syn_oid)
)
+                          return LDAP_SUCCESS;
        mask = index_mask( be, desc, &dbname, &prefix );

        if( mask == 0 ) {

Question: Although this works fine the fix that we really want is
somewhere in avl_find? Or is this ok?
TODO: Didn't check the back-bdb code.


One other problem exists in the certificateExactIndexer routine. It
leaks keys when something else then a real certificate is entered in a
userCertificate field.
The fix for this problem is to add code to check before adding keys.

        /* we should have at least one value at this point */
        assert( values != NULL && values[0].bv_val != NULL );

        for( i=0; values[i].bv_val != NULL; i++ ) {
-               /* empty -- just count them */
+               p = values[i].bv_val;
+                xcert = d2i_X509(NULL, &p, values[i].bv_len);
+                if ( !xcert ) {
+#ifdef NEW_LOGGING
+                        LDAP_LOG( CONFIG, ENTRY,
+                                "certificateExactIndexer: error parsing
cert: %s\n",
+                                ERR_error_string(ERR_get_error(),NULL),
0, 0);
+#else
+                        Debug( LDAP_DEBUG_ARGS,
"certificateExactIndexer: "
+                               "error parsing cert: %s\n",
+                               ERR_error_string(ERR_get_error(),NULL),
+                               NULL, NULL );
+#endif
+                       *keysp = NULL;
+                        return LDAP_INVALID_SYNTAX;
+                }
+               X509_free(xcert);
        }

        keys = ch_malloc( sizeof( struct berval ) * (i+1) );

Even better would be to also add a check on the certificate when it's
inserted in ldap:

+static int
+       certValidate(
+       Syntax *syntax,
+       struct berval *in )
+{
+#ifdef CERT_SYNCHECK
+       X509 *xcert;
+       unsigned char *p = in->bv_val;
+       struct berval serial;
+       struct berval issuer_dn;
+
+       xcert = d2i_X509(NULL, &p, in->bv_len);
+       if ( !xcert ) return LDAP_INVALID_SYNTAX;
+       if ( !asn1_integer2str(xcert->cert_info->serialNumber, &serial)
) {
+               X509_free(xcert);
+               return LDAP_INVALID_SYNTAX;
+       }
+       X509_free(xcert);
+       ber_memfree(serial.bv_val);
+#endif
+       return LDAP_SUCCESS;
+}
+#else
+static int
+       certValidate(
+       Syntax *syntax,
+struct berval *in )
+{
+       return LDAP_SUCCESS;
+}
 #endif

 static int
@@ -4345,7 +4431,7 @@
                0, booleanValidate, NULL, NULL},
        {"( 1.3.6.1.4.1.1466.115.121.1.8 DESC 'Certificate' "
                X_BINARY X_NOT_H_R ")",
-               SLAP_SYNTAX_BINARY|SLAP_SYNTAX_BER, berValidate, NULL,
NULL},
+               SLAP_SYNTAX_BINARY|SLAP_SYNTAX_BER, certValidate, NULL,
NULL},
        {"( 1.3.6.1.4.1.1466.115.121.1.9 DESC 'Certificate List' "
                X_BINARY X_NOT_H_R ")",
                SLAP_SYNTAX_BINARY|SLAP_SYNTAX_BER, berValidate, NULL,
NULL};

Question: I would have preferred to check if certificateExactMatch is
enabled in slapd.conf. However the needed structure isn't there.
Adding it would meen changing all the other indexing routines as well.
Do we want this? Or do we stick with the define CERT_SYNCHECK?

Do I submit the patch or do I have to change anything?

Thanks in advance,

Mark Ruijter