Runtime: Fix System.Xml.Cryptography.Xml : SignedXmlTest.AsymmetricDSASignature

[krwq]

Test is failing because SignatureValue.Length is not 40. This behavior can be correct. When starting to accept values >= 40 it fails for different reasons. Further investigate why is this happening.

cc: @tintoy @anthonylangsworth @peterwurzinger - could you help investigating this one if you got more context?

[tintoy]

I could try - I'm in there right now :)

[krwq]

thank you @tintoy

[tintoy]

@krwq @anthonylangsworth @peterwurzinger - Ok, think I found the problem:

https://github.com/dotnet/corefx/blob/master/src/Common/src/System/Security/Cryptography/DSACng.ImportExport.cs#L34

// J is not required and is not even used on CNG blobs. It should however be less than P (J == (P-1) / Q). This validation check
// is just to maintain parity with DSACryptoServiceProvider, which also performs this check.
if (parameters.J != null && parameters.J.Length >= parameters.P.Length)
    throw new ArgumentException(SR.Cryptography_InvalidDsaParameters_MismatchedPJ);

Verified that this is the issue it's hitting by modifying DSAKeyValue.LoadXml:

DSAParameters parameters = new DSAParameters
{
    P = (pNode != null) ? Convert.FromBase64String(pNode.InnerText) : null,
    Q = (pNode != null) ? Convert.FromBase64String(qNode.InnerText) : null,
    G = (gNode != null) ? Convert.FromBase64String(gNode.InnerText) : null,
    Y = Convert.FromBase64String(yNode.InnerText),
    J = (jNode != null) ? Convert.FromBase64String(jNode.InnerText) : null,
    Seed = (seedNode != null) ? Convert.FromBase64String(seedNode.InnerText) : null,
    Counter = (seedNode != null) ? Convert.FromBase64String(pgenCounterNode.InnerText)[0] : 0
};
if (parameters.J != null && parameters.J.Length >= parameters.P.Length)
    throw new InvalidOperationException("Gotcha!");

Test now fails with an inner-most exception of "Gotcha!":

   System.Security.Cryptography.Xml.Tests.SignedXmlTest.AsymmetricDSAVerify [FAIL]
      System.Security.Cryptography.CryptographicException : An error occurred parsing the key components
      ---- System.InvalidOperationException : Gotcha!
      Stack Trace:
            at System.Security.Cryptography.Xml.DSAKeyValue.LoadXml(XmlElement value)
            at System.Security.Cryptography.Xml.KeyInfo.LoadXml(XmlElement value)
            at System.Security.Cryptography.Xml.Signature.LoadXml(XmlElement value)
            at System.Security.Cryptography.Xml.SignedXml.LoadXml(XmlElement value)
            at System.Security.Cryptography.Xml.Tests.SignedXmlTest.AsymmetricDSAVerify()
         ----- Inner Stack Trace -----
            at System.Security.Cryptography.Xml.DSAKeyValue.LoadXml(XmlElement value)

[tintoy]

The broader problem is that the inner exception is getting lost / munged somewhere along the way - the actual exception message is quite informative:

The specified DSA parameters are not valid; J (if present) must be shorter than P.

I'll have a look at Internal.Cryptography.CryptoThrowHelper next since this seems to be where the exception message is getting borked.

[anthonylangsworth]

Looks good, @tintoy . Thanks.

[tintoy]

Nope, looks like that's not the problem. I'll keep looking.

[tintoy]

Ok, here's where the exception originates:

https://github.com/dotnet/corefx/blob/master/src/Common/src/System/Security/Cryptography/ECCng.ImportExport.cs#L476

[tintoy]

Ok, after wearing my F11 key down to a nub, it looks to me like the import / export of the Counter parameter is not symmetric across import / export on DSAKeyValue:

Import:

Convert.FromBase64String(pgenCounterNode.InnerText)[0]

Export:

Convert.ToBase64String(Utils.ConvertIntToByteArray(dsaParams.Counter))

[tintoy]

Yep, that's definitely it - I modified a quick-and-dirty import / export round-trip test (which was failing with this error) to simply store the counter as an integer and then it started working.

Anyone else want to guess what ConvertIntToByteArray is doing? :-)

[tintoy]

Ok - copied the converse implementation from the netfx reference source, and tested the round-tripping. Definitely works correctly now.

[krwq]

nice work @tintoy! Thanks!

[peterwurzinger]

Hi there!

The assumtion, that Counter is only 1 byte long was my bad - apologies! :-(
Did that fix more issues than this single one? If I member correctly there were more issues related to DSA.

[tintoy]

Did that fix more issues than this single one?

It may, I haven't checked (sorry, haven't have time yet).

[krwq]

Changing the milestone since we don't support creating DSA on OSX. One thing we could do for all similar tests is to export DSA on different platform and import it in the tests.

[krwq]

Actually closing since after resolving all other issues this is now duplicate of: https://github.com/dotnet/corefx/issues/17001