Attachment 316749 Details for Bug 459607 – smaller patch to fix ssl shutdown

[patch] smaller patch to fix ssl shutdown

vsftpd-2.0.7-fix_ssl.patch (text/plain), 5.77 KB, created by Justin Payne on 2008-09-15 13:43:40 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Justin Payne

Created: 2008-09-15 13:43:40 UTC

Size: 5.77 KB

patch

obsolete

>diff -up fix/tunables.c.fix fix/tunables.c
>--- fix/tunables.c.fix	2008-09-05 15:24:02.000000000 -0400
>+++ fix/tunables.c	2008-09-05 15:53:29.000000000 -0400
>@@ -72,6 +72,8 @@ int tunable_mdtm_write = 1;
> int tunable_lock_upload_files = 1;
> int tunable_pasv_addr_resolve = 0;
> int tunable_userlist_log = 0;
>+int tunable_strict_ssl_read_eof = 0;
>+int tunable_strict_ssl_write_shutdown = 0;
> 
> unsigned int tunable_accept_timeout = 60;
> unsigned int tunable_connect_timeout = 60;
>diff -up fix/tunables.h.fix fix/tunables.h
>--- fix/tunables.h.fix	2008-09-05 15:24:54.000000000 -0400
>+++ fix/tunables.h	2008-09-05 15:54:11.000000000 -0400
>@@ -68,6 +68,8 @@ extern int tunable_mdtm_write;          
> extern int tunable_lock_upload_files;         /* Lock uploading files */
> extern int tunable_pasv_addr_resolve;         /* DNS resolve pasv_addr */
> extern int tunable_userlist_log;           /* Log every failed login attempt */
>+extern int tunable_strict_ssl_read_eof;       /* Need SSL_shutdown() on read */
>+extern int tunable_strict_ssl_write_shutdown; /* Need SSL_shutdown() on write */
> 
> /* Integer/numeric defines */
> extern unsigned int tunable_accept_timeout;
>diff -up fix/parseconf.c.fix fix/parseconf.c
>--- fix/parseconf.c.fix	2008-09-05 15:23:24.000000000 -0400
>+++ fix/parseconf.c	2008-09-05 15:54:41.000000000 -0400
>@@ -100,6 +100,8 @@ parseconf_bool_array[] =
>   { "lock_upload_files", &tunable_lock_upload_files },
>   { "pasv_addr_resolve", &tunable_pasv_addr_resolve },
>   { "userlist_log", &tunable_userlist_log },
>+  { "strict_ssl_read_eof", &tunable_strict_ssl_read_eof },
>+  { "strict_ssl_write_shutdown", &tunable_strict_ssl_write_shutdown },
>   { 0, 0 }
> };
> 
>diff -up fix/ssl.c.fix fix/ssl.c
>--- fix/ssl.c.fix	2008-09-05 14:37:49.000000000 -0400
>+++ fix/ssl.c	2008-09-05 21:33:02.000000000 -0400
>@@ -34,8 +34,11 @@ static int ssl_session_init(struct vsf_s
> static void setup_bio_callbacks();
> static long bio_callback(
>   BIO* p_bio, int oper, const char* p_arg, int argi, long argl, long retval);
>+static void maybe_log_shutdown_state(struct vsf_session* p_sess);
>+static void maybe_log_ssl_error_state(struct vsf_session* p_sess, int ret);
> 
> static int ssl_inited;
>+static struct mystr debug_str;
> 
> void
> ssl_init(struct vsf_session* p_sess)
>@@ -209,10 +212,11 @@ ssl_getline(const struct vsf_session* p_
> }
> 
> int
>-ssl_read(void* p_ssl, char* p_buf, unsigned int len)
>+ssl_read(struct vsf_session* p_sess, char* p_buf, unsigned int len)
> {
>   int retval;
>   int err;
>+  SSL* p_ssl = p_sess->p_data_ssl;
>   do
>   {
>     retval = SSL_read((SSL*) p_ssl, p_buf, len);
>@@ -220,6 +224,18 @@ ssl_read(void* p_ssl, char* p_buf, unsig
>   }
>   while (retval < 0 && (err == SSL_ERROR_WANT_READ ||
>                         err == SSL_ERROR_WANT_WRITE));
>+  // If we hit an EOF, make sure it was from the peer, not injected by the
>+  // attacker.
>+  if (retval == 0 && SSL_get_shutdown(p_ssl) != SSL_RECEIVED_SHUTDOWN)
>+  {
>+      str_alloc_text(&debug_str, "Connection terminated without SSL shutdown "
>+              "- buggy client?");
>+      vsf_log_line(p_sess, kVSFLogEntryDebug, &debug_str);
>+      if (tunable_strict_ssl_read_eof)
>+      {
>+          return -1;
>+      }
>+  }
>   return retval;
> }
> 
>@@ -263,10 +279,51 @@ ssl_accept(struct vsf_session* p_sess, i
>   return 1;
> }
> 
>-void
>+int
> ssl_data_close(struct vsf_session* p_sess)
> {
>-  SSL_free(p_sess->p_data_ssl);
>+    int success = 1;
>+    SSL* p_ssl = p_sess->p_data_ssl;
>+    if (p_ssl)
>+    {
>+        int ret;
>+        maybe_log_shutdown_state(p_sess);
>+        // This is a mess. Ideally, when we're the sender, we'd like to get to the
>+        // SSL_RECEIVED_SHUTDOWN state to get a cryptographic guarantee that the
>+        // peer received all the data and shut the connection down cleanly. It
>+        // doesn't matter hugely apart from logging, but it's a nagging detail.
>+        // Unfortunately, no FTP client I found was able to get sends into that
>+        // state, so the best we can do is issue SSL_shutdown but not check the
>+        // errors / returns. At least this enables the receiver to be sure of the
>+        // integrity of the send in terms of unwanted truncation.
>+        ret = SSL_shutdown(p_ssl);
>+        maybe_log_shutdown_state(p_sess);
>+        if (ret == 0)
>+        {
>+            ret = SSL_shutdown(p_ssl);
>+            maybe_log_shutdown_state(p_sess);
>+            if (ret != 1)
>+            {
>+                if (tunable_strict_ssl_write_shutdown)
>+                {
>+                    success = 0;
>+                }
>+                maybe_log_shutdown_state(p_sess);
>+                maybe_log_ssl_error_state(p_sess, ret);
>+            }
>+        }
>+        else if (ret < 0)
>+        {
>+            if (tunable_strict_ssl_write_shutdown)
>+            {
>+                success = 0;
>+            }
>+            maybe_log_ssl_error_state(p_sess, ret);
>+        }
>+        SSL_free(p_ssl);
>+        p_sess->p_data_ssl = NULL;
>+    }
>+    return success;
> }
> 
> void
>diff -up fix/ssl.h.fix fix/ssl.h
>--- fix/ssl.h.fix	2008-09-05 16:58:46.000000000 -0400
>+++ fix/ssl.h	2008-09-05 21:35:57.000000000 -0400
>@@ -6,12 +6,12 @@ struct mystr;
> 
> void ssl_getline(const struct vsf_session* p_sess, struct mystr* p_str,
>                  char end_char, char* p_buf, unsigned int buflen);
>-int ssl_read(void* p_ssl, char* p_buf, unsigned int len);
>+int ssl_read(struct vsf_session* p_sess, char* p_buf, unsigned int len);
> int ssl_write(void* p_ssl, const char* p_buf, unsigned int len);
> int ssl_write_str(void* p_ssl, const struct mystr* p_str);
> void ssl_init(struct vsf_session* p_sess);
> int ssl_accept(struct vsf_session* p_sess, int fd);
>-void ssl_data_close(struct vsf_session* p_sess);
>+int ssl_data_close(struct vsf_session* p_sess);
> void ssl_comm_channel_init(struct vsf_session* p_sess);
> void handle_auth(struct vsf_session* p_sess);
> void handle_pbsz(struct vsf_session* p_sess);

Actions: View | Diff

Attachments on bug 459607: 314952 | 316749 | 317331 | 333868