[xmlsec] Failure to encode special chars in X509SubjectName node
Aleksey Sanin
aleksey at aleksey.com
Wed Jun 4 08:47:28 PDT 2008
Hi, Cliff!
Sorry for delay with the response. Somehow I lost few items from
my "todo" list and this was one of them.
You are right and this is indeed a real problem. I fixed it in
all the places you've mentioned and similar places in other
crypto libraries (nss, mscrypto). The change is checked in but
I also attached the patch in case you want to try it directly.
Thank you for the bug report!
Aleksey
Cliff Hones wrote:
> I have an X509 certificate which has an ampersand within its
> "Subject" text. When signing with this certificate, the content
> of the X509SubjectName node is incorrectly set - it terminates
> at the ampersand (which is not encoded as &). Also, xmllib
> reports "unterminated entity reference".
>
> I can fix this behaviour by adding a suitable call to the routine
> xmlEncodeSpecialChars in openssl/x509.c in the function
> xmlSecOpenSSLX509SubjectNameNodeWrite
>
> Note that xmlEncodeSpecialChars requires a "doc" as first argument,
> which is not available in this routine, but in fact NULL can be
> passed as the doc argument is not used.
>
> I think this call should also be added to
> xmlSecOpenSSLX509IssuerSerialNodeWrite
> for the IssuerName node, as this could also contain text with
> an "&" (or indeed other special XML characters).
>
> This problem could also be present in other places where xmlsec sets
> node content to a raw string sourced from non-XML. I haven't looked
> to see if there are any other such occurrences.
>
> Do you consider this a bug? Should I submit it to the Gnome bugzilla?
>
-------------- next part --------------
Index: src/templates.c
===================================================================
--- src/templates.c (revision 988)
+++ src/templates.c (working copy)
@@ -1221,7 +1221,7 @@
return(NULL);
}
if(name != NULL) {
- xmlNodeSetContent(res, name);
+ xmlSecNodeEncodeAndSetContent(res, name);
}
return(res);
}
@@ -1518,7 +1518,7 @@
}
if (issuerName != NULL) {
- xmlNodeSetContent(res, issuerName);
+ xmlSecNodeEncodeAndSetContent(res, issuerName);
}
return(res);
}
@@ -1561,7 +1561,7 @@
}
if (serial != NULL) {
- xmlNodeSetContent(res, serial);
+ xmlSecNodeEncodeAndSetContent(res, serial);
}
return(res);
}
@@ -1960,7 +1960,7 @@
return(-1);
}
- xmlNodeSetContent(xpathNode, expression);
+ xmlSecNodeEncodeAndSetContent(xpathNode, expression);
return((nsList != NULL) ? xmlSecTmplNodeWriteNsList(xpathNode, nsList) : 0);
}
@@ -1998,7 +1998,7 @@
}
xmlSetProp(xpathNode, xmlSecAttrFilter, type);
- xmlNodeSetContent(xpathNode, expression);
+ xmlSecNodeEncodeAndSetContent(xpathNode, expression);
return((nsList != NULL) ? xmlSecTmplNodeWriteNsList(xpathNode, nsList) : 0);
}
@@ -2044,7 +2044,7 @@
}
- xmlNodeSetContent(xpointerNode, expression);
+ xmlSecNodeEncodeAndSetContent(xpointerNode, expression);
return((nsList != NULL) ? xmlSecTmplNodeWriteNsList(xpointerNode, nsList) : 0);
}
Index: src/keyinfo.c
===================================================================
--- src/keyinfo.c (revision 988)
+++ src/keyinfo.c (working copy)
@@ -779,8 +779,7 @@
name = xmlSecKeyGetName(key);
if(name != NULL) {
- /* TODO: encode the key name */
- xmlNodeSetContent(node, name);
+ xmlSecNodeEncodeAndSetContent(node, name);
}
return(0);
}
Index: src/xmltree.c
===================================================================
--- src/xmltree.c (revision 999)
+++ src/xmltree.c (working copy)
@@ -598,6 +598,43 @@
}
/**
+ * xmlSecNodeEncodeAndSetContent:
+ * @node: the pointer to an XML node.
+ * @buffer: the pointer to the node content.
+ *
+ * Encodes "special" characters in the @buffer and sets the result
+ * as the node content.
+ *
+ * Returns 0 on success or a negative value if an error occurs.
+ */
+int
+xmlSecNodeEncodeAndSetContent(xmlNodePtr node, const xmlChar * buffer) {
+ xmlSecAssert2(node != NULL, -1);
+ xmlSecAssert2(node->doc != NULL, -1);
+
+ if(buffer != NULL) {
+ xmlChar * tmp;
+
+ tmp = xmlEncodeSpecialChars(node->doc, buffer);
+ if (tmp == NULL) {
+ xmlSecError(XMLSEC_ERRORS_HERE,
+ NULL,
+ "xmlEncodeSpecialChars",
+ XMLSEC_ERRORS_R_XML_FAILED,
+ "Failed to encode special characters");
+ return(-1);
+ }
+
+ xmlNodeSetContent(node, tmp);
+ xmlFree(tmp);
+ } else {
+ xmlNodeSetContent(node, NULL);
+ }
+
+ return(0);
+}
+
+/**
* xmlSecAddIDs:
* @doc: the pointer to an XML document.
* @cur: the pointer to an XML node.
Index: src/mscrypto/x509.c
===================================================================
--- src/mscrypto/x509.c (revision 988)
+++ src/mscrypto/x509.c (working copy)
@@ -1165,7 +1165,7 @@
xmlFree(buf);
return(-1);
}
- xmlNodeSetContent(cur, buf);
+ xmlSecNodeEncodeAndSetContent(cur, buf);
xmlFree(buf);
return(0);
}
@@ -1358,7 +1358,7 @@
XMLSEC_ERRORS_NO_MESSAGE);
return(-1);
}
- xmlNodeSetContent(issuerNameNode, buf);
+ xmlSecNodeEncodeAndSetContent(issuerNameNode, buf);
xmlFree(buf);
ret = xmlSecMSCryptoASN1IntegerWrite(issuerNumberNode, &(cert->pCertInfo->SerialNumber));
@@ -1473,7 +1473,7 @@
xmlFree(buf);
return(-1);
}
- xmlNodeSetContent(cur, buf);
+ xmlSecNodeEncodeAndSetContent(cur, buf);
xmlFree(buf);
return(0);
Index: src/openssl/x509.c
===================================================================
--- src/openssl/x509.c (revision 988)
+++ src/openssl/x509.c (working copy)
@@ -1154,7 +1154,7 @@
xmlFree(buf);
return(-1);
}
- xmlNodeSetContent(cur, buf);
+ xmlSecNodeEncodeAndSetContent(cur, buf);
xmlFree(buf);
return(0);
}
@@ -1357,7 +1357,7 @@
XMLSEC_ERRORS_NO_MESSAGE);
return(-1);
}
- xmlNodeSetContent(issuerNameNode, buf);
+ xmlSecNodeEncodeAndSetContent(issuerNameNode, buf);
xmlFree(buf);
buf = xmlSecOpenSSLASN1IntegerWrite(X509_get_serialNumber(cert));
@@ -1369,7 +1369,7 @@
XMLSEC_ERRORS_NO_MESSAGE);
return(-1);
}
- xmlNodeSetContent(issuerNumberNode, buf);
+ xmlSecNodeEncodeAndSetContent(issuerNumberNode, buf);
xmlFree(buf);
return(0);
@@ -1488,7 +1488,7 @@
xmlFree(buf);
return(-1);
}
- xmlNodeSetContent(cur, buf);
+ xmlSecNodeEncodeAndSetContent(cur, buf);
xmlFree(buf);
return(0);
Index: src/nss/x509.c
===================================================================
--- src/nss/x509.c (revision 988)
+++ src/nss/x509.c (working copy)
@@ -1196,7 +1196,7 @@
xmlFree(buf);
return(-1);
}
- xmlNodeSetContent(cur, buf);
+ xmlSecNodeEncodeAndSetContent(cur, buf);
xmlFree(buf);
return(0);
}
@@ -1386,7 +1386,7 @@
XMLSEC_ERRORS_NO_MESSAGE);
return(-1);
}
- xmlNodeSetContent(issuerNameNode, buf);
+ xmlSecNodeEncodeAndSetContent(issuerNameNode, buf);
xmlFree(buf);
buf = xmlSecNssASN1IntegerWrite(&(cert->serialNumber));
@@ -1504,7 +1504,7 @@
xmlFree(buf);
return(-1);
}
- xmlNodeSetContent(cur, buf);
+ xmlSecNodeEncodeAndSetContent(cur, buf);
xmlFree(buf);
return(0);
Index: include/xmlsec/xmltree.h
===================================================================
--- include/xmlsec/xmltree.h (revision 999)
+++ include/xmlsec/xmltree.h (working copy)
@@ -74,7 +74,9 @@
const xmlSecByte *buffer,
xmlSecSize size,
xmlNodePtr* replaced);
-
+XMLSEC_EXPORT int xmlSecNodeEncodeAndSetContent
+ (xmlNodePtr node,
+ const xmlChar *buffer);
XMLSEC_EXPORT void xmlSecAddIDs (xmlDocPtr doc,
xmlNodePtr cur,
const xmlChar** ids);
Index: win32/mycfg.bat
===================================================================
Cannot display: file marked as a binary type.
svn:mime-type = application/x-msdos-program
More information about the xmlsec
mailing list