-
Notifications
You must be signed in to change notification settings - Fork 961
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GDS: implement ApplicationSelfAdmin privilege in GlobalDiscoverySampleServer #2338
GDS: implement ApplicationSelfAdmin privilege in GlobalDiscoverySampleServer #2338
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2338 +/- ##
==========================================
- Coverage 54.08% 53.92% -0.17%
==========================================
Files 334 335 +1
Lines 64430 64509 +79
Branches 13241 13265 +24
==========================================
- Hits 34850 34787 -63
- Misses 25840 25955 +115
- Partials 3740 3767 +27 ☔ View full report in Codecov by Sentry. |
The functionality of this pull request was in parts successfully tested with the UAExpert as a client and the NetCoreGlobalDiscoveryServer from the Samples as a GDS (with the changes of the PR applied to the GDS server library). After registering the UAExpert to the NetCoreGlobalDicoveryServer by manually adding the UAExperts certificate to the trust and applications folder, the UAExpert could issue a certificate signing request and finish it on behalf of itself. Alse the GetTrustList method could be called. |
Pleased to hear that. For a fully automatic workflow the changes from this pull request are needed as well: You need to call the Method AddOwnCertificateToTrustedStoreAsync() with the GDS CA as parameter, so that the GDS trusts the CA signed Certificate of the UAExpert. Then the UAExpert can receive the TrustList as well using the GDS Pull registration. |
…nSelfAdminPrivilege
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @romanett, I started playing with your implementation and I would really like to see a positive/negative tests where a non registered or registered application try to walk through the permitted steps like self renew etc. and pass / fail. Its a security feature so we need to ensure all pieces positive/negative cases are covered.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@romaett , is there a conflict with #2444?
Should we wait before merging this one?
use WellKnownRoles
delete obsolete roles document WellKnownRoles
Hi @romanett, can I take another look? |
@mregen yes, well known roles are implemented now |
…nSelfAdminPrivilege
Libraries/Opc.Ua.Server/Configuration/ConfigurationNodeManager.cs
Outdated
Show resolved
Hide resolved
…ub.com/romanett/UA-.NETStandard into ImplementApplicationSelfAdminPrivilege
|
looks like I broke some tests, but the problem may have been hidden before due to some async code which was not awaited. |
fails because GDS CA is not trusted by the Client, this error was hidden before. We don´t need the validation here because it is test code, i changed the code to directly apply the certificate: Or should I add the CA to the Issuer store? 02/26/2024 17:20:31.158 Checking application instance certificate. |
Proposed changes
Many OPC UA Pull Clients rely on the GDS implementing the ApplicationSelfAdmin privilege, to pull the trust list after sucessfully receiving a gds signed certificate. This pull request implements this authorization mehtod.
This authorization is described here in the OPC Spec:
https://reference.opcfoundation.org/GDS/v105/docs/7.6
Related Issues
(GDS pull model method authorization)
Types of changes
Checklist
Further comments