[xmlsec] x509vfy.c:xmlSecOpenSSLX509NamesCompare()

[Aleksey Sanin]

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):

[aleksey at lsh 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:
> =====================================

-------------- next part --------------
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));
 }