From 4fe99a6c7949056e1bf27f146ad604061b6a3669 Mon Sep 17 00:00:00 2001 From: Phil Pennock Date: Thu, 17 May 2012 16:18:34 -0400 Subject: [PATCH 1/1] More GnuTLS cleanups/fixes. Decided "unknown (reason)" in tls_peerdn was wrong, stripped that, added replacement guard. Moved cipherbuf construction to where it makes more sense, where peerdn is extracted, so that setting the exim vars gets back closer to just some pointer switching. Fix missing failure check after handshake in client. Fix tls.c tls_ungetc() and friends by pointing watermark vars at state content. Regenerated test-suite D-H params so we don't have too small values, which was causing connection rejections. Test-suite output where new test cert info is logged (there will be a couple more, when I fix a lingering problem with tls_peerdn being unset in client log-lines). Give test-suite client command some --help. --- src/src/tls-gnu.c | 73 +++++++++++++++++--------------- src/src/tls.c | 15 ++++++- test/aux-fixed/gnutls-params | 81 ++++++++++++++++++++++++++++++------ test/log/2002 | 2 +- test/mail/2002.CALLER | 2 +- test/src/client.c | 22 ++++++---- 6 files changed, 138 insertions(+), 57 deletions(-) diff --git a/src/src/tls-gnu.c b/src/src/tls-gnu.c index 4e1e5104b..328466cc3 100644 --- a/src/src/tls-gnu.c +++ b/src/src/tls-gnu.c @@ -76,6 +76,7 @@ typedef struct exim_gnutls_state { int fd_out; BOOL peer_cert_verified; BOOL trigger_sni_changes; + BOOL have_set_peerdn; const struct host_item *host; uschar *peerdn; uschar *received_sni; @@ -103,7 +104,7 @@ typedef struct exim_gnutls_state { } exim_gnutls_state_st; static const exim_gnutls_state_st exim_gnutls_state_init = { - NULL, NULL, NULL, VERIFY_NONE, -1, -1, FALSE, FALSE, + NULL, NULL, NULL, VERIFY_NONE, -1, -1, FALSE, FALSE, FALSE, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, @@ -297,11 +298,7 @@ Argument: static void extract_exim_vars_from_tls_state(exim_gnutls_state_st *state) { -gnutls_protocol_t protocol; gnutls_cipher_algorithm_t cipher; -gnutls_kx_algorithm_t kx; -gnutls_mac_algorithm_t mac; -uschar *p; #ifdef HAVE_GNUTLS_SESSION_CHANNEL_BINDING int old_pool; int rc; @@ -316,25 +313,6 @@ cipher = gnutls_cipher_get(state->session); /* returns size in "bytes" */ tls_bits = gnutls_cipher_get_key_size(cipher) * 8; -if (!*state->cipherbuf) - { - protocol = gnutls_protocol_get_version(state->session); - mac = gnutls_mac_get(state->session); - kx = gnutls_kx_get(state->session); - - string_format(state->cipherbuf, sizeof(state->cipherbuf), - "%s:%s:%u", - gnutls_protocol_get_name(protocol), - gnutls_cipher_suite_get_name(kx, cipher, mac), - tls_bits); - - /* I don't see a way that spaces could occur, in the current GnuTLS - code base, but it was a concern in the old code and perhaps older GnuTLS - releases did return "TLS 1.0"; play it safe, just in case. */ - for (p = state->cipherbuf; *p != '\0'; ++p) - if (isspace(*p)) - *p = '-'; - } tls_cipher = state->cipherbuf; DEBUG(D_tls) debug_printf("cipher: %s\n", tls_cipher); @@ -994,7 +972,8 @@ return OK; *************************************************/ /* Called from both server and client code. -Only this is allowed to set state->peerdn and we use that to detect double-calls. +Only this is allowed to set state->peerdn and state->have_set_peerdn +and we use that to detect double-calls. Arguments: state exim_gnutls_state_st * @@ -1008,21 +987,45 @@ peer_status(exim_gnutls_state_st *state) const gnutls_datum *cert_list; int rc; unsigned int cert_list_size = 0; +gnutls_protocol_t protocol; +gnutls_cipher_algorithm_t cipher; +gnutls_kx_algorithm_t kx; +gnutls_mac_algorithm_t mac; gnutls_certificate_type_t ct; gnutls_x509_crt_t crt; -uschar *dn_buf; +uschar *p, *dn_buf; size_t sz; -if (state->peerdn) +if (state->have_set_peerdn) return OK; +state->have_set_peerdn = TRUE; -state->peerdn = US"unknown"; +state->peerdn = NULL; +/* tls_cipher */ +cipher = gnutls_cipher_get(state->session); +protocol = gnutls_protocol_get_version(state->session); +mac = gnutls_mac_get(state->session); +kx = gnutls_kx_get(state->session); + +string_format(state->cipherbuf, sizeof(state->cipherbuf), + "%s:%s:%d", + gnutls_protocol_get_name(protocol), + gnutls_cipher_suite_get_name(kx, cipher, mac), + (int) gnutls_cipher_get_key_size(cipher) * 8); + +/* I don't see a way that spaces could occur, in the current GnuTLS +code base, but it was a concern in the old code and perhaps older GnuTLS +releases did return "TLS 1.0"; play it safe, just in case. */ +for (p = state->cipherbuf; *p != '\0'; ++p) + if (isspace(*p)) + *p = '-'; + +/* tls_peerdn */ cert_list = gnutls_certificate_get_peers(state->session, &cert_list_size); if (cert_list == NULL || cert_list_size == 0) { - state->peerdn = US"unknown (no certificate)"; DEBUG(D_tls) debug_printf("TLS: no certificate from peer (%p & %d)\n", cert_list, cert_list_size); if (state->verify_requirement == VERIFY_REQUIRED) @@ -1035,7 +1038,6 @@ ct = gnutls_certificate_type_get(state->session); if (ct != GNUTLS_CRT_X509) { const char *ctn = gnutls_certificate_type_get_name(ct); - state->peerdn = string_sprintf("unknown (type %s)", ctn); DEBUG(D_tls) debug_printf("TLS: peer cert not X.509 but instead \"%s\"\n", ctn); if (state->verify_requirement == VERIFY_REQUIRED) @@ -1122,7 +1124,7 @@ if ((rc < 0) || (verify & (GNUTLS_CERT_INVALID|GNUTLS_CERT_REVOKED)) != 0) DEBUG(D_tls) debug_printf("TLS certificate verification failed (%s): peerdn=%s\n", - *error, state->peerdn); + *error, state->peerdn ? state->peerdn : US""); if (state->verify_requirement == VERIFY_REQUIRED) { @@ -1135,7 +1137,8 @@ if ((rc < 0) || (verify & (GNUTLS_CERT_INVALID|GNUTLS_CERT_REVOKED)) != 0) else { state->peer_cert_verified = TRUE; - DEBUG(D_tls) debug_printf("TLS certificate verified: peerdn=%s\n", state->peerdn); + DEBUG(D_tls) debug_printf("TLS certificate verified: peerdn=%s\n", + state->peerdn ? state->peerdn : US""); } tls_peerdn = state->peerdn; @@ -1479,6 +1482,10 @@ do } while ((rc == GNUTLS_E_AGAIN) || (rc == GNUTLS_E_INTERRUPTED)); alarm(0); +if (rc != GNUTLS_E_SUCCESS) + return tls_error(US"gnutls_handshake", + sigalrm_seen ? "timed out" : gnutls_strerror(rc), state->host); + DEBUG(D_tls) debug_printf("gnutls_handshake was successful\n"); /* Verify late */ @@ -1492,7 +1499,7 @@ if (state->verify_requirement != VERIFY_NONE && rc = peer_status(state); if (rc != OK) return rc; -/* Sets various Exim expansion variables; always safe within server */ +/* Sets various Exim expansion variables; may need to adjust for ACL callouts */ extract_exim_vars_from_tls_state(state); diff --git a/src/src/tls.c b/src/src/tls.c index 92a36332d..0c98aeba9 100644 --- a/src/src/tls.c +++ b/src/src/tls.c @@ -30,15 +30,19 @@ static void dummy(int x) { dummy(x-1); } #else /* Static variables that are used for buffering data by both sets of -functions and the common functions below. */ +functions and the common functions below. +We're moving away from this; GnuTLS is already using a state, which +can switch, so we can do TLS callouts during ACLs. */ -static uschar *ssl_xfer_buffer = NULL; static const int ssl_xfer_buffer_size = 4096; +#ifndef USE_GNUTLS +static uschar *ssl_xfer_buffer = NULL; static int ssl_xfer_buffer_lwm = 0; static int ssl_xfer_buffer_hwm = 0; static int ssl_xfer_eof = 0; static int ssl_xfer_error = 0; +#endif uschar *tls_channelbinding_b64 = NULL; @@ -81,6 +85,13 @@ return TRUE; #ifdef USE_GNUTLS #include "tls-gnu.c" + +#define ssl_xfer_buffer (current_global_tls_state->xfer_buffer) +#define ssl_xfer_buffer_lwm (current_global_tls_state->xfer_buffer_lwm) +#define ssl_xfer_buffer_hwm (current_global_tls_state->xfer_buffer_hwm) +#define ssl_xfer_eof (current_global_tls_state->xfer_eof) +#define ssl_xfer_error (current_global_tls_state->xfer_error) + #else #include "tls-openssl.c" #endif diff --git a/test/aux-fixed/gnutls-params b/test/aux-fixed/gnutls-params index 5fd15f841..a67d1432b 100644 --- a/test/aux-fixed/gnutls-params +++ b/test/aux-fixed/gnutls-params @@ -1,16 +1,71 @@ ------BEGIN RSA PRIVATE KEY----- -MIIBOgIBAAJBANaJrAW82pGvpnCZtUm1gGYBkQU7IT4FHuBu/f6TaakRt2Tl6jPm -STeFY7HCxeKO+NaxrRqGj+77bdW1McEaPg8CAwEAAQJAUC8Dft9/d40FcbdZVRPD -yhxSxfg8K/CBAlQplXEmQBxiJ7zDsdqJC2C8qO/HYzgLNNKKMFsq+SkiwRuP0ZoH -DQIhAN/aWQpj1Z7MhNervDKNx3mVbsJb59Cw51Z7TE8CpU/NAiEA9VjdkywEyJox -MTh5kWx/0USTvf+Tm5Lr1BCivrocUUsCIFL8uZxPWf5gml6Fd5QF2uW34nTS0qeF -2AE4s6OGtf0NAiEA31nePV0S8lHQUuxqiNMjBylbVjPFzLDIJ3HKQWQZ8wcCIBRy -w144Nd8BGkUPlChqoW1y1XU43Wz5VI8g5ZFiuzPk ------END RSA PRIVATE KEY----- + +Generator: 11:3e:bd:2e:a2:c2:4d:bb:a7:b4:bf:01 + b3:73:00:b9:ae:7f:69:7f:91:69:de:8d + 02:6e:15:8e:3f:47:19:75:bb:1a:a4:61 + 5c:77:59:0b:ca:76:93:72:c9:32:66:71 + 6a:96:16:ba:1e:92:ca:d9:92:6a:99:7f + 82:df:b1:5f:90:cf:3f:a0:24:c9:c4:d4 + 30:91:30:21:07:da:93:69:52:44:3b:68 + e2:2a:53:64:7f:82:07:37:b2:a8:b8:0c + 27:b6:e0:b8:62:25:5b:53:10:43:44:cb + e7:72:1b:4a:36:b1:6e:f9:c2:08:ff:93 + 87:ba:d3:3c:d4:4d:26:6a:eb:2e:2f:87 + 13:56:db:55:8f:9a:04:e5:d0:89:a5:4e + 24:90:a2:6b:30:2d:13:49:7f:ca:61:74 + ad:aa:0d:e6:ef:07:87:aa:56:28:64:f3 + dd:8a:1f:8b:b6:a7:a4:82:e3:5c:7f:76 + c8:42:31:e2:4b:41:bf:00:16:19:a2:ed + be:85:9a:8e:82:c4:09:9d:09:fb:53:3e + bf:67:d8:c9:01:1c:e3:b2:d3:f3:40:e2 + 93:61:6a:93:fd:b6:72:56:63:f7:56:9c + 08:28:44:3f:f2:bf:9e:97:d8:53:b0:c4 + 74:11:0e:c0:c8:9f:eb:6a:aa:37:25:a7 + 15:91:d8:82:5d:36:5e:81:29:15:2c:a7 + d4:f0:74:43:b0:fb:9a:a4:88:c8:79:b0 + f6:39:58:6c:ed:95:f2:00:ef:03:cd:8f + 41:28:6a:e7:8a:b1:48:75:6a:72:e0:78 + 30:a0:8f:a1 + +Prime: 4c:de:d0:89:11:fc:b9:15:66:77:7a:b2 + af:f9:eb:f2:b3:33:50:44:d7:3d:e6:8b + e6:3b:a2:b0:b3:0d:62:64:e1:5e:22:ca + 91:43:a2:ff:b7:bf:49:13:12:5d:8c:b9 + ad:65:b7:5f:3a:98:69:a3:a3:06:4c:a6 + 54:87:a6:35:d5:a7:a0:3d:cb:95:50:87 + 42:47:47:c5:42:62:4a:b8:f3:fe:da:14 + 6e:33:d0:26:c7:44:8a:49:a9:a5:89:cc + e9:37:6c:05:3f:ec:f6:a1:c2:06:40:7d + 03:1b:d0:1c:31:c8:0c:14:84:bc:dc:db + f0:b1:22:b0:62:4d:43:f9:4d:c5:b3:13 + 5f:52:0a:82:eb:fa:ed:d2:f2:d4:4b:fc + 17:17:3b:6e:aa:02:d6:da:73:1b:de:08 + a7:29:f7:f6:2f:a8:cd:8e:fb:8a:84:4c + 8d:42:46:3c:ba:dc:5f:e6:b3:00:bc:7e + 74:08:c4:5a:51:3b:56:e9:09:25:8a:d1 + 8a:15:b8:42:6e:88:3d:f8:a2:b4:12:5e + 33:91:f2:b5:bf:33:48:b9:77:72:41:22 + 50:1e:94:53:4f:78:d4:1d:4e:7e:b0:61 + 50:7e:e9:4c:e3:89:81:b4:ba:3e:69:4b + cb:53:67:eb:cd:ab:00:ab:0d:80:d5:6c + f2:d2:96:0d:fb:6f:48:30:df:b4:4d:20 + 17:a4:6f:71:9e:d7:08:36:24:ed:93:14 + 26:73:ac:29:59:61:d8:29:7e:e8:54:2e + aa:f8:0b:35:2c:1b:57:d3:6a:e4:b4:01 + 0d:7e:6d:0f + -----BEGIN DH PARAMETERS----- -MGUCYKCtXam0x/2mj+EibbOu+m/WAR33VA+YHPYQZuqr6PrwYnUcex5Hm4/QNsGy -b0o6BgckIFopfTgrUUANGuOlqAbGAwfzV2FxnEorKXTCP36hBFSWtFDbEcFVxQqr -jfVLwwIBBg== +MIICaAKCATBM3tCJEfy5FWZ3erKv+evyszNQRNc95ovmO6Kwsw1iZOFeIsqRQ6L/ +t79JExJdjLmtZbdfOphpo6MGTKZUh6Y11aegPcuVUIdCR0fFQmJKuPP+2hRuM9Am +x0SKSamliczpN2wFP+z2ocIGQH0DG9AcMcgMFIS83NvwsSKwYk1D+U3FsxNfUgqC +6/rt0vLUS/wXFztuqgLW2nMb3ginKff2L6jNjvuKhEyNQkY8utxf5rMAvH50CMRa +UTtW6QklitGKFbhCbog9+KK0El4zkfK1vzNIuXdyQSJQHpRTT3jUHU5+sGFQfulM +44mBtLo+aUvLU2frzasAqw2A1Wzy0pYN+29IMN+0TSAXpG9xntcINiTtkxQmc6wp +WWHYKX7oVC6q+As1LBtX02rktAENfm0PAoIBMBE+vS6iwk27p7S/AbNzALmuf2l/ +kWnejQJuFY4/Rxl1uxqkYVx3WQvKdpNyyTJmcWqWFroeksrZkmqZf4LfsV+Qzz+g +JMnE1DCRMCEH2pNpUkQ7aOIqU2R/ggc3sqi4DCe24LhiJVtTEENEy+dyG0o2sW75 +wgj/k4e60zzUTSZq6y4vhxNW21WPmgTl0ImlTiSQomswLRNJf8phdK2qDebvB4eq +Vihk892KH4u2p6SC41x/dshCMeJLQb8AFhmi7b6Fmo6CxAmdCftTPr9n2MkBHOOy +0/NA4pNhapP9tnJWY/dWnAgoRD/yv56X2FOwxHQRDsDIn+tqqjclpxWR2IJdNl6B +KRUsp9TwdEOw+5qkiMh5sPY5WGztlfIA7wPNj0EoaueKsUh1anLgeDCgj6E= -----END DH PARAMETERS----- - diff --git a/test/log/2002 b/test/log/2002 index 50da89588..fc77535db 100644 --- a/test/log/2002 +++ b/test/log/2002 @@ -1,7 +1,7 @@ 1999-03-02 09:44:33 exim x.yz daemon started: pid=pppp, no queue runs, listening for SMTP on port 1225 1999-03-02 09:44:33 10HmaX-0005vi-00 <= CALLER@test.ex H=[127.0.0.1] P=smtps X=TLS1.0:RSA_AES_256_CBC_SHA1:256 S=sss 1999-03-02 09:44:33 TLS error on connection from (rhu.barb) [ip4.ip4.ip4.ip4] (gnutls_handshake): The peer did not send any certificate. -1999-03-02 09:44:33 10HmaY-0005vi-00 <= CALLER@test.ex H=[ip4.ip4.ip4.ip4] P=smtps X=TLS1.0:RSA_AES_256_CBC_SHA1:256 DN="C=UK,L=Cambridge,O=University of Cambridge,OU=Computing Service,CN=Philip Hazel" S=sss +1999-03-02 09:44:33 10HmaY-0005vi-00 <= CALLER@test.ex H=[ip4.ip4.ip4.ip4] P=smtps X=TLS1.0:RSA_AES_256_CBC_SHA1:256 DN="C=UK,O=The Exim Maintainers,OU=Test Suite,CN=Phil Pennock" S=sss 1999-03-02 09:44:33 Start queue run: pid=pppp -qf 1999-03-02 09:44:33 10HmaX-0005vi-00 => CALLER R=abc T=local_delivery 1999-03-02 09:44:33 10HmaX-0005vi-00 Completed diff --git a/test/mail/2002.CALLER b/test/mail/2002.CALLER index 8a5ba80f7..83dd8f48f 100644 --- a/test/mail/2002.CALLER +++ b/test/mail/2002.CALLER @@ -18,7 +18,7 @@ Received: from [ip4.ip4.ip4.ip4] id 10HmaY-0005vi-00 for CALLER@test.ex; Tue, 2 Mar 1999 09:44:33 +0000 tls-certificate-verified: 1 -TLS: cipher=TLS1.0:RSA_AES_256_CBC_SHA1:256 peerdn=C=UK,L=Cambridge,O=University of Cambridge,OU=Computing Service,CN=Philip Hazel +TLS: cipher=TLS1.0:RSA_AES_256_CBC_SHA1:256 peerdn=C=UK,O=The Exim Maintainers,OU=Test Suite,CN=Phil Pennock This is a test encrypted message from a verified host. diff --git a/test/src/client.c b/test/src/client.c index 6a083d055..d9ad8139f 100644 --- a/test/src/client.c +++ b/test/src/client.c @@ -361,13 +361,14 @@ return session; * Main Program * *************************************************/ -/* Usage: client - - - [] - [] - [] -*/ +const char * const HELP_MESSAGE = "\n\ +Usage: client\n\ + \n\ + \n\ + []\n\ + []\n\ + []\n\ +\n"; int main(int argc, char **argv) { @@ -403,6 +404,13 @@ unsigned char *inptr = inbuffer; while (argc >= argi + 1 && argv[argi][0] == '-') { + if (strcmp(argv[argi], "-help") == 0 || + strcmp(argv[argi], "--help") == 0 || + strcmp(argv[argi], "-h") == 0) + { + printf(HELP_MESSAGE); + exit(0); + } if (strcmp(argv[argi], "-tls-on-connect") == 0) { tls_on_connect = 1; -- 2.30.2