From 971a8374a82bb32c8743aabffe09f17fcdbe69c8 Mon Sep 17 00:00:00 2001
From: rfm <rfm@72102866-910b-0410-8b05-ffd578937521>
Date: Thu, 21 Nov 2013 15:13:14 +0000
Subject: [PATCH] Reorganise a bit to ensure that TLS is properly shut down
 before the network connection it relies on is closed.  Needed in case
 information is buffered in the TLS layer and needs flushing to the remote end
 before shutdown.

git-svn-id: svn+ssh://svn.gna.org/svn/gnustep/libs/base/trunk@37390 72102866-910b-0410-8b05-ffd578937521
---
 ChangeLog                   |  7 +++++++
 Source/GSFileHandle.m       | 12 ++++++++++--
 Source/NSFileHandle.m       | 32 ++++++++++++++++----------------
 Source/win32/GSFileHandle.m |  5 +++++
 4 files changed, 38 insertions(+), 18 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 911b55448..5bf0db290 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2013-11-21  Richard Frith-Macdonald <rfm@gnu.org>
+
+	* Source/GSFileHandle.m: Do an -sslDisconnect while finalising
+	(before underlying network connection is closed).
+	* Source/NSFileHandle.m: Add assertion to check that TLS has been
+	shut down by concrete subclass dealloc.
+
 2013-11-20  Richard Frith-Macdonald <rfm@gnu.org>
 
 	* Source/NSUserDefaults.m: Fix for bug 40620
diff --git a/Source/GSFileHandle.m b/Source/GSFileHandle.m
index 02c63a0d4..6e898d5ca 100644
--- a/Source/GSFileHandle.m
+++ b/Source/GSFileHandle.m
@@ -214,8 +214,7 @@ static NSString*	NotificationKey = @"NSFileHandleNotificationKey";
   [self ignoreWriteDescriptor];
 
 #if	USE_ZLIB
-  /*
-   * The gzDescriptor should always be closed when we have done with it.
+  /* The gzDescriptor should always be closed when we have done with it.
    */
   if (gzDescriptor != 0)
     {
@@ -223,6 +222,14 @@ static NSString*	NotificationKey = @"NSFileHandleNotificationKey";
       gzDescriptor = 0;
     }
 #endif
+
+  /* Ensure any SSL/TLS connection has been properly shut down.
+   */
+  [self sslDisconnect];
+
+  /* Close file descriptor if necessary (setting correct non-blocking
+   * characteristics since we may have changed them).
+   */
   if (descriptor != -1)
     {
       [self setNonBlocking: wasNonBlocking];
@@ -232,6 +239,7 @@ static NSString*	NotificationKey = @"NSFileHandleNotificationKey";
 	  descriptor = -1;
 	}
     }
+  [super finalize];
 }
 
 // Initializing a GSFileHandle Object
diff --git a/Source/NSFileHandle.m b/Source/NSFileHandle.m
index b4c583440..2de11c55e 100644
--- a/Source/NSFileHandle.m
+++ b/Source/NSFileHandle.m
@@ -953,29 +953,22 @@ GSTLSHandlePush(gnutls_transport_ptr_t handle, const void *buffer, size_t len)
 
 - (void) closeFile
 {
-  [self sslDisconnect];
+  [self sslDisconnect];         // Shut down TLS before closing socket
   [super closeFile];
 }
 
 - (void) dealloc
 {
-  // TLS may need to read data during teardown, and we need to wait for it.
-  [self setNonBlocking: NO];
-  // Don't DESTROY ivars below. First release them, then set nil, because
-  // `session' may need this back-reference during TLS teardown.
-  TEST_RELEASE(opts);
-  TEST_RELEASE(session);
-  opts = nil;
-  session = nil;
+  /* Any TLS connection needs to be shut down before the network connection
+   * is closed, which means that the concrete subclass must do that.
+   * Therefore, the session should be inactive by the time we get here.
+   */
+  NSAssert(NO == [session active], NSInternalInconsistencyException);
+  DESTROY(session);
+  DESTROY(opts);
   [super dealloc];
 }
 
-- (void) finalize
-{
-  [self sslDisconnect];
-  [super finalize];
-}
-
 - (NSInteger) read: (void*)buf length: (NSUInteger)len
 {
   if (YES == [session active])
@@ -987,7 +980,14 @@ GSTLSHandlePush(gnutls_transport_ptr_t handle, const void *buffer, size_t len)
 
 - (void) sslDisconnect
 {
-  [session disconnect];
+  if (nil != session)
+    {
+      // TLS may need to read data during teardown, and we need to wait for it.
+      [self setNonBlocking: NO];
+      [session disconnect];
+      DESTROY(session);
+    }
+  DESTROY(opts);
 }
 
 - (BOOL) sslHandshakeEstablished: (BOOL*)result outgoing: (BOOL)isOutgoing
diff --git a/Source/win32/GSFileHandle.m b/Source/win32/GSFileHandle.m
index a297c599d..ce830a065 100644
--- a/Source/win32/GSFileHandle.m
+++ b/Source/win32/GSFileHandle.m
@@ -270,6 +270,11 @@ getAddr(NSString* name, NSString* svc, NSString* pcl, struct sockaddr_in *sin)
       gzclose(gzDescriptor);
     }
 #endif
+
+  /* Ensure any SSL/TLS connection has been properly shut down.
+   */
+  [self sslDisconnect];
+
   if (descriptor != -1)
     {
       [self setNonBlocking: wasNonBlocking];