Refactor SecureChannel Encryption/Decryption & add SecurityPolicyInfo #3293
Conversation
… when reusing transport channel Introduce logic to abort outstanding asynchronous requests (e.g., keepalive and publish requests) when a session is disposed.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #3293 +/- ##
==========================================
- Coverage 57.85% 57.78% -0.07%
==========================================
Files 368 370 +2
Lines 80144 80533 +389
Branches 13907 13955 +48
==========================================
+ Hits 46367 46539 +172
- Misses 29562 29786 +224
+ Partials 4215 4208 -7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
romanett
changed the title
| if (securityPolicyUri != null) | ||
| { | ||
| switch (securityPolicyUri) | ||
| return securityPolicyUri.Contains("#ECC_", StringComparison.Ordinal); |
There was a problem hiding this comment.
Maybe extend SecuritiypolicyInfo, with an IsEcc Property?
There was a problem hiding this comment.
Add AlgortihmFamilyEnum RSA|EC to SecurityPolcyInfo
| /// <summary> | ||
| /// Defines functions to implement ECC cryptography. | ||
| /// </summary> | ||
| public static class EccUtils |
There was a problem hiding this comment.
Is EccUtils the right name for this because SymmetricEncryptAndSign is called by SecureChannel for alle Policies?
|
@randy-armstrong can you check why this is failing. I did not include your refactored EncryptedSecret yet, wanted to do this separately. |
| return new ArraySegment<byte>( | ||
| data.Array, | ||
| 0, | ||
| data.Offset + data.Count + kAesGcmTagLength); |
There was a problem hiding this comment.
kChaChaPolyTagLength instead of kAesGcmTagLength ?
| } | ||
| } | ||
|
|
||
| private static void ApplyChaCha20Poly1305Mask(ChannelToken token, uint lastSequenceNumber, byte[] iv) |
There was a problem hiding this comment.
The new implementation does not involve mix-in of the SequenceNumber and IV in the cipher. Isn't this a security issue ?
|
|
||
| if (signingKey != null) | ||
| { | ||
| using HMAC hmac = securityPolicy.CreateSignatureHmac(signingKey); |
There was a problem hiding this comment.
Isn't creating the instance per message costly ?
|
|
||
| if (!signOnly) | ||
| { | ||
| using var aes = Aes.Create(); |
There was a problem hiding this comment.
Isn't creating the instance per message costly ?
2 participants
This pull request introduces several improvements and refactorings to the security handling and resource management in the OPC UA stack, particularly focusing on the
ChannelTokenand security policy utilities. The main themes are enhanced security policy information retrieval, and a refactor of theChannelTokenclass to clarify property usage and resource disposal.Security policy utilities:
GetInfomethod and a lazily-initialized dictionary (s_securityPolicyUriToInfo) to efficiently retrieveSecurityPolicyInfoobjects by URI, improving security policy lookup and validation. [1] [2]ChannelToken refactoring and improvements:
Refactors the
ChannelTokenclass to clarify and update property responsibilities:SecurityPolicyproperty to track the security policy in use.SecureChannelSecret,ServerCertificate,ClientCertificate).HMAC,SymmetricAlgorithm) in favor of simpler byte array properties, and makes related properties internal for better encapsulation. (Fea73fe4L96R96, Fea73fe4L112R112, Stack/Opc.Ua.Core/Stack/Tcp/ChannelToken.csL120-R157)Simplifies the
Disposelogic inChannelTokenby removing explicit disposal of cryptographic objects, relying on garbage collection and improved property management. [1] [2]Types of changes
What types of changes does your code introduce?
Checklist
Further comments