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.

Sign up for GitHub

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

Jump to bottom

Disable TLS renegotiation and fix compile error on OpenBSD #9943

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Disable TLS renegotiation and fix compile error on OpenBSD #9943

wants to merge 1 commit into from

Conversation

Al2Klimov
Copy link
Member

@sthen Ok?

@Al2Klimov Al2Klimov added the core/build-fix Follow-up fix, not released yet label Dec 19, 2023
@Al2Klimov Al2Klimov added this to the 2.15.0 milestone Dec 19, 2023
@cla-bot cla-bot bot added the cla/signed label Dec 19, 2023
@Al2Klimov Al2Klimov mentioned this pull request Dec 20, 2023
Open
@sthen
Copy link
Contributor

sthen commented Dec 22, 2023

ok with me!

yhabteab
yhabteab reviewed Dec 22, 2023
View reviewed changes
Copy link
Member

@yhabteab yhabteab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have absolutely no clue about OpenBSD, but how do you turn off tls renegotiation then?

@Al2Klimov
Copy link
Member Author

That's the neat part, you don't! It's already the default. https://github.com/libressl/portable/blob/68ad61fd6d199607af327188c2dad0779f98fa46/ChangeLog#L2316-L2318

yhabteab
yhabteab previously approved these changes Dec 22, 2023
View reviewed changes
Copy link
Member

@yhabteab yhabteab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤷

julianbrost
julianbrost reviewed Jan 2, 2024
View reviewed changes
lib/base/tlsutility.cpp Outdated
Comment on lines 103 to 105
# ifdef SSL_OP_NO_RENEGOTIATION
flags |= SSL_OP_NO_RENEGOTIATION;
# endif /* SSL_OP_NO_RENEGOTIATION */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there some macro that could be used to detect LibreSSL? If OpenSSL would change something with SSL_OP_NO_RENEGOTIATION (something like rename it in v4, I hope they don't, but they probably could), this could would probably still compile but I'd argue that this would be a situation someone should look into, so it would be better if we check for the library we're writing this for rather than the existence of some constant.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be both LIBRESSL_VERSION_NUMBER and LIBRESSL_VERSION_TEXT as described in OPENSSL_VERSION_NUMBER(3). (Note: man.openbsd.org has currently operational difficulties, that's why this man mirror was used)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normally we'd suggest not checking based on library name/version.

The same concern would apply to pretty much any other change that openssl could conceivably make to their API...

@Al2Klimov
Copy link
Member Author

Actually, despite what I thought, v2.14.0 from ports allows renegotiation by default:

    Verify return code: 19 (self signed certificate in certificate chain)
---
R
RENEGOTIATING
depth=1 CN = aklimov-openbsd.my.domain
verify error:num=19:self signed certificate in certificate chain
verify return:0
R
RENEGOTIATING
depth=1 CN = aklimov-openbsd.my.domain
verify error:num=19:self signed certificate in certificate chain
verify return:0
closed
aklimov-openbsd#

However with

--- lib/base/tlsutility.cpp
+++ lib/base/tlsutility.cpp
@@ -90,5 +90,5 @@ static void InitSslContext(const Shared<boost::asio::ssl::context>::Ptr& context
 	long flags = SSL_CTX_get_options(sslContext);

-	flags |= SSL_OP_CIPHER_SERVER_PREFERENCE;
+	flags |= SSL_OP_CIPHER_SERVER_PREFERENCE | SSL_OP_NO_CLIENT_RENEGOTIATION;

 	SSL_CTX_set_options(sslContext, flags);

under patches/:

    Verify return code: 19 (self signed certificate in certificate chain)
---
R
RENEGOTIATING
14177350148600:error:1400444C:SSL routines:CONNECT_CR_SRVR_HELLO:tlsv1 alert no renegotiation:/usr/src/lib/libssl/ssl_pkt.c:753:SSL alert number 100
14177350148600:error:140040E5:SSL routines:CONNECT_CR_SRVR_HELLO:ssl handshake failure:/usr/src/lib/libssl/ssl_pkt.c:495:
aklimov-openbsd#

@Al2Klimov Al2Klimov changed the title Fix compile error on OpenBSD which has no SSL_OP_NO_RENEGOTIATION Disable TLS renegotiation and fix compile error on OpenBSD Jan 17, 2024
@oxzi oxzi mentioned this pull request Apr 2, 2024
Open
@Al2Klimov Al2Klimov linked an issue Apr 2, 2024 that may be closed by this pull request
Icinga2 fails to build against LibreSSL 3.8.3 #10034
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sthen sthen sthen left review comments

@oxzi oxzi oxzi left review comments

@yhabteab yhabteab yhabteab left review comments

@julianbrost julianbrost Awaiting requested review from julianbrost

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
cla/signed core/build-fix Follow-up fix, not released yet
Projects
None yet
Milestone
2.15.0
Development

Successfully merging this pull request may close these issues.

Icinga2 fails to build against LibreSSL 3.8.3
5 participants
@Al2Klimov @sthen @julianbrost @oxzi @yhabteab