OpenSSL updates: allow to configure or skip verification #152
Conversation
| } | ||
| // Host name verification is done by OpenSSL itself, however if we are connecting to an ip-address, | ||
| // no verification is made, so we have to do it manually. | ||
| // Just in case if this is ever required, leave it here commented out. |
There was a problem hiding this comment.
is SSL_set_verify SSL_VERIFY_PEER not needed?
https://www.openssl.org/docs/man1.1.1/man3/SSL_set_verify.html
at also appears in the examples in https://github.com/iSECPartners/ssl-conservatory/blob/1449f3ae918623699947e24f388c531b30ad2c87/openssl/test_client.c#L115-L119
There was a problem hiding this comment.
Yep, makes sense to have some safe defaults
| @@ -143,42 +178,42 @@ SSLSocket::SSLSocket(const NetworkAddress& addr, const SSLParams & ssl_params, S | |||
| if (!ssl) | |||
| throw std::runtime_error("Failed to create SSL instance"); | |||
|
|
|||
| std::unique_ptr<ASN1_OCTET_STRING, decltype(&ASN1_OCTET_STRING_free)> ip_addr(a2i_IPADDRESS(addr.Host().c_str()), &ASN1_OCTET_STRING_free); | |||
|
|
|||
| HANDLE_SSL_ERROR(ssl, SSL_set_fd(ssl, handle_)); | |||
| if (ssl_params.use_SNI) | |||
There was a problem hiding this comment.
sidenote: maybe that is not needed at all, it sounds safe to call SSL_set_tlsext_host_name always.
| /** Mode of verifying host ssl certificate against name of the host, set with SSL_set_hostflags. | ||
| * For details see https://www.openssl.org/docs/man1.1.1/man3/SSL_set_hostflags.html | ||
| */ | ||
| DECLARE_FIELD(host_flags, int, SetHostVerifyFlags, DEFAULT_VALUE); |
There was a problem hiding this comment.
Test? For example maybe we can trigger X509_CHECK_FLAG_NO_WILDCARDS? I guess it should make the github.demo.trial.altinity.cloud stop working, because it uses wildcard cert:
depth=0 CN = *.demo.trial.altinity.cloud
There was a problem hiding this comment.
Rn, it is too hard to set up any complex tests with the tools we have
But I already asked QA team to do the in-depth tests with various tricky ssl (mis)configurations. They'll do it once they have resources.
Enmk
force-pushed
the
openssl_allow_skip_verification
branch
from
219cff8 to
d2f589a
Compare
Added ClientOptions: - skip_verification - to skip all verification - host_flags - to set hostname verification mode - configuration - to configure SSL connection Updated and refactored tests. Updated tests/simple/main.cpp to reuse so utility code from unit-tests.
Enmk
force-pushed
the
openssl_allow_skip_verification
branch
from
d2f589a to
257909b
Compare
Added ClientOptions:
Updated and refactored tests.
Updated tests/simple/main.cpp to reuse so utility code from unit-tests.
Closes: #142