Drop workarounds for GnuTLS <3.4.6 #3790
Open
+9
−65
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
And gnutls_transport_get_int(). As of GnuTLS 3.1.9, we no longer have to use the _ptr() functions. The _int() functions are a simpler alternative, assuming TCP is the transport layer. As of this commit, there's no longer any real benefit to keeping pcmk__tls_get_client_sock(), except to provide a layer of abstraction so that GnuTLS library functions are called in fewer places. https://gnutls.org/manual/html_node/Setting-up-the-transport-layer.html Ref T967 Signed-off-by: Reid Wahl <[email protected]>
Except in deprecated crm_gnutls_global_init(). As of GnuTLS 3.3.0, the GnuTLS library is initialized on load, and it's no longer necessary to call these functions explicitly. https://www.gnutls.org/manual/html_node/Initialization.html https://www.gnutls.org/manual/html_node/Core-TLS-API.html#gnutls_005fglobal_005finit Ref T967 Signed-off-by: Reid Wahl <[email protected]>
Instead of calling gnutls_certificate_set_verify_function() with the custom callback verify_peer_cert(). gnutls_session_set_verify_cert() is available since GnuTLS 3.4.6. It sets a verify function for the entire session, overriding any verify function set for a particular certificate (for example, using gnutls_certificate_set_verify_function()). For our purposes, each session has a unique certificate anyway, so the effect is the same either way. gnutls_session_set_verify_cert() sets up a verify callback function automatically, using hostname and flags parameters. At the time of this commit, it's called auto_verify_cb(); it calls gnutls_certificate_verify_peers() or a related function and returns 0 on success or GNUTLS_E_CERTIFICATE_ERROR or GNUTLS_E_CERTIFICATE_VERIFICATION_ERROR on error. * Our verify_peer_cert() function passes NULL to gnutls_certificate_verify_peers3() to disable hostname verification. Accordingly, we pass NULL to gnutls_session_set_verify_cert(). * We don't currently override the default verify flags (which would have required a call to gnutls_certificate_set_verify_flags()). So we pass 0 for the flags argument here, which says to use the defaults. There will be changes in the output upon error, as we lose our custom error processing from verify_peer_cert(), but that seems acceptable. Closes T967 Signed-off-by: Reid Wahl <[email protected]>
clumens
reviewed
Jan 9, 2025
| * FIXME: When we can use gnutls >= 3.3.0, we don't have to call | ||
| * gnutls_global_init anymore. | ||
| */ | ||
| gnutls_global_init(); | ||
| gnutls_global_set_log_level(8); |
There was a problem hiding this comment.
While you're here, can you also add a comment to remind us to not bump the log level beyond 8, since higher levels can log sensitive information? I should have done this when I added the code.
| */ | ||
| gnutls_certificate_set_verify_function(tls->credentials.cert, verify_peer_cert); | ||
| // Register a function to verify the peer's certificate | ||
| gnutls_session_set_verify_cert(session, NULL, 0); |
There was a problem hiding this comment.
Did you get a chance to test this out, or should I do that?
There was a problem hiding this comment.
Honestly no, though I traced the gnutls source code and documentation, which made this seem like a drop-in replacement.
Feel free to test if you want. I can, but I need to figure out how to test good and bad (and possibly absent on one end, if relevant) certificates. That will happen after business hours today, as I'm running out of time.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes T967