[xmlsec] mscrypto, failing signature creation

[Aleksey Sanin]

"Better" is the worst enemy of "good".

Could you please try this patch out?

https://github.com/lsh123/xmlsec/pull/112

I'll merge it if it looks good to you (I still don't have
Windows environment :) ).

Aleksey

On 5/7/17 7:04 AM, Miklos Vajna wrote:
> Hi,
> 
> I'm working on upgrading xmlsec as bundled by LibreOffice and with the
> updated version creating a new signature on Windows doesn't work
> anymore. I bisected the problem and it's commit
> fd1807670cd6b919a213f5603dd27bf2b056c93d that causes the problem for us.
> The error callback of LibreOffice is is invoked like this:
> 
> debug:2688:2836: ..\src\mscrypto\x509.c:718: xmlSecMSCryptoKeyDataX509XmlWrite() 'x509' 'xmlSecMSCryptoKeyDataX509GetCert' 1 'pos=0' The operation completed successfully.
> debug:2688:2836: ..\src\keyinfo.c:178: xmlSecKeyInfoNodeWrite() 'x509' 'xmlSecKeyDataXmlWrite' 1 'node=X509Data' The operation completed successfully.
> debug:2688:2836: ..\src\xmldsig.c:806: xmlSecDSigCtxProcessKeyInfoNode() '' 'xmlSecKeyInfoNodeWrite' 1 ' ' The operation completed successfully.
> debug:2688:2836: ..\src\xmldsig.c:503: xmlSecDSigCtxProcessSignatureNode() '' 'xmlSecDSigCtxProcessKeyInfoNode' 1 ' ' The operation completed successfully.
> debug:2688:2836: ..\src\xmldsig.c:286: xmlSecDSigCtxSign() '' 'xmlSecDSigCtxSignatureProcessNode' 1 ' ' The operation completed successfully.
> 
> Reading the mentioned commit and this trace, here is a trivial revert
> that fixes the problem for me:
> 
> ----
> diff --git a/src/mscrypto/x509.c b/src/mscrypto/x509.c
> index 08c9088d..40dbb39a 100644
> --- a/src/mscrypto/x509.c
> +++ b/src/mscrypto/x509.c
> @@ -392,11 +392,7 @@ xmlSecMSCryptoKeyDataX509GetCert(xmlSecKeyDataPtr data, xmlSecSize pos) {
>      xmlSecAssert2(ctx->hMemStore != 0, NULL);
>      xmlSecAssert2(ctx->numCerts > pos, NULL);
> 
> -    while (pos > 0) {
> -       pCert = CertEnumCertificatesInStore(ctx->hMemStore, pCert);
> -       if(pCert == NULL) {
> -            break;
> -       }
> +    while ((pCert = CertEnumCertificatesInStore(ctx->hMemStore, pCert)) && (pos > 0)) {
>          pos--;
>      }
> 
> 
> ----
> 
> Sounds like this wasn't meant to be a functional change, but in case pos
> is 0, then now we return NULL unconditionally, while previously
> CertEnumCertificatesInStore() was invoked once.
> 
> Aleksey, should I send a pull request with just this revert, or you
> have an idea how to fix the problem in a better way?
> 
> Thanks,
> 
> Miklos
> 
> 
> 
> _______________________________________________
> xmlsec mailing list
> xmlsec at aleksey.com
> http://www.aleksey.com/mailman/listinfo/xmlsec
>