check Application URI of the server Certificate on OpenSecureChannel

OPCFoundation / UA-.NETStandard

OPC Unified Architecture .NET Standard

Other

1.94k stars 942 forks source link

check Application URI of the server Certificate on OpenSecureChannel #2583

Closed romanett closed 5 months ago

romanett commented 5 months ago

Proposed changes

Implement OpenSecureChannel in compliance with Spec Part 5.4.1

The ApplicationUri specified in the Server Certificate is the same as the ApplicationUri provided in the EndpointDescription.

Related Issues

Fixes #2032

Types of changes

[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 5 months ago

Codecov Report

Attention: Patch coverage is 30.76923% with 9 lines in your changes are missing coverage. Please review.

Project coverage is 54.59%. Comparing base (fb0fc44) to head (c925e68). Report is 3 commits behind head on master.

Files	Patch %	Lines
Libraries/Opc.Ua.Client/Session.cs	25.00%	6 Missing and 3 partials :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #2583 +/- ## ========================================== + Coverage 54.52% 54.59% +0.07% ========================================== Files 342 342 Lines 65044 65054 +10 Branches 13331 13334 +3 ========================================== + Hits 35462 35517 +55 + Misses 25728 25679 -49 - Partials 3854 3858 +4 ```

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

romanett commented 5 months ago

To create a test you would need to override some methods of the session to be able to change the endpoint description, can someone provide a hint?

romanett commented 5 months ago

I did some additional manual testing both and cases disallow a new connection:

server with Application Uri in Cert not matching the returned Endpoint Description
EndPointDescription without specified ApplicationUri