[xmlsec] nss / XMLSEC_KEYINFO_FLAGS_X509DATA_DONT_VERIFY_CERTS handling

[Aleksey Sanin]

I am not sure if this is a bug or not. You idea about not
calling verify function sounds correct. Unfortunately,
I am traveling till mid Feb with no time or good internet
to look into it more. Will it wait till then or it is urgent?

Thanks,

Aleksey

On 1/18/17 1:27 PM, Miklos Vajna wrote:
> 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
> 
> 
> 
> _______________________________________________
> xmlsec mailing list
> xmlsec at aleksey.com
> http://www.aleksey.com/mailman/listinfo/xmlsec
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 842 bytes
Desc: OpenPGP digital signature
URL: <http://www.aleksey.com/pipermail/xmlsec/attachments/20170118/30c9da36/attachment.sig>