Add improved handling for TLS certificates for static builds. #17605
Conversation
github-actions
bot
added
the
area/packaging
|
Flagging @netdata/agent as a reviewer for help with testing. |
| select_internal_certs() { | ||
| echo "Using bundled TLS configuration and certificates" | ||
| ln -sf /opt/netdata/share/ssl /opt/netdata/etc/ssl |
There was a problem hiding this comment.
This doesn't work because if a directory or symlink to a directory with the target name already exists, the symlink will be created inside it.
There is -n option (fixes the problem)
-n, --no-dereference
treat LINK_NAME as a normal file if it is a symbolic link to a directory
but POSIX ln doesn't have it.
There was a problem hiding this comment.
# ln -sfv /opt/netdata/share/ssl /opt/netdata/etc/ssl
'/opt/netdata/etc/ssl/ssl' -> '/opt/netdata/share/sslThere was a problem hiding this comment.
This should be fixed by the latest commit.
| test_certs() { | ||
| /opt/netdata/bin/curl --fail --silent --output /dev/null "${NETDATA_CERT_TEST_URL}" || return 1 |
There was a problem hiding this comment.
Should we check for 60 exit code only?
60 Peer certificate cannot be authenticated with known CA
certificates.
There was a problem hiding this comment.
Failing to check at all means we run in to the same potential issue we have now, so I would prefer to just act as if the check failed in that case instead of risking claiming not working.
There was a problem hiding this comment.
As far as I understand, we only need to use the bundled version when the error is "The peer certificate cannot be authenticated using known CA certificates." Why doing it on any other error? If someone set wrong "test claim URL" or any other not "cant auth using known CA certs" error. Can you provide an example of other error you think we need to use bundle?
There was a problem hiding this comment.
There are literally hundreds of reasons that the cURL command could fail that have nothing to do with certificates. Any of those cases mean that we have no idea if the system certificates work or not.
Unless it’s an offline install though, it’s safer in that case to use our bundled certificates instead of the system certificates, because we know that will work in the common case of trying to connect to the Cloud, while we do not know if the system certificates will.
| else | ||
| replace_symlink() { | ||
| target="${1}" | ||
| name="${2}" |
There was a problem hiding this comment.
@Ferroin ,
please, take a look on this warning.
Summary
Our current static builds include a bundled copy of the CA certificates available in the build environment, and will automatically fall back to using those if they can’t find a usable set of CA certificates on the target system.
This works in a majority of cases, but it has a couple of issues:
This PR works to remedy those issues. It adds additional logic to the installation code embedded in the static builds that handles the TLS certificates, adding support both for checking the usability of the selected certificates and for skipping the system certificates and just using the bundled ones. This functionality is controlled by two new options and associated environment variables:
--certificatesandNETDATA_CERT_MODEallow specifying the certificate handling mode.autoandsystemhave the current behavior.checkexpands the current behavior by testing the detected system certificate store for usability by connecting to a well-known URL.bundledignores any system certificates and unconditionally uses the bundled certificates. Defaults toautoif unspecified (thus preserving the existing behavior).--certificate-test-urlandNETDATA_CERT_TEST_URLallow overriding the URL used bycheckmode for testing whether the system certificates are usable. Defaults tohttps://app.netdata.cloudif unspecified.The options override any value specified by the environment variables.
Additionally, the kickstart script is updated to pass the claiming URL as the certificate test URL when installing a static build, and to make the certificate handling mode default to
checkfor installs other than offline installs andautomode for offline installs. The values passed by the kickstart script can still be overridden by using the--static-install-optionsoption to add the--certificatesor--certificate-test-urloptions as needed.Test Plan
Testing requires manually installing static builds produced from this PR on a variety of systems with various values for the
--certificatesand--certificate-test-urloptions and observing how the symlink at/opt/netdata/etc/sslis updated.