Skip to content

Commit 47abdb3

Browse files
cgzonescgzones
committed
Fix SSL verification
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).
1 parent a781b84 commit 47abdb3

File tree

4 files changed

+61
-76
lines changed

4 files changed

+61
-76
lines changed

‎src/netlog/netlog-dtls.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,7 @@ int dtls_manager_init(OpenSSLCertificateAuthMode auth_mode, DTLSManager **ret) {
202202
"DTLS: Failed to allocate memory for SSL CTX: %m");
203203

204204
SSL_CTX_set_default_verify_paths(ctx);
205+
SSL_CTX_set_verify_depth(ctx, VERIFICATION_DEPTH + 1);
205206

206207
m = new(DTLSManager, 1);
207208
if (!m)

‎src/netlog/netlog-ssl.c‎

Lines changed: 56 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -6,95 +6,76 @@
66
#include "openssl-util.h"
77
#include "socket-util.h"
88

9+
#include "netlog-dtls.h"
910
#include "netlog-tls.h"
1011

11-
int ssl_verify_certificate_validity(int s, X509_STORE_CTX *store) {
12-
SSL* ssl = X509_STORE_CTX_get_ex_data(store, SSL_get_ex_data_X509_STORE_CTX_idx());
12+
static_assert(offsetof(TLSManager, auth_mode) == offsetof(DTLSManager, auth_mode));
13+
14+
/* inspired by SSL_set_verify(3) */
15+
int ssl_verify_certificate_validity(int preverify_ok, X509_STORE_CTX *store) {
16+
const SSL* ssl = X509_STORE_CTX_get_ex_data(store, SSL_get_ex_data_X509_STORE_CTX_idx());
1317
const char *pretty = (const char *) SSL_get_ex_data(ssl, EX_DATA_PRETTYADDRESS);
14-
_cleanup_(OPENSSL_freep) void *subject = NULL, *issuer = NULL;
15-
TLSManager *m = (TLSManager *) SSL_get_ex_data(ssl, EX_DATA_TLSMANAGER);
16-
X509 *cert = X509_STORE_CTX_get_current_cert(store);
18+
const TLSManager *m = (const TLSManager *) SSL_get_ex_data(ssl, EX_DATA_TLSMANAGER);
19+
const X509 *error_cert = X509_STORE_CTX_get_current_cert(store);
1720
int depth = X509_STORE_CTX_get_error_depth(store);
1821
int error = X509_STORE_CTX_get_error(store);
19-
int verify_mode = SSL_get_verify_mode(ssl);
20-
long rc;
22+
char subject_buf[256];
23+
char issuer_buf[256];
24+
int log_level;
2125

2226
assert(store);
27+
assert(pretty);
28+
assert(m);
2329

24-
log_debug("TLS: Verifying SSL certificates of server: %s", pretty);
25-
26-
if (cert) {
27-
subject = X509_NAME_oneline(X509_get_subject_name(cert), 0, 0);
28-
issuer = X509_NAME_oneline(X509_get_issuer_name(cert), 0, 0);
29-
}
30+
X509_NAME_oneline(X509_get_subject_name(error_cert), subject_buf, sizeof(subject_buf));
31+
X509_NAME_oneline(X509_get_issuer_name(error_cert), issuer_buf, sizeof(issuer_buf));
3032

31-
if (verify_mode == SSL_VERIFY_NONE) {
32-
log_debug("TLS: SSL Certificate validation DISABLED but Error at depth: %d, issuer=%s, subject=%s: server=%s %s",
33-
depth, (char *) subject, (char *) issuer, pretty, X509_verify_cert_error_string(error));
33+
log_debug("TLS: Verifying SSL certificates of server %s: certificate: subject='%s' issuer='%s' depth=%d preverify_ok=%d error=%d/%s ...",
34+
pretty, subject_buf, issuer_buf, depth, preverify_ok, error, X509_verify_cert_error_string(error));
3435

35-
return 1;
36+
if (depth > VERIFICATION_DEPTH) {
37+
/*
38+
* From man:SSL_set_verif(3):
39+
*
40+
* Catch a too long certificate chain. The depth limit set using
41+
* SSL_CTX_set_verify_depth() is by purpose set to "limit+1" so
42+
* that whenever the "depth>verify_depth" condition is met, we
43+
* have violated the limit and want to log this error condition.
44+
* We must do it here, because the CHAIN_TOO_LONG error would not
45+
* be found explicitly; only errors introduced by cutting off the
46+
* additional certificates would be logged.
47+
*/
48+
preverify_ok = 0;
49+
error = X509_V_ERR_CERT_CHAIN_TOO_LONG;
50+
X509_STORE_CTX_set_error(store, error);
3651
}
3752

38-
rc = SSL_get_verify_result(ssl);
39-
if (rc != X509_V_OK) {
40-
switch(rc) {
41-
case X509_V_ERR_CERT_HAS_EXPIRED: {
42-
switch (m->auth_mode) {
43-
case OPEN_SSL_CERTIFICATE_AUTH_MODE_DENY: {
44-
log_error_errno(SYNTHETIC_ERRNO(EINVAL),
45-
"TLS: Failed to verify certificate server=%s: %s", pretty, X509_verify_cert_error_string(rc));
46-
return 0;
47-
}
48-
break;
49-
case OPEN_SSL_CERTIFICATE_AUTH_MODE_WARN: {
50-
log_warning_errno(SYNTHETIC_ERRNO(EINVAL),
51-
"TLS: Failed to verify certificate server=%s: %s", pretty, X509_verify_cert_error_string(rc));
52-
53-
return 1;
54-
}
55-
break;
56-
case OPEN_SSL_CERTIFICATE_AUTH_MODE_ALLOW: {
57-
log_debug("TLS: Failed to verify certificate server=%s: %s", pretty, X509_verify_cert_error_string(rc));
58-
return 1;
59-
}
60-
61-
break;
62-
default:
63-
break;
64-
}}
65-
break;
66-
case X509_V_ERR_CERT_REVOKED: {
67-
switch (m->auth_mode) {
68-
case OPEN_SSL_CERTIFICATE_AUTH_MODE_DENY: {
69-
log_error_errno(SYNTHETIC_ERRNO(EINVAL),
70-
"TLS: Failed to verify certificate server=%s: %s", pretty, X509_verify_cert_error_string(rc));
71-
return 0;
72-
}
73-
break;
74-
case OPEN_SSL_CERTIFICATE_AUTH_MODE_WARN: {
75-
log_warning_errno(SYNTHETIC_ERRNO(EINVAL),
76-
"TLS: Failed to verify certificate server=%s: %s", pretty, X509_verify_cert_error_string(rc));
53+
if (preverify_ok) {
54+
log_debug("TLS: Verified SSL certificate of server=%s (certificate: subject='%s' issuer='%s' depth=%d): %s",
55+
pretty, subject_buf, issuer_buf, depth, X509_verify_cert_error_string(error));
56+
return preverify_ok;
57+
}
7758

78-
return 1;
79-
}
80-
break;
81-
case OPEN_SSL_CERTIFICATE_AUTH_MODE_ALLOW: {
82-
log_debug_errno(SYNTHETIC_ERRNO(EINVAL),
83-
"TLS: Failed to verify certificate server=%s: %s", pretty, X509_verify_cert_error_string(rc));
84-
return 1;
85-
}
86-
break;
87-
default:
88-
break;
89-
}}
90-
break;
91-
default:
92-
log_error("TLS: Failed to validate remote certificate server=%s: %s. Aborting connection ...", pretty, X509_verify_cert_error_string(rc));
93-
return 0;
94-
}
59+
switch (m->auth_mode) {
60+
case OPEN_SSL_CERTIFICATE_AUTH_MODE_DENY:
61+
log_level = LOG_ERR;
62+
break;
63+
case OPEN_SSL_CERTIFICATE_AUTH_MODE_WARN:
64+
log_level = LOG_WARNING;
65+
preverify_ok = 1;
66+
break;
67+
case OPEN_SSL_CERTIFICATE_AUTH_MODE_ALLOW:
68+
log_level = LOG_DEBUG;
69+
preverify_ok = 1;
70+
break;
71+
default:
72+
assert_not_reached("Invalid certificate authentication mode"); /* mode NO does not set this callback */
9573
}
9674

97-
log_debug("TLS: SSL certificates verified server=%s: %s", pretty, X509_verify_cert_error_string(rc));
75+
log_full(log_level, "TLS: Failed to verify certificate of server=%s (certificate: subject='%s' issuer='%s' depth=%d)%s: %s",
76+
pretty, subject_buf, issuer_buf, depth,
77+
preverify_ok ? ", ignoring" : "",
78+
X509_verify_cert_error_string(error));
9879

99-
return 1;
80+
return preverify_ok;
10081
}

‎src/netlog/netlog-ssl.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,11 @@
55

66
#include "macro.h"
77

8+
#define VERIFICATION_DEPTH 20
9+
810
#define EX_DATA_TLSMANAGER 0
911
#define EX_DATA_PRETTYADDRESS 1
1012

11-
int ssl_verify_certificate_validity(int status, X509_STORE_CTX *store);
13+
int ssl_verify_certificate_validity(int preverify_ok, X509_STORE_CTX *store);
1214

1315
DEFINE_TRIVIAL_CLEANUP_FUNC(SSL_CTX*, SSL_CTX_free);

‎src/netlog/netlog-tls.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,7 @@ int tls_manager_init(OpenSSLCertificateAuthMode auth, TLSManager **ret ) {
202202
"TLS: Failed to allocate memory for SSL CTX: %m");
203203

204204
SSL_CTX_set_default_verify_paths(ctx);
205+
SSL_CTX_set_verify_depth(ctx, VERIFICATION_DEPTH + 1);
205206

206207
m = new(TLSManager, 1);
207208
if (!m)

0 commit comments

Comments
 (0)