[xmlsec] nss / XMLSEC_KEYINFO_FLAGS_X509DATA_DONT_VERIFY_CERTS handling

[Miklos Vajna]

Hi,

As a follow-up up to <https://github.com/lsh123/xmlsec/pull/73>, I was
testing how XMLSEC_KEYINFO_FLAGS_X509DATA_DONT_VERIFY_CERTS is working
in the nss backend.

As far as I understand, this flag is supposed to be similar to 'curl -k'
or 'wget -k', i.e. it should only cause the certificate verification to
be disabled, everything else should be the same.

So here is a small script I made for testing:

----
cd $HOME/git/xmlsec/tests/aleksey-xmldsig-01
export LD_LIBRARY_PATH=$HOME/git/xmlsec/src/nss/.libs
$HOME/git/xmlsec/apps/xmlsec1 verify  --crypto nss --crypto-config /tmp/xmlsec-crypto-config --enabled-key-data x509 enveloping-sha256-rsa-sha256-verify.xml
----

This should (and does) fail currently, as the certificate is not trusted
(no --trusted option is used).

However, I would expect it to succeed in case I patch the xmlsec test
app like this:

----
diff --git a/apps/xmlsec.c b/apps/xmlsec.c
index ea49cb6..ba812af 100644
--- a/apps/xmlsec.c
+++ b/apps/xmlsec.c
@@ -1822,6 +1822,7 @@ xmlSecAppPrepareKeyInfoReadCtx(xmlSecKeyInfoCtxPtr keyInfoCtx) {
     if(xmlSecAppCmdLineParamIsSet(&X509SkipStrictChecksParam)) {
         keyInfoCtx->flags |= XMLSEC_KEYINFO_FLAGS_X509DATA_SKIP_STRICT_CHECKS;
     }
+    keyInfoCtx->flags |= XMLSEC_KEYINFO_FLAGS_X509DATA_DONT_VERIFY_CERTS;
 #endif /* XMLSEC_NO_X509 */
 
     /* read enabled key data list */
----

Instead of success, I get this:

----
func=xmlSecKeysMngrGetKey:file=keys.c:line=1246:obj=unknown:subj=xmlSecKeysMngrFindKey:error=1:xmlsec library function failed: 
func=xmlSecDSigCtxProcessKeyInfoNode:file=xmldsig.c:line=790:obj=unknown:subj=unknown:error=45:key is not found:details=NULL
func=xmlSecDSigCtxProcessSignatureNode:file=xmldsig.c:line=503:obj=unknown:subj=xmlSecDSigCtxProcessKeyInfoNode:error=1:xmlsec library function failed: 
func=xmlSecDSigCtxVerify:file=xmldsig.c:line=341:obj=unknown:subj=xmlSecDSigCtxSignatureProcessNode:error=1:xmlsec library function failed: 
Error: signature failed 
ERROR
SignedInfo References (ok/all): 0/0
Manifests References (ok/all): 0/0
Error: failed to verify file "enveloping-sha256-rsa-sha256-verify.xml"
----

Am I correct that this is a bug in the nss backend?

Here is my attempt to fix this:

----
diff --git a/src/nss/x509.c b/src/nss/x509.c
index 5dc7b69..e4f4bb8 100644
--- a/src/nss/x509.c
+++ b/src/nss/x509.c
@@ -687,13 +687,11 @@ xmlSecNssKeyDataX509XmlRead(xmlSecKeyDataId id, xmlSecKeyPtr key,
         return(-1);
     }
 
-    if((keyInfoCtx->flags & XMLSEC_KEYINFO_FLAGS_X509DATA_DONT_VERIFY_CERTS) == 0) {
-        ret = xmlSecNssKeyDataX509VerifyAndExtractKey(data, key, keyInfoCtx);
-        if(ret < 0) {
-            xmlSecInternalError("xmlSecNssKeyDataX509VerifyAndExtractKey",
-                                xmlSecKeyDataKlassGetName(id));
-            return(-1);
-        }
+    ret = xmlSecNssKeyDataX509VerifyAndExtractKey(data, key, keyInfoCtx);
+    if(ret < 0) {
+        xmlSecInternalError("xmlSecNssKeyDataX509VerifyAndExtractKey",
+                            xmlSecKeyDataKlassGetName(id));
+        return(-1);
     }
     return(0);
 }
diff --git a/src/nss/x509vfy.c b/src/nss/x509vfy.c
index fee2644..02080c9 100644
--- a/src/nss/x509vfy.c
+++ b/src/nss/x509vfy.c
@@ -213,13 +213,18 @@ xmlSecNssX509StoreVerify(xmlSecKeyDataStorePtr store, CERTCertList* certs,
             continue;
         }
 
-        /* it's important to set the usage here, otherwise no real verification
-         * is performed. */
-        status = CERT_VerifyCertificate(CERT_GetDefaultCertDB(),
-                                        cert, PR_FALSE,
-                                        certificateUsageEmailSigner, 
-                                        timeboundary , NULL, NULL, NULL);
-	    if (status == SECSuccess) {
+        if((keyInfoCtx->flags & XMLSEC_KEYINFO_FLAGS_X509DATA_DONT_VERIFY_CERTS) == 0) {
+            /* it's important to set the usage here, otherwise no real verification
+             * is performed. */
+            status = CERT_VerifyCertificate(CERT_GetDefaultCertDB(),
+                                            cert, PR_FALSE,
+                                            certificateUsageEmailSigner,
+                                            timeboundary , NULL, NULL, NULL);
+            if(status == SECSuccess) {
+                break;
+            }
+        } else {
+            status = SECSuccess;
             break;
         }
     }
----

I.e. instead of disabling the whole
xmlSecNssKeyDataX509VerifyAndExtractKey(), still execute it when the
"don't verify" flag is set, just don't call recently fixed
CERT_VerifyCertificate() function.

Is this the correct approach to fix the problem?

If so, I'm happy to extend the xmlsec1 test app to have a cmdline
parameter that sets XMLSEC_KEYINFO_FLAGS_X509DATA_DONT_VERIFY_CERTS, and
then I could sent a pull request together with a testcase.

But perhaps my assumptions are wrong. ;-)

Thanks,

Miklos
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://www.aleksey.com/pipermail/xmlsec/attachments/20170118/3747daae/attachment.sig>