ktls-utils 1.2 release #107
Merged
Conversation
This file contains hidden or 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
Parfait complains about using a pathname to perform an access(2) and then passing the same pathname to open(2). Between the access(2) and the open(2) calls, the permissions can change. I think this is harmless for tlshd, but all the same, let's clean this up. Signed-off-by: Chuck Lever <[email protected]>
Clean up: Commit c380357 ("tlshd: Demote warnings about missing options") removed some debug messages that used the local "error" variable. "error" is passed to glib, but the returned value is no longer checked or used. Fixes: c380357 ("tlshd: Demote warnings about missing options") Signed-off-by: Chuck Lever <[email protected]>
Suggested by CodeQL. Signed-off-by: Chuck Lever <[email protected]>
The NFS mount command is to add keys to the .nfs keyring. Also add a keyring for NFSD configuration. Signed-off-by: Chuck Lever <[email protected]>
Clean up: Move it next to the other related macro definitions. Signed-off-by: Chuck Lever <[email protected]>
tlshd's certificate verification functions return GNUTLS_E_CERTIFICATE_ERROR, not GNUTLS_E_CERTIFICATE_VERIFICATION_ERROR, when certification verification is unsuccessful. This gets rid of an unexpected journal message: tlshd_start_tls_handshake unhandled error -43, returning EACCES Fixes: ecaf660 ("tlshd: Report peer certificate verification failures") Signed-off-by: Chuck Lever <[email protected]>
The error flow for find_key_by_type_and_desc() is wrong -- that API returns -1 when it fails, not zero. Fixes: a70f143 ("keyring: merge keyring lookup in tlshd_keyring_link_session()") Signed-off-by: Chuck Lever <[email protected]>
- No caller ever examines the return value for tlshd_keyring_link_session(), thus the errno set here is always ignored. - The errno values set here are not documented. Clean up by removing set-but-unexamined variables. Signed-off-by: Chuck Lever <[email protected]>
Usually we use tlshd_log_perror() in these case to convert the errno value to an error message. However, here there is more to display than just the name of the failing function. Signed-off-by: Chuck Lever <[email protected]>
For completeness, the sanity check at the top of tlshd_keyring_link_session() should check for an empty string in addition to a NULL string pointer. Signed-off-by: Chuck Lever <[email protected]>
As a debugging aid and for auditing, emit a one-line summary of incoming certificate(s) after a successful handshake. Server-side only. Signed-off-by: Chuck Lever <[email protected]>
The verification routine has already succeeded. The verification status report shows only: The certificate is trusted. If the handshake is successful, that is obvious. Signed-off-by: Chuck Lever <[email protected]>
These were never adopted by the kernel x.509-based TLS consumers, and are to be replaced with handshake tags. Signed-off-by: Chuck Lever <[email protected]>
Refactor the code that emits the handshake completion notice. Simplify and de-duplicate. One functional change: Emit the completion notice only when debugging is enabled. It's a best practice to avoid giving remote attackers a mechanism to flood the system journal. Signed-off-by: Chuck Lever <[email protected]>
Client handshakes return CERTIFICATE_VERIFICATION_ERROR. Fixes: 731d1b2 ("tlshd: Handshake needs to check for CERTIFICATE_ERROR") Signed-off-by: Chuck Lever <[email protected]>
Valgrind reports: ==110975== Invalid read of size 4 ==110975== at 0x405B01: tlshd_gnutls_priority_init (ktls.c:422) ==110975== by 0x4039D1: main (main.c:109) ==110975== Address 0x534b1ec is 1,548 bytes inside a block of size 8,304 free'd ==110975== at 0x4847B4C: free (vg_replace_malloc.c:989) ==110975== by 0x405A02: tlshd_gnutls_priority_init (ktls.c:373) ==110975== by 0x4039D1: main (main.c:109) ==110975== Block was alloc'd at ==110975== at 0x484C214: calloc (vg_replace_malloc.c:1675) ==110975== by 0x48CBD1F: gnutls_priority_init (in /usr/lib64/libgnutls.so.30.37.1) ==110975== by 0x4059DE: tlshd_gnutls_priority_init (ktls.c:366) ==110975== by 0x4039D1: main (main.c:109) The cipher list is allocated as part of the pcache. But tlshd_gnutls_priority_init() releases the pcache immediately after gnutls_priority_cipher_list() returns, then continues to access the returned cipher list, which is now free memory. Refactor tlshd_gnutls_priority_init() so that the lifetime of the pcache is extended until tlshd is done with the cipher list array. Refresh of fix-certificate-verification-error
Clean up valgrind output. Signed-off-by: Chuck Lever <[email protected]>
Reduce false leak reports in valgrind output. Signed-off-by: Chuck Lever <[email protected]>
Since the child doesn't return NL_SKIP, the incoming message is never freed. Signed-off-by: Chuck Lever <[email protected]>
These resources aren't really leaked because the child process exits immediately, but they do show up on valgrind. Release them before exiting so the real leaks are more obvious. Signed-off-by: Chuck Lever <[email protected]>
Since the spawned processes do not handle netlink notifications, they should not leave the notification socket open. Signed-off-by: Chuck Lever <[email protected]>
Clean up dynamically allocated resources after a shutdown signal to reduce the number of false valgrind leak reports. Signed-off-by: Chuck Lever <[email protected]>
Prepare for implementing support for certificate revocation lists by de-duplicating the code that reads in the trust store. Suggested-by: Long Xin <[email protected]> Signed-off-by: Chuck Lever <[email protected]>
Enable server administrators to configure a certificate revocation list. This follows good security practice: The trust store acts as an "allow" list and the CRL acts as a "deny" list. Signed-off-by: Rik Theys <[email protected]> Signed-off-by: Chuck Lever <[email protected]>
If an x509.crl option is specifiedin the authenticate.client section of the configuration file, use it as a certificate revocation list. This commit only adds the check for tcp based TLS sessions. Support for QUIC still needs to be added. Signed-off-by: Rik Theys <[email protected]> Signed-off-by: Chuck Lever <[email protected]>
Update the tlshd.conf man page to include the x509.crl option available in the authenticate.server and authenticate.client sections. Signed-off-by: Rik Theys <[email protected]> Signed-off-by: Chuck Lever <[email protected]>
Manage dynamically-allocated resources in struct tlshd_handshake_parms by adding a function to release these resources before the handshake child process exits. This improves the ability of tlshd to dynamically allocate and manage memory containing variably-sized arguments such as hostnames, arrays, and socket addresses. The parms.peername and parms.peeraddr fields will be included in the set of dynamically-allocated resources, so add the new "release" API call /after/ the handshake completion notice. Signed-off-by: Chuck Lever <[email protected]>
The number of remote peer IDs passed back from the TLS library is not known in advance. Use a flexible array to handle one-at-a-time storage of the peerids as they are retrieved. Signed-off-by: Chuck Lever <[email protected]>
The number of peer IDs passed up from the kernel is not known in advance. Use a flexible array to handle one-at-a-time storage of the peerids as they are retrieved from the netlink arguments. Signed-off-by: Chuck Lever <[email protected]>
There are no consumers that use peeraddr as a sockaddr. Render it as a presentation address as soon as getpeername(3) gets the socket address. Signed-off-by: Chuck Lever <[email protected]>
Ensure that, when allocation or DNS query fails, parms->peername always remains NULL. tlshd should not assume that the buffer returned from getnameinfo(3) will remain untouched if that call is unsuccessful. Fixes: 7655d96 ("tlshd: Move peername/peeraddr preparation") Reported-by: Ken Milmore <[email protected]> Signed-off-by: Chuck Lever <[email protected]>
Signed-off-by: Chuck Lever <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Finalize the set of commits to appear in ktls-utils release 1.2.