Skip to content

Require exact LDS certificate ApplicationUri registration (OPC 10000-4)#4052

Merged
marcschier merged 4 commits into
masterfrom
marcschier/review-fix/lds-application-uri
Jul 22, 2026
Merged

Require exact LDS certificate ApplicationUri registration (OPC 10000-4)#4052
marcschier merged 4 commits into
masterfrom
marcschier/review-fix/lds-application-uri

Conversation

@marcschier

@marcschier marcschier commented Jul 19, 2026

Copy link
Copy Markdown
Collaborator

Description

This PR splits the LDS registration-validation changes from #4021.

Specification

OPC 10000-4 v1.05.07 §5.5.5.1 requires RegisterServer to use a SecureChannel that supports Client authentication and requires a Discovery Server to reject a registration when serverUri does not match the applicationUri in the Server Certificate used for that SecureChannel.

Failure

LDS registration validation previously skipped the ApplicationUri check when the SecureChannel had no Client certificate, accepted a certificate with no ApplicationUri, and logged malformed-certificate parsing errors before continuing with a successful registration.

Fix

  • Require a SecureChannel Client certificate for registration.
  • Parse the certificate fail closed and return BadCertificateInvalid when it is malformed.
  • Require at least one certificate ApplicationUri and compare every URI entry to ServerUri with ordinal equality.
  • Return BadServerUriInvalid when no certificate ApplicationUri exactly matches ServerUri.

Validation

  • Full Opc.Ua.Lds.Tests: net10.0 (171 passed, 3 skipped), net48 (171 passed, 3 skipped).
  • git diff --check passed.

Related Issues

Checklist

Only verified claims are checked.

  • I have signed the CLA and read the CONTRIBUTING doc.
  • I have added tests that prove my fix is effective or that my feature works and increased code coverage.
  • I have added all necessary documentation.
  • I have verified that my changes do not introduce (new) build or analyzer warnings.
  • I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed.
  • I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings.
  • I have addressed all PR feedback received.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@marcschier
marcschier marked this pull request as ready for review July 19, 2026 11:31
Copilot AI review requested due to automatic review settings July 19, 2026 11:31
@marcschier
marcschier marked this pull request as draft July 19, 2026 11:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Tightens LDS RegisterServer registration validation to comply with OPC UA spec requirements around SecureChannel client authentication and exact ApplicationUri matching in the client certificate, and adds unit tests for the new rejection/acceptance cases.

Changes:

  • Require a SecureChannel client certificate for RegisterServer and fail closed on malformed certificates.
  • Enforce exactly one certificate ApplicationUri and require ordinal equality with RegisteredServer.ServerUri.
  • Add a new focused test suite covering accept/reject scenarios for LDS registration validation.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
tests/Opc.Ua.Lds.Tests/RegistrationValidationTests.cs Adds new tests validating strict certificate presence/parsing and exact single-ApplicationUri registration behavior.
src/Opc.Ua.Lds.Server/LdsServer.cs Implements stricter validation: require client cert, reject malformed certs, and enforce a single exact ApplicationUri match.

Comment thread tests/Opc.Ua.Lds.Tests/RegistrationValidationTests.cs Outdated
Comment thread src/Opc.Ua.Lds.Server/LdsServer.cs
Comment thread src/Opc.Ua.Lds.Server/LdsServer.cs Outdated
Comment thread tests/Opc.Ua.Lds.Tests/RegistrationValidationTests.cs Outdated
@marcschier
marcschier marked this pull request as ready for review July 19, 2026 11:55
Comment thread src/Opc.Ua.Lds.Server/LdsServer.cs Outdated
@marcschier
marcschier marked this pull request as draft July 19, 2026 12:45
@marcschier
marcschier marked this pull request as ready for review July 19, 2026 14:13
marcschier and others added 2 commits July 19, 2026 18:10
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@marcschier
marcschier marked this pull request as draft July 19, 2026 18:04
@marcschier
marcschier marked this pull request as ready for review July 19, 2026 18:08
@marcschier
marcschier requested a review from romanett July 19, 2026 18:37
@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 23 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.55%. Comparing base (70c74e7) to head (939199d).
⚠️ Report is 21 commits behind head on master.

Files with missing lines Patch % Lines
src/Opc.Ua.Lds.Server/LdsServer.cs 0.00% 23 Missing ⚠️

❌ Your patch check has failed because the patch coverage (0.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4052   +/-   ##
=======================================
  Coverage   73.55%   73.55%           
=======================================
  Files        1345     1345           
  Lines      180036   180094   +58     
  Branches    31677    31684    +7     
=======================================
+ Hits       132427   132477   +50     
+ Misses      36911    36906    -5     
- Partials    10698    10711   +13     
Files with missing lines Coverage Δ
src/Opc.Ua.Lds.Server/LdsServer.cs 6.98% <0.00%> (-0.36%) ⬇️

... and 32 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@marcschier marcschier added the ready Ready to merge once CI Passes label Jul 21, 2026

@koepalex koepalex left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@marcschier
marcschier merged commit f10169b into master Jul 22, 2026
130 of 164 checks passed
@marcschier
marcschier deleted the marcschier/review-fix/lds-application-uri branch July 22, 2026 02:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready Ready to merge once CI Passes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants