[xmlsec] Problem with some cert which has a negative serial number
Aleksey Sanin
aleksey at aleksey.com
Mon Feb 21 20:28:29 PST 2005
I think I have a patch that should fix the problem with negative
serial numbers (see attached). I would appreciate if you can try
it to make sure that it works for you.
Also if you have an example with certificate having negative
serial number, I would appreciate if you can share it so I
can create a test case. I failed to get a negative serial
number from openssl :)
Thanks,
Aleksey
-------------- next part --------------
Index: src/bn.c
===================================================================
RCS file: /cvs/gnome/xmlsec/src/bn.c,v
retrieving revision 1.14
diff -u -w -r1.14 bn.c
--- src/bn.c 26 Jan 2005 16:52:09 -0000 1.14
+++ src/bn.c 22 Feb 2005 04:12:38 -0000
@@ -170,9 +170,10 @@
*/
int
xmlSecBnFromString(xmlSecBnPtr bn, const xmlChar* str, xmlSecSize base) {
- xmlSecSize i, len;
+ xmlSecSize i, len, size;
xmlSecByte ch;
xmlSecByte* data;
+ int positive;
int nn;
int ret;
@@ -192,7 +193,7 @@
* In truth, it would be likely less than 1/2 input string length
* because each byte is represented by 2 chars. If needed,
* buffer size would be increased by Mul/Add functions.
- * Finally, we add one byte for 00 prefix if first byte is > 127.
+ * Finally, we can add one byte for 00 or 10 prefix.
*/
ret = xmlSecBufferSetMaxSize(bn, xmlSecBufferGetSize(bn) + len / 2 + 1 + 1);
if(ret < 0) {
@@ -204,8 +205,49 @@
return (-1);
}
- for(i = 0; i < len; i++) {
- ch = str[i];
+ /* figure out if it is positive or negative number */
+ positive = 1;
+ i = 0;
+ while(i < len) {
+ ch = str[i++];
+
+ /* skip spaces */
+ if(isspace(ch)) {
+ continue;
+ }
+
+ /* check if it is + or - */
+ if(ch == '+') {
+ positive = 1;
+ break;
+ } else if(ch == '-') {
+ positive = 0;
+ break;
+ }
+
+ /* otherwise, it must be start of the number */
+ nn = xmlSecBnLookupTable[ch];
+ if((nn >= 0) && ((xmlSecSize)nn < base)) {
+ xmlSecAssert2(i > 0, -1);
+
+ /* no sign, positive by default */
+ positive = 1;
+ --i; /* make sure that we will look at this character in next loop */
+ break;
+ } else {
+ xmlSecError(XMLSEC_ERRORS_HERE,
+ NULL,
+ NULL,
+ XMLSEC_ERRORS_R_INVALID_DATA,
+ "char=%c;base=%d",
+ ch, base);
+ return (-1);
+ }
+ }
+
+ /* now parse the number itself */
+ while(i < len) {
+ ch = str[i++];
if(isspace(ch)) {
continue;
}
@@ -243,9 +285,10 @@
}
}
- /* check whether need to add 00 prefix */
+ /* check if we need to add 00 prefix */
data = xmlSecBufferGetData(bn);
- if(data[0] > 127) {
+ size = xmlSecBufferGetSize(bn);
+ if(size > 0 && data[0] > 127) {
ch = 0;
ret = xmlSecBufferPrepend(bn, &ch, 1);
if(ret < 0) {
@@ -257,6 +300,26 @@
return (-1);
}
}
+
+ /* do 2's compliment and add 1 to represent negative value */
+ if(positive == 0) {
+ data = xmlSecBufferGetData(bn);
+ size = xmlSecBufferGetSize(bn);
+ for(i = 0; i < size; ++i) {
+ data[i] ^= 0xFF;
+ }
+
+ ret = xmlSecBnAdd(bn, 1);
+ if(ret < 0) {
+ xmlSecError(XMLSEC_ERRORS_HERE,
+ NULL,
+ "xmlSecBnAdd",
+ XMLSEC_ERRORS_R_XMLSEC_FAILED,
+ "base=%d", base);
+ return (-1);
+ }
+ }
+
return(0);
}
@@ -272,8 +335,12 @@
*/
xmlChar*
xmlSecBnToString(xmlSecBnPtr bn, xmlSecSize base) {
+ xmlSecBn bn2;
+ int positive = 1;
xmlChar* res;
- xmlSecSize i, len;
+ xmlSecSize i, len, size;
+ xmlSecByte* data;
+ int ret;
int nn;
xmlChar ch;
@@ -281,12 +348,61 @@
xmlSecAssert2(base > 1, NULL);
xmlSecAssert2(base <= sizeof(xmlSecBnRevLookupTable), NULL);
+
+ /* copy bn */
+ data = xmlSecBufferGetData(bn);
+ size = xmlSecBufferGetSize(bn);
+ ret = xmlSecBnInitialize(&bn2, size);
+ if(ret < 0) {
+ xmlSecError(XMLSEC_ERRORS_HERE,
+ NULL,
+ "xmlSecBnCreate",
+ XMLSEC_ERRORS_R_XMLSEC_FAILED,
+ "size=%d", size);
+ return (NULL);
+ }
+
+ ret = xmlSecBnSetData(&bn2, data, size);
+ if(ret < 0) {
+ xmlSecError(XMLSEC_ERRORS_HERE,
+ NULL,
+ "xmlSecBnSetData",
+ XMLSEC_ERRORS_R_XMLSEC_FAILED,
+ "size=%d", size);
+ xmlSecBnFinalize(&bn2);
+ return (NULL);
+ }
+
+ /* check if it is a negative number or not */
+ data = xmlSecBufferGetData(&bn2);
+ size = xmlSecBufferGetSize(&bn2);
+ if((size > 0) && (data[0] > 127)) {
+ /* subtract 1 and do 2's compliment */
+ ret = xmlSecBnAdd(&bn2, -1);
+ if(ret < 0) {
+ xmlSecError(XMLSEC_ERRORS_HERE,
+ NULL,
+ "xmlSecBnAdd",
+ XMLSEC_ERRORS_R_XMLSEC_FAILED,
+ "size=%d", size);
+ xmlSecBnFinalize(&bn2);
+ return (NULL);
+ }
+ for(i = 0; i < size; ++i) {
+ data[i] ^= 0xFF;
+ }
+
+ positive = 0;
+ } else {
+ positive = 1;
+ }
+
/* Result string len is
* len = log base (256) * <bn size>
* Since the smallest base == 2 then we can get away with
* len = 8 * <bn size>
*/
- len = 8 * xmlSecBufferGetSize(bn) + 1;
+ len = 8 * size + 1 + 1;
res = (xmlChar*)xmlMalloc(len + 1);
if(res == NULL) {
xmlSecError(XMLSEC_ERRORS_HERE,
@@ -294,18 +410,20 @@
NULL,
XMLSEC_ERRORS_R_MALLOC_FAILED,
"len=%d", len);
+ xmlSecBnFinalize(&bn2);
return (NULL);
}
memset(res, 0, len + 1);
- for(i = 0; (xmlSecBufferGetSize(bn) > 0) && (i < len); i++) {
- if(xmlSecBnDiv(bn, base, &nn) < 0) {
+ for(i = 0; (xmlSecBufferGetSize(&bn2) > 0) && (i < len); i++) {
+ if(xmlSecBnDiv(&bn2, base, &nn) < 0) {
xmlSecError(XMLSEC_ERRORS_HERE,
NULL,
"xmlSecBnDiv",
XMLSEC_ERRORS_R_XMLSEC_FAILED,
"base=%d", base);
xmlFree(res);
+ xmlSecBnFinalize(&bn2);
return (NULL);
}
xmlSecAssert2((size_t)nn < sizeof(xmlSecBnRevLookupTable), NULL);
@@ -317,6 +435,12 @@
for(len = i; (len > 1) && (res[len - 1] == '0'); len--);
res[len] = '\0';
+ /* add "-" for negative numbers */
+ if(positive == 0) {
+ res[len] = '-';
+ res[++len] = '\0';
+ }
+
/* swap the string because we wrote it in reverse order */
for(i = 0; i < len / 2; i++) {
ch = res[i];
@@ -324,6 +448,7 @@
res[len - i - 1] = ch;
}
+ xmlSecBnFinalize(&bn2);
return(res);
}
@@ -408,7 +533,9 @@
}
data = xmlSecBufferGetData(bn);
- for(over = 0, i = xmlSecBufferGetSize(bn); i > 0;) {
+ i = xmlSecBufferGetSize(bn);
+ over = 0;
+ while(i > 0) {
xmlSecAssert2(data != NULL, -1);
over = over + multiplier * data[--i];
Index: src/mscrypto/x509vfy.c
===================================================================
RCS file: /cvs/gnome/xmlsec/src/mscrypto/x509vfy.c,v
retrieving revision 1.3
diff -u -w -r1.3 x509vfy.c
--- src/mscrypto/x509vfy.c 27 Sep 2003 03:12:22 -0000 1.3
+++ src/mscrypto/x509vfy.c 22 Feb 2005 04:12:40 -0000
@@ -568,9 +568,28 @@
if((pCert == NULL) && (NULL != issuerName) && (NULL != issuerSerial)) {
xmlSecBn issuerSerialBn;
CERT_NAME_BLOB cnb;
+ CRYPT_INTEGER_BLOB cib;
BYTE *cName = NULL;
DWORD cNameLen = 0;
+
+ /* get issuer name */
+ cName = xmlSecMSCryptoCertStrToName(X509_ASN_ENCODING | PKCS_7_ASN_ENCODING,
+ issuerName,
+ CERT_OID_NAME_STR | CERT_NAME_STR_REVERSE_FLAG,
+ &cNameLen);
+ if(cName == NULL) {
+ xmlSecError(XMLSEC_ERRORS_HERE,
+ NULL,
+ "xmlSecMSCryptoCertStrToName",
+ XMLSEC_ERRORS_R_XMLSEC_FAILED,
+ XMLSEC_ERRORS_NO_MESSAGE);
+ return (NULL);
+ }
+ cnb.pbData = cName;
+ cnb.cbData = cNameLen;
+
+ /* get serial number */
ret = xmlSecBnInitialize(&issuerSerialBn, 0);
if(ret < 0) {
xmlSecError(XMLSEC_ERRORS_HERE,
@@ -578,6 +597,7 @@
"xmlSecBnInitialize",
XMLSEC_ERRORS_R_XMLSEC_FAILED,
XMLSEC_ERRORS_NO_MESSAGE);
+ xmlFree(cName);
return(NULL);
}
@@ -589,25 +609,29 @@
XMLSEC_ERRORS_R_XMLSEC_FAILED,
XMLSEC_ERRORS_NO_MESSAGE);
xmlSecBnFinalize(&issuerSerialBn);
+ xmlFree(cName);
return(NULL);
}
- cName = xmlSecMSCryptoCertStrToName(X509_ASN_ENCODING | PKCS_7_ASN_ENCODING,
- issuerName,
- CERT_OID_NAME_STR | CERT_NAME_STR_REVERSE_FLAG,
- &cNameLen);
- if(cName == NULL) {
+ /* I have no clue why at a sudden a swap is needed to
+ * convert from lsb... This code is purely based upon
+ * trial and error :( WK
+ */
+ ret = xmlSecBnReverse(&issuerSerialBn);
+ if(ret < 0) {
xmlSecError(XMLSEC_ERRORS_HERE,
NULL,
- "xmlSecMSCryptoCertStrToName",
+ "xmlSecBnReverse",
XMLSEC_ERRORS_R_XMLSEC_FAILED,
XMLSEC_ERRORS_NO_MESSAGE);
xmlSecBnFinalize(&issuerSerialBn);
+ xmlFree(cName);
return (NULL);
}
- cnb.pbData = cName;
- cnb.cbData = cNameLen;
+ cib.pbData = xmlSecBufferGetData(&issuerSerialBn);
+ cib.cbData = xmlSecBufferGetSize(&issuerSerialBn);
+
while((pCert = CertFindCertificateInStore(store,
PKCS_7_ASN_ENCODING | X509_ASN_ENCODING,
0,
@@ -622,9 +646,8 @@
if((pCert->pCertInfo != NULL) &&
(pCert->pCertInfo->SerialNumber.pbData != NULL) &&
(pCert->pCertInfo->SerialNumber.cbData > 0) &&
- (0 == xmlSecBnCompareReverse(&issuerSerialBn, pCert->pCertInfo->SerialNumber.pbData,
- pCert->pCertInfo->SerialNumber.cbData))) {
-
+ (CertCompareIntegerBlob(&(pCert->pCertInfo->SerialNumber), &cib) == TRUE)
+ ) {
break;
}
}
More information about the xmlsec
mailing list