[exim-cvs] More GnuTLS cleanups/fixes.

Top Page
Delete this message
Reply to this message
Author: Exim Git Commits Mailing List
Date:  
To: exim-cvs
Subject: [exim-cvs] More GnuTLS cleanups/fixes.
Gitweb: http://git.exim.org/exim.git/commitdiff/4fe99a6c7949056e1bf27f146ad604061b6a3669
Commit:     4fe99a6c7949056e1bf27f146ad604061b6a3669
Parent:     2c17bb02e213012d5d98ebac506a67b23b2cf693
Author:     Phil Pennock <pdp@???>
AuthorDate: Thu May 17 16:18:34 2012 -0400
Committer:  Phil Pennock <pdp@???>
CommitDate: Thu May 17 16:18:34 2012 -0400


    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 4e1e510..328466c 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"<unset>");


   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"<unset>");
   }


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 92a3633..0c98aeb 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 5fd15f8..a67d143 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 50da895..fc77535 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@??? 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@??? 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@??? 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 <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 8a5ba80..83dd8f4 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@???; 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 6a083d0..d9ad813 100644
--- a/test/src/client.c
+++ b/test/src/client.c
@@ -361,13 +361,14 @@ return session;
 *                 Main Program                   *
 *************************************************/


-/* Usage: client
-          <IP address>
-          <port>
-          [<outgoing interface>]
-          [<cert file>]
-          [<key file>]
-*/
+const char * const HELP_MESSAGE = "\n\
+Usage: client\n\
+          <IP address>\n\
+          <port>\n\
+          [<outgoing interface>]\n\
+          [<cert file>]\n\
+          [<key file>]\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;