Skip to content

Commit 4bc3dd2

Browse files
schwabeJenkins-dev
authored andcommitted
Also print key agreement when printing negotiated details
With TLS 1.0 to 1.2, the used key agreement was depended on the certificates themselves. With TLS 1.3 this is no longer the case but basically always X25519 was used. So this information has not been very interesting so far. With OpenSSL 3.5.0 and the new X25519MLKEM768 hybrid key agreement, the used key agreement group actually becomes interesting information. This commit adds printing this information for OpenSSL 3.0.0+ and uses a compat version for OpenSSL 3.0-3.1 to avoid an additional ifdef in the code itself. This also renames SSL Handshake to TLS Handshake since we never actually supported SSL and renames c_ssl to ssl as it is no longer const. Jira: OVPN3-1341 Signed-off-by: Arne Schwabe <[email protected]>
1 parent 57eba59 commit 4bc3dd2

File tree

3 files changed

+30
-9
lines changed

3 files changed

+30
-9
lines changed

openvpn/openssl/compat.hpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,3 +111,13 @@ EVP_MD_free(const EVP_MD *md)
111111
}
112112

113113
#endif
114+
#if OPENSSL_VERSION_NUMBER < 0x30200000L && OPENSSL_VERSION_NUMBER >= 0x30000000L
115+
static inline const char *
116+
SSL_get0_group_name(SSL *s)
117+
{
118+
/* int is the return type according the manual page but gcc complains that
119+
* this a long to int conversion. So explicitly cast to int */
120+
int nid = static_cast<int>(SSL_get_negotiated_group(s));
121+
return SSL_group_to_name(s, nid);
122+
}
123+
#endif

openvpn/openssl/ssl/sslctx.hpp

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1054,11 +1054,11 @@ class OpenSSLContext : public SSLFactoryAPI
10541054
}
10551055

10561056
// Print a one line summary of SSL/TLS session handshake.
1057-
static std::string ssl_handshake_details(const ::SSL *c_ssl)
1057+
static std::string ssl_handshake_details(::SSL *ssl)
10581058
{
10591059
std::ostringstream os;
10601060

1061-
::X509 *cert = SSL_get_peer_certificate(c_ssl);
1061+
::X509 *cert = SSL_get_peer_certificate(ssl);
10621062

10631063
if (cert)
10641064
os << "peer certificate: CN=" << OpenSSLPKI::x509_get_field(cert, NID_commonName);
@@ -1100,7 +1100,7 @@ class OpenSSLContext : public SSLFactoryAPI
11001100
X509_free(cert);
11011101
}
11021102

1103-
const SSL_CIPHER *ciph = SSL_get_current_cipher(c_ssl);
1103+
const SSL_CIPHER *ciph = SSL_get_current_cipher(ssl);
11041104
if (ciph)
11051105
{
11061106
char *desc = SSL_CIPHER_description(ciph, nullptr, 0);
@@ -1110,15 +1110,26 @@ class OpenSSLContext : public SSLFactoryAPI
11101110
}
11111111
else
11121112
{
1113-
os << ", cipher: " << desc;
1113+
std::string cipher_str(desc);
1114+
// remove \n at the end and any other excess whitespace
1115+
string::trim(cipher_str);
1116+
os << ", cipher: " << cipher_str;
11141117
OPENSSL_free(desc);
11151118
}
11161119
}
1117-
// This has been changed in upstream SSL to have a const
1118-
// parameter, so we cast away const for older versions compatibility
1119-
// (Upstream commit: c04b66b18d1a90f0c6326858e4b8367be5444582)
1120-
if (SSL_session_reused(const_cast<::SSL *>(c_ssl)))
1120+
1121+
if (SSL_session_reused(ssl))
11211122
os << " [REUSED]";
1123+
1124+
#if OPENSSL_VERSION_NUMBER >= 0x30000000L
1125+
const char *key_agreement = SSL_get0_group_name(ssl);
1126+
if (!key_agreement)
1127+
{
1128+
key_agreement = "(error fetching key-agreeement)";
1129+
}
1130+
os << ", key-agreement: " << key_agreement;
1131+
#endif
1132+
11221133
return os.str();
11231134
}
11241135

openvpn/ssl/proto.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2928,7 +2928,7 @@ class ProtoContext : public logging::LoggingMixin<OPENVPN_DEBUG_PROTO,
29282928

29292929
void active()
29302930
{
2931-
OVPN_LOG_INFO("SSL Handshake: " << Base::ssl_handshake_details());
2931+
OVPN_LOG_INFO("TLS Handshake: " << Base::ssl_handshake_details());
29322932

29332933
/* Our internal state machine only decides after push request what protocol
29342934
* options we want to use. Therefore we also have to postpone data key

0 commit comments

Comments
 (0)