feat(inputs.opcua): Add server certificate trust configuration #17825
Conversation
Add support for explicitly trusting server certificates when connecting to OPC UA servers with self-signed certificates. This resolves connection failures with "StatusBadSecurityChecksFailed" error when using SignAndEncrypt security mode.
telegraf-tiger
bot
added
area/opcua
feat
There was a problem hiding this comment.
Thanks @skartikey for the PR! Some small comments in the code, the biggest being the naming of the option...
Co-authored-by: Sven Rebhan <36194019+srebhan@users.noreply.github.com>
Rename configuration option from server_certificate to remote_certificate for improved clarity, especially for opcua_listener plugin. Also restructure README to make "Server Certificate Trust" a subsection under Configuration.
- Rename config option for clarity across opcua and opcua_listener plugins - Simplify documentation: remove third-party specifics and example configs - Fix race condition: read certificate content immediately after validation - Simplify test assertions to inline require statements - Update coding guidelines with review learnings
|
Download PR build artifacts for linux_amd64.tar.gz, darwin_arm64.tar.gz, and windows_amd64.zip. 📦 Click here to get additional PR build artifactsArtifact URLs |
There was a problem hiding this comment.
Thanks for the update @skartikey! I think we should get rid of the additional checks as none of them will guarantee the certificate handling to succeed. So removing all the magic and just provide the remote-certificate-file option is as good as doing all those checks IMO. The library will fail if it cannot load the file and we can pass that on to the user...
| // Validate remote certificate file exists if provided (regardless of security mode) | ||
| if o.RemoteCertificate != "" { | ||
| if _, err := os.Stat(o.RemoteCertificate); err != nil { | ||
| if os.IsNotExist(err) { | ||
| return fmt.Errorf("%w: remote certificate file does not exist: %s", ErrInvalidConfiguration, o.RemoteCertificate) | ||
| } | ||
| return fmt.Errorf("%w: cannot access remote certificate file: %s: %w", ErrInvalidConfiguration, o.RemoteCertificate, err) | ||
| } | ||
| } |
There was a problem hiding this comment.
Do we really need this check? Can't this be handled in the CreateClient function when actually reading the certificate? My reasoning is that this check is actually useless because the file permissions can be changed or the file can be removed between here and the actual read call, so why check it here if it doesn't prevent an error in the read location?
| if o.RemoteCertificate != "" { | ||
| cert, err := os.ReadFile(o.RemoteCertificate) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("%w: failed to read remote certificate file %s: %w", ErrInvalidConfiguration, o.RemoteCertificate, err) |
There was a problem hiding this comment.
Do you need the ErrInvalidConfiguration for checks down below? If so, please add a comment so people don't remove it later. If you don't need it, please remove that part to shorten the error. Furthermore, this actually isn't an invalid configuration but maybe just a permission issue... ;-)
|
|
||
| // If a remote certificate is explicitly configured, use it to override | ||
| // the certificate from the endpoint. This allows trusting self-signed certificates. | ||
| if len(o.remoteCertificate) > 0 { | ||
| o.Log.Debugf("Using explicitly configured remote certificate from %s", o.Config.RemoteCertificate) | ||
| opts = append(opts, opcua.RemoteCertificate(o.remoteCertificate)) | ||
| } |
There was a problem hiding this comment.
So reading of the above, I think you should remove all those checks above and just use the opcua.RemoteCertificateFile (or whatever it was) call here and leave the error handling to the library!
| #### Obtaining the Server Certificate | ||
|
|
There was a problem hiding this comment.
Maybe remove this caption and put it in the text, but that's just a matter of taste not a must.
3 participants
Summary
Add support for explicitly trusting server certificates when connecting to OPC UA servers with self-signed certificates. This resolves connection failures with "StatusBadSecurityChecksFailed" error when using SignAndEncrypt security mode.
The new configuration option allows users to specify a path to the server's certificate file, enabling secure connections to OPC UA servers that use self-signed certificates without manual OS-level certificate trust configuration.
Example configuration:
Checklist
Related issues
resolves #17608