[xmlsec] Problem with some cert which has a negative serial number
Michael Mi
Hao.Mi at Sun.COM
Mon Feb 21 21:29:35 PST 2005
I'd love to try it.
But can you tell me how to merge the diff into my bn.c when I am using
the libxmlsec tarball?
Michael
Aleksey Sanin wrote:
> 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
>
>------------------------------------------------------------------------
>
>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;
> }
> }
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://www.aleksey.com/pipermail/xmlsec/attachments/20050222/126a4b88/attachment-0002.htm
More information about the xmlsec
mailing list