Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(D)TLS fixes, especially certificate verification #128

Merged
merged 12 commits into from
Nov 5, 2024
Merged

Conversation

cgzones
Copy link
Contributor

@cgzones cgzones commented Oct 29, 2024

See individual commits for more details.

The SSL verification is used by TLS and DTLS, refactor to a separate
translation unit.
Used version from systemd/systemd@90d5967
since the current upstream implementation via TAKE_GENERIC is not
available here.
SSL_set_fd(3) does not consume the file descriptor.
The SSL context is currently unconditionally created during connect,
while it is only destroyed in free. Move the initialization to init,
since the context is independent from the individual connection.
SSL_write(3) does not set errno, but sets an internal error code.
Also check for EAGAIN cases.
Avoid accidental TLS verification passed in
ssl_verify_certificate_validity() due to a sockaddr_pretty() failure.
Store the prettified string instead of the raw address struct.
Drop the mode NONE, since NO, DENY, WARN and ALLOW should cover all
cases.
Use DENY as default as its the most secure one, and users might expect
applications to be secure by default.  Failures are reported in the logs
and can then be exempted by choosing a different option.
Currently SSL verification is broken as SSL_get_verify_result(3) is not
supposed to be called in the verify callback, but afterwards.  Thus
currently the wrong error code is checked.
Re-implement the callback similar to the example from SSL_set_verify(3).
@ssahani ssahani merged commit ca76343 into systemd:main Nov 5, 2024
1 check passed
@cgzones cgzones deleted the tls branch November 5, 2024 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants