ENG-2386: implement "encryption" certificates for "encrypted" connection #123
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
cmt changes
led0nk
requested review from
JeremyTheocharis,
kanapuli and
Scarjit
and removed request for
JeremyTheocharis
There was a problem hiding this comment.
Overall Impressions
-
Great Feature Work:
The implementation of encryption certificates for Basic256Sha256, Basic256, and Basic128Rsa15 is a key step for securing OPC UA connections. The work is promising and sets a strong foundation for future enhancements. -
User Experience Focus:
Although this is a security feature, it’s crucial that even non-security experts can understand and use it. The logs, configuration messages, and documentation should be clear, friendly, and guide the user seamlessly—mirroring the user-centric experience of products like Apple and Linear.
Detailed Technical Feedback
-
Certificate Generation & Seed Handling (opcua_plugin/connect.go & generate_cert.go):
- Random String Length:
Increase the default certificate seed random string length from 8 to 64 characters for better entropy. - User Messaging:
Log a clear message that includes a copy-paste-ready configuration snippet when a random certificate seed is generated. For example:
"The client certificate was generated randomly upon startup and will change on every restart. To use the current dynamically generated certificate as a fixed certificate, copy the following configuration snippet into your config: certificateSeed: "<generated_seed>"." (where generated_seed is the current auto-generated g.certificateSeed)
- Random String Length:
-
Configuration & Documentation (opcua_plugin/opcua.go):
- Rewording of the
certificateSeedField:
Update the description to:
"The certificate seed is used to deterministically generate the client certificate when using encryption. If not set, a new (random) certificate will be generated at every restart, requiring the OPC UA server to trust all client certificates. For a fixed certificate, set this value to a fixed random string of at least 64 characters."
- Rewording of the
Example:
if g.CertificateSeed == "" {
// Generate a 64-character random seed if none is provided
g.CertificateSeed = randomString(64)
// Prepare a configuration snippet for the user to copy into their config
configSnippet := fmt.Sprintf("certificateSeed: \"%s\"", g.CertificateSeed)
g.Log.Infof("The client certificate was generated randomly upon startup and will change on every restart.\n"+
"To use a fixed certificate, please copy and paste the following line into your configuration:\n%s", configSnippet)
}
Here an example for the README:
Encryption Certificates & Certificate Seed
To enhance the security of OPC UA connections, benthos-umh now supports generating client certificates when using encryption. This feature operates in two modes:
-
Dynamic Mode (Default):
If no certificate seed is provided, benthos-umh automatically generates a new random certificate seed (64 characters long) at each startup. This results in a new client certificate every time the service restarts, meaning the OPC UA server must be configured to trust all client certificates. -
Fixed Mode:
For environments where the OPC UA server requires each client certificate to be explicitly trusted, you can set a fixed certificate seed. By providing a fixed random string (of at least 64 characters) via the configuration, the client certificate will be deterministically generated on every startup, ensuring consistency.
How It Works
When operating in Dynamic Mode, the application logs a clear message that includes a copy-paste-ready configuration snippet. For example, you might see a log message like this:
Client Certificate Notice:
The client certificate was generated randomly upon startup and will change on every restart.
To use the current dynamically generated certificate as a fixed certificate, copy the following configuration snippet into your config:
certificateSeed: "REPLACE_WITH_GENERATED_SEED"
Simply copy that line into your configuration file to lock the certificate to the current seed, ensuring that the same certificate is generated on every restart.
Configuration Example
Below is an example of how to set up the OPC UA input with a fixed certificate seed in your benthos.yaml:
input:
opcua:
endpoint: 'opc.tcp://localhost:46010'
nodeIDs: ['ns=2;s=IoTSensors']
securityMode: SignAndEncrypt
securityPolicy: Basic256Sha256
certificateSeed: "your-fixed-64-character-random-string"Replace "your-fixed-64-character-random-string" with a secure, random string that is at least 64 characters long. If you leave out the certificateSeed field, benthos-umh will generate one automatically at startup, which means the client certificate will change with every restart.
opcua_plugin/connect.go
Outdated
| g.Log.Errorf("Failed to generate certificate: %v", err) | ||
| return nil, err | ||
| } | ||
| // Generate certificates if we don't connect without Security |
There was a problem hiding this comment.
maybe no double negation? Generate certificates, when we connect with security
| } | ||
| // cache the tls.Certificate for future calls | ||
| g.cachedTLSCertificate = &cert | ||
| g.Log.Infof("The clients certificate was created, to use an encrypted connection "+ |
There was a problem hiding this comment.
this only make sense when the certificate seed was choosen by the user / is fixed. if its random, it will not work
There was a problem hiding this comment.
if its random, you can instead output:
The client certificate was generated randomly upon starting up, and will change upon every restart. In order to fix it, e.g., to trust the client certificate in the server, please set the certificateSeed cofniguration variable to a random 64 length string (e.g., generated by a random generator) or alternatively use this to fix the current certificate: %s), (g.CertificateSeed)
| @@ -46,7 +47,8 @@ var OPCUAConfigSpec = service.NewConfigSpec(). | |||
| Field(service.NewBoolField("useHeartbeat").Description("Set to true to provide an extra message with the servers timestamp as a heartbeat").Default(false)). | |||
| Field(service.NewIntField("pollRate").Description("The rate in milliseconds at which to poll the OPC UA server when not using subscriptions. Defaults to 1000ms (1 second).").Default(DefaultPollRate)). | |||
| Field(service.NewBoolField("autoReconnect").Description("Set to true to automatically reconnect to the OPC UA server when the connection is lost. Defaults to 'false'").Default(false)). | |||
| Field(service.NewIntField("reconnectIntervalInSeconds").Description("The interval in seconds at which to reconnect to the OPC UA server when the connection is lost. This is only used if `autoReconnect` is set to true. Defaults to 5 seconds.").Default(5)) | |||
| Field(service.NewIntField("reconnectIntervalInSeconds").Description("The interval in seconds at which to reconnect to the OPC UA server when the connection is lost. This is only used if `autoReconnect` is set to true. Defaults to 5 seconds.").Default(5)). | |||
| Field(service.NewStringField("certificateSeed").Description("The certificate seed is a secret string provided by the user, which will then get hashed to create the clients certificate. This is needed to ensure an explicit trusted client, which is significant for an encrypted connection. If not set, this will be provided by a random created string.").Default("")) | |||
There was a problem hiding this comment.
can we put that into the README as well? Maybe also rename that:
The certificate seed is used to deterministically create the client certificate when using encryption. If not set, then the certificate will be random upon restart, which means the server must trust all client certificates. If the server requires to trust each single client certificate, then the certificate seed must be set to a random (but therefore fixed) string with at least 64 characters.
opcua_plugin/connect.go
Outdated
| if g.CertificateSeed == "" { | ||
| // Generate an 8-character random string if no 'certificateSeed' | ||
| // provided by the user. | ||
| g.CertificateSeed = randomString(8) |
There was a problem hiding this comment.
I would increase the random string from 8 to 64
.DS_Store
Outdated
Description:
What we are trying to achieve here:
Basic256Sha256Basic256(depracated)Basic128Rsa15(depracated)User flow:
important: set the fields
securityModeandsecurityPolicyapplication-uri, benthos-umh will retry connection and then get the specified nodes / browse firstThe easiest way for user-experience would be to set the option
Trust all clientsin the OPC-UA-Servers configuration, which depends on the Software you're running on.Examples:
Basic256Sha256
See the logs of this run here:
basic256sha256.log
Basic256
See the logs of this run here:
basic256.log
Basic128Rsa15
basic128rsa15.log
What needs to be done here
As an outlook:
benthos-umh/opcua_plugin/connect.go
Lines 394 to 426 in 4db3728
-> this would lead to benthos retrying to connect every 5 seconds, which makes it much harder for the user to get the correct
application-urifrom the logs (as you can see in all of the provided logs)