Skip to content

Windows: use (existing) Schannel, not (shipped) OpenSSL #9956

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
Draft
wants to merge 11 commits into
base: master
Choose a base branch
from
Draft

Windows: use (existing) Schannel, not (shipped) OpenSSL #9956

wants to merge 11 commits into from

Conversation

Al2Klimov
Copy link
Member

fixes #9955

@Al2Klimov Al2Klimov self-assigned this Jan 3, 2024
@cla-bot cla-bot bot added the cla/signed label Jan 3, 2024
@icinga-probot icinga-probot bot added area/api REST API area/distributed Distributed monitoring (master, satellites, clients) area/windows Windows agent and plugins labels Jan 3, 2024
@Al2Klimov
Copy link
Member Author

Al2Klimov commented Jan 9, 2024 •
edited
Loading

@laudrup's Boost.WinTLS seems not to compile as CERT_CHAIN_ENGINE_CONFIG lacks hExclusiveRoot. That member only exists if NTDDI_VERSION >= NTDDI_WIN7 (0x06010000). The only explanation: NTDDI_VERSION <= NTDDI_WS08SP4 (0x06000400), i.e. we kinda unofficially support Windows Server 2008.

@RincewindsHat Didn't you mention customers with a such OS once?

@Al2Klimov
Require NT 6.1+, i.e. Windows 7 and Server 2008 R2
09ec955 
Enables CERT_CHAIN_ENGINE_CONFIG#hExclusiveRoot used by Boost.WinTLS.
Icinga 2 already doesn't start on the original Server 2008.
@laudrup
Copy link

laudrup commented Jan 10, 2024

Boost.WinTLS seems not to compile as CERT_CHAIN_ENGINE_CONFIG lacks hExclusiveRoot. That member only exists if NTDDI_VERSION >= NTDDI_WIN7 (0x06010000). The only explanation: NTDDI_VERSION <= NTDDI_WS08SP4 (0x06000400), i.e. we kinda unofficially support Windows Server 2008.

Not sure if this is relevant for you anymore and I cannot really see that this project uses Boost.WinTLS, but I haven't really thought about what requirements there could be for the platform where Boost.WinTLS is actually used so thanks for pointing that out.

As I don't use Windows myself and mainly build and test this with Github actions where older versions of Windows are not supported I'm not sure I can figure out what to do about this, but thanks again anyway.

@Al2Klimov Al2Klimov force-pushed the SChannel-9955 branch 2 times, most recently from 2e68b97 to ce28926 Compare July 3, 2024 14:47
@Al2Klimov Al2Klimov force-pushed the SChannel-9955 branch 2 times, most recently from 9a599dd to 530ba23 Compare July 4, 2024 09:20
Al2Klimov
Al2Klimov commented Jul 4, 2024
View reviewed changes
lib/base/tlsutility.cpp
{
char errbuf[256];

// Enforce TLS v1.2 as minimum
context->set_options(
boost::asio::ssl::context::default_workarounds |
boost::asio::ssl::context::no_compression |
Copy link
Member Author

Choose a reason for hiding this comment

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

MS seems to have disabled compression by default, so we don't need/have an equivalent here.

Copy link
Member Author

Choose a reason for hiding this comment

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

GnuTLS 3.6.0 and even OpenSSL 1.1.1 disabled compression, too.

lib/base/tlsutility.cpp
{
char errbuf[256];

// Enforce TLS v1.2 as minimum
context->set_options(
boost::asio::ssl::context::default_workarounds |
Copy link
Member Author

Choose a reason for hiding this comment

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

This is SSL_OP_ALL, which workarounds bugs in foreign clients, e.g. SSL_OP_SAFARI_ECDHE_ECDSA_BUG. If nobody's against, I'd omit this on Windows (given there's an equivalent at all).

@Al2Klimov Al2Klimov changed the title WIP Windows: use (existing) Schannel, not (shipped) OpenSSL Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Al2Klimov Al2Klimov

Labels
area/api REST API area/distributed Distributed monitoring (master, satellites, clients) area/windows Windows agent and plugins cla/signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

On Windows use SChannel, not OpenSSL, for TLS connections
2 participants
@Al2Klimov @laudrup