search

dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.09k stars 4.7k forks source link

C14N transform not omitting xml default namespace #28844

Open brunocapu opened 5 years ago

brunocapu commented 5 years ago

Bad C14N

The XmlDsigC14NTransform (and other Transforms) class does not omit a local xml prefix if defined with default string value http://www.w3.org/XML/1998/namespace when performing the transformation, which is critical for XML signatures digest computation.

General

According to the C14N documentation (http://www.w3.org/TR/2001/REC-xml-c14n-20010315 and latest version), a local xml prefix defined with default string value should be omitted:

2.3 Processing Model ... To finish processing L, simply process every namespace node in L, except omit namespace node with local name xml, which defines the xml prefix, if its string value is http://www.w3.org/XML/1998/namespace.

Indeed, other applications (Java or C++ based) result the expected behavior described in W3C recommendation.

This is a critical behavior as, if this corner case is present, it will not only produce an incorrect signature but also result false positives for .NET based XML signatures and false negatives for 3rd parties XML signatures.

This apparently affects all .NET Frameworks and all .NET Core versions.

Sample

In attachment there is a simple XML sample (netcore/hello.xml in xml-bad-c14n.zip) in order to reproduce the described corner case:

<?xml version="1.0" encoding="UTF-8"?>
<rootElement xmlns:app="http://www.contoso.com/app-namespace">
    <app:toSignElement Id="aa0000ff" xmlns:xml="http://www.w3.org/XML/1998/namespace">
        Hello XML c14n!
    </app:toSignElement>
</rootElement>

.Net

Also a .NET Core 2.2 console application project (netcore directory in xml-bad-c14n.zip) which executes a XmlDsigC14NTransform of one of the XML sample element.

The netcore application results the following c14n:

<app:toSignElement xmlns:app="http://www.contoso.com/app-namespace" xmlns:xml="http://www.w3.org/XML/1998/namespace" Id="aa0000ff">
                Hello XML c14n!
        </app:toSignElement>

Java

On the other hand, the Java sample (java directory in xml-bad-c14n.zip) results the following c14n for the same XML element:

<app:toSignElement xmlns:app="http://www.contoso.com/app-namespace" Id="aa0000ff">
                Hello XML c14n!
        </app:toSignElement>

[NOTE] The Java application does not include the xml prefix.

It is also possible to find on apache source code repositories the code snippet of the specific namespace omitting on C14N implementations, for instance.

Update

We have just noticed a commented line on Utils.cs which looks like it predicates this exactly verification. And a hard false is being returned instead: Utils.IsXmlPrefixDefinitionNode (lines 148-152)

Thank you

bartonjs commented 5 years ago

I just went code spelunking, and it looks like that function has had that definition (with the commented out code) since it was introduced in .NET Framework 1.1 (back in 2003).

Running the corefx tests doesn't show that anything that we test depends on this behavior; but that's both good and bad... I'm 99.9% sure that this wouldn't meet the requirements for an update to .NET Framework without a "retargeting change". And there's definitely a compatibility risk in being unable to process existing saved signed data.

I'm honestly not sure what it would take to give us confidence in taking a change here. Certainly as a caller you can choose to remove all of the xmlns:xml="http://www.w3.org/XML/1998/namespace" attributes before signing or checking the signature...