Fix handling of Subject Names with specials Characters / and = using " as escaping character

romanett commented 2 months ago

Proposed changes

As per OPC UA Spec a Subject Name can be provided to methods with a special escaping sequence: If the value contains a ‘/’ or a ‘=’ then it shall be enclosed in double quotes (‘”’).

https://reference.opcfoundation.org/GDS/v105/docs/7.8.3

This fix removes this special escaping sequence when parsing a subject Name.

Related Issues

2586

Types of changes

What types of changes does your code introduce?

[x] Bugfix (non-breaking change which fixes an issue)
[ ] Enhancement (non-breaking change which adds functionality)
[ ] Test enhancement (non-breaking change to increase test coverage)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected, requires version increase of Nuget packages)
[ ] Documentation Update (if none of the other choices apply)

Checklist

[x] I have read the CONTRIBUTING doc.
[x] I have signed the CLA.
[x] I ran tests locally with my changes, all passed.
[ ] I fixed all failing tests in the CI pipelines.
[ ] I fixed all introduced issues with CodeQL and LGTM.
[ ] I have added tests that prove my fix is effective or that my feature works and increased code coverage.
[ ] I have added necessary documentation (if appropriate).
[ ] Any dependent changes have been merged and published in downstream modules.

Further comments

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 54.74%. Comparing base (884ddf3) to head (c7c80db).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #2591 +/- ## ========================================== + Coverage 54.70% 54.74% +0.03% ========================================== Files 342 342 Lines 65041 65042 +1 Branches 13331 13331 ========================================== + Hits 35581 35605 +24 + Misses 25612 25595 -17 + Partials 3848 3842 -6 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

KircMax commented 2 months ago

Thanks for looking into this! I am reading the Spec. a bit differently than it is implemented here I think, but maybe I am just wrong...: "The value may be any printable character except for ‘”’. If the value contains a ‘/’ or a ‘=’ then it shall be enclosed in double quotes (‘”’). " --> I read it that those strings from your test are actually to be interpreted as invalid strings since they contain a '"' ... I'd expect the whole string to be enclosed -> not: "CN=UA Yellow Green Server,DC=dogblueberry,O=OPC\"=\"Foundation" but instead: "CN=UA Yellow Green Server,DC=dogblueberry,O=\"OPC=Foundation\"" So only the value of O= would be enclosed by quotes ... But as I said I might be reading it wrong, just wanted to hint this.

romanett commented 2 months ago

@KircMax Thanks for the hint. However for your case we seem to handle it correctly already w.o. the patch. Can you provide the complete subject name contained in the Certificate Request created by your server? You can read it out e.g. with Bounce Castle:

var pkcs10CertificationRequest = new Org.BouncyCastle.Pkcs.Pkcs10CertificationRequest(certificateRequestByteArray);
var info = pkcs10CertificationRequest.GetCertificationRequestInfo();
var Subjectname = info.Subject;

OPCFoundation / UA-.NETStandard