Yes :) Very close :) To simplify the code I've also changed xmlSecOpenSSLX509_NAME_cmp() function to use xmlSecOpenSSLX509_NAME_ENTRY_cmp(). The patch is attached and it should be in CVS in about an hour.
Your test witht this patch prints the following (I removed some printfs for simplicity):
[EMAIL PROTECTED] openssl]$ ./x509vfytest
xmlSecOpenSSLX509NamesCompare(): sorting a1 entries ...
xmlSecOpenSSLX509NamesCompare(): sorting b1 entries ...
xmlSecOpenSSLX509NamesCompare(): a1(buf)=/OU=test_certificate1/OU=test_certificate2/OU=test_certificate3
xmlSecOpenSSLX509NamesCompare(): b1(buf)=/OU=test_certificate1/OU=test_certificate2/OU=test_certificate3
test A4.1: return 0
xmlSecOpenSSLX509NamesCompare(): sorting a1 entries ...
xmlSecOpenSSLX509NamesCompare(): sorting b1 entries ...
xmlSecOpenSSLX509NamesCompare(): a1(buf)=/OU=test_certificate1/OU=test_certificate2/OU=test_certificate3
xmlSecOpenSSLX509NamesCompare(): b1(buf)=/OU=test_certificate1/OU=test_certificate2/OU=test_certificate2
test A4.2: return 1
xmlSecOpenSSLX509NamesCompare(): sorting a1 entries ...
xmlSecOpenSSLX509NamesCompare(): sorting b1 entries ...
xmlSecOpenSSLX509NamesCompare(): a1(buf)=/OU=test_certificate1/OU=test_certificate2/OU=test_certificate3
xmlSecOpenSSLX509NamesCompare(): b1(buf)=/O=test_certificate2/OU=test_certificate2/OU=test_certificate3
test A4.3: return -1
This seems correct to me.
Thanks again for bug report and the test!
Aleksey
Roumen Petrov wrote:
Might source similar to next lines: =====================================
Index: src/openssl/x509vfy.c
===================================================================
RCS file: /cvs/gnome/xmlsec/src/openssl/x509vfy.c,v
retrieving revision 1.15
diff -u -r1.15 x509vfy.c
--- src/openssl/x509vfy.c 20 Apr 2003 22:24:42 -0000 1.15
+++ src/openssl/x509vfy.c 2 Jul 2003 15:46:23 -0000
@@ -956,41 +956,29 @@
}
static
-int xmlSecOpenSSLX509_NAME_cmp(const X509_NAME *a, const X509_NAME *b)
- {
- int i,j;
- X509_NAME_ENTRY *na,*nb;
+int xmlSecOpenSSLX509_NAME_cmp(const X509_NAME *a, const X509_NAME *b) {
+ int i,ret;
+ X509_NAME_ENTRY *na,*nb;
- xmlSecAssert2(a != NULL, -1);
- xmlSecAssert2(b != NULL, 1);
+ xmlSecAssert2(a != NULL, -1);
+ xmlSecAssert2(b != NULL, 1);
- if (sk_X509_NAME_ENTRY_num(a->entries)
- != sk_X509_NAME_ENTRY_num(b->entries))
- return sk_X509_NAME_ENTRY_num(a->entries)
- -sk_X509_NAME_ENTRY_num(b->entries);
- for (i=sk_X509_NAME_ENTRY_num(a->entries)-1; i>=0; i--)
- {
- na=sk_X509_NAME_ENTRY_value(a->entries,i);
- nb=sk_X509_NAME_ENTRY_value(b->entries,i);
- j=na->value->length-nb->value->length;
- if (j) return(j);
- j=memcmp(na->value->data,nb->value->data,
- na->value->length);
- if (j) return(j);
- }
-
- /* We will check the object types after checking the values
- * since the values will more often be different than the object
- * types. */
- for (i=sk_X509_NAME_ENTRY_num(a->entries)-1; i>=0; i--)
- {
- na=sk_X509_NAME_ENTRY_value(a->entries,i);
- nb=sk_X509_NAME_ENTRY_value(b->entries,i);
- j=OBJ_cmp(na->object,nb->object);
- if (j) return(j);
- }
- return(0);
+ if (sk_X509_NAME_ENTRY_num(a->entries) != sk_X509_NAME_ENTRY_num(b->entries)) {
+ return sk_X509_NAME_ENTRY_num(a->entries) - sk_X509_NAME_ENTRY_num(b->entries);
+ }
+
+ for (i=sk_X509_NAME_ENTRY_num(a->entries)-1; i>=0; i--) {
+ na=sk_X509_NAME_ENTRY_value(a->entries,i);
+ nb=sk_X509_NAME_ENTRY_value(b->entries,i);
+
+ ret = xmlSecOpenSSLX509_NAME_ENTRY_cmp(&na, &nb);
+ if(ret != 0) {
+ return(ret);
}
+ }
+
+ return(0);
+}
/**
@@ -1048,9 +1036,33 @@
*/
static int
xmlSecOpenSSLX509_NAME_ENTRY_cmp(const X509_NAME_ENTRY **a, const X509_NAME_ENTRY
**b) {
+ int ret;
+
xmlSecAssert2(a != NULL, -1);
xmlSecAssert2(b != NULL, 1);
+ xmlSecAssert2((*a) != NULL, -1);
+ xmlSecAssert2((*b) != NULL, 1);
+
+ /* first compare values */
+ if(((*a)->value == NULL) && ((*b)->value != NULL)) {
+ return(-1);
+ } else if(((*a)->value != NULL) && ((*b)->value == NULL)) {
+ return(1);
+ } else if(((*a)->value == NULL) && ((*b)->value == NULL)) {
+ return(0);
+ }
+
+ ret = (*a)->value->length - (*b)->value->length;
+ if(ret != 0) {
+ return(ret);
+ }
+
+ ret = memcmp((*a)->value->data, (*b)->value->data, (*a)->value->length);
+ if(ret != 0) {
+ return(ret);
+ }
+ /* next compare names */
return(OBJ_cmp((*a)->object, (*b)->object));
}
