Track clean connection closure

The only way to see an Ok(0) from Connection::read() is via a close_notify alert.

Unclean closure (ie, just a TCP EOF without a close_notify) leads
to an `UnexpectedEof` from Connection::read().  Applications which
want to accept that can do so -- bogo_shim is such an example.

Add tests for this and IoState::peer_has_closed()

rustls/examples/internal/bogo_shim.rs

@@ -614,7 +614,6 @@ fn exec(opts: &Options, mut sess: ClientOrServer, count: usize) {
     ];
     let mut conn = net::TcpStream::connect(&addrs[..]).expect("cannot connect");
     let mut sent_shutdown = false;
-    let mut seen_eof = false;
     let mut sent_exporter = false;
     let mut quench_writes = false;
 
@@ -630,19 +629,6 @@ fn exec(opts: &Options, mut sess: ClientOrServer, count: usize) {
                 err @ Err(_) => err.expect("read failed"),
             };
 
-            if len == 0 {
-                if opts.check_close_notify {
-                    if !seen_eof {
-                        seen_eof = true;
-                    } else {
-                        quit_err(":CLOSE_WITHOUT_CLOSE_NOTIFY:");
-                    }
-                } else {
-                    println!("EOF (plain)");
-                    return;
-                }
-            }
-
             if let Err(err) = sess.process_new_packets() {
                 flush(&mut sess, &mut conn); /* send any alerts before exiting */
                 handle_err(err);
@@ -728,6 +714,13 @@ fn exec(opts: &Options, mut sess: ClientOrServer, count: usize) {
             }
             Ok(len) => len,
             Err(err) if err.kind() == io::ErrorKind::WouldBlock => 0,
+            Err(err) if err.kind() == io::ErrorKind::UnexpectedEof => {
+                if opts.check_close_notify {
+                    quit_err(":CLOSE_WITHOUT_CLOSE_NOTIFY:");
+                }
+                println!("EOF (tcp)");
+                return;
+            }
             Err(err) => panic!("unhandled read error {:?}", err),
         };
 

rustls/src/conn.rs

@@ -595,7 +595,8 @@ pub(crate) struct ConnectionCommon {
     pub(crate) suite: Option<SupportedCipherSuite>,
     pub(crate) alpn_protocol: Option<Vec<u8>>,
     /// If the peer has signaled end of stream.
-    peer_eof: bool,
+    has_received_close_notify: bool,
+    has_seen_eof: bool,
     pub(crate) traffic: bool,
     pub(crate) early_traffic: bool,
     sent_fatal_alert: bool,
@@ -622,7 +623,8 @@ impl ConnectionCommon {
             record_layer: record_layer::RecordLayer::new(),
             suite: None,
             alpn_protocol: None,
-            peer_eof: false,
+            has_received_close_notify: false,
+            has_seen_eof: false,
             traffic: false,
             early_traffic: false,
             sent_fatal_alert: false,
@@ -649,7 +651,7 @@ impl ConnectionCommon {
         IoState {
             tls_bytes_to_write: self.sendable_tls.len(),
             plaintext_bytes_to_read: self.received_plaintext.len(),
-            peer_has_closed: self.peer_eof,
+            peer_has_closed: self.has_received_close_notify,
         }
     }
 
@@ -839,7 +841,7 @@ impl ConnectionCommon {
         // In the handshake case we don't have readable plaintext before the handshake has
         // completed, but also don't want to read if we still have sendable tls.
         self.received_plaintext.is_empty()
-            && !self.peer_eof
+            && !self.connection_was_cleanly_closed()
             && (self.traffic || self.sendable_tls.is_empty())
     }
 
@@ -857,7 +859,7 @@ impl ConnectionCommon {
         // If we get a CloseNotify, make a note to declare EOF to our
         // caller.
         if alert.description == AlertDescription::CloseNotify {
-            self.peer_eof = true;
+            self.has_received_close_notify = true;
             return Ok(());
         }
 
@@ -938,9 +940,9 @@ impl ConnectionCommon {
 
     /// Are we done? i.e., have we processed all received messages,
     /// and received a close_notify to indicate that no new messages
-    /// will arrive or ran out of bytes on the incoming TLS stream?
-    fn connection_at_eof(&self) -> bool {
-        self.peer_eof && !self.message_deframer.has_pending()
+    /// will arrive?
+    fn connection_was_cleanly_closed(&self) -> bool {
+        self.has_received_close_notify && !self.message_deframer.has_pending()
     }
 
     /// Read TLS content from `rd`.  This method does internal
@@ -949,7 +951,7 @@ impl ConnectionCommon {
     pub(crate) fn read_tls(&mut self, rd: &mut dyn io::Read) -> io::Result<usize> {
         let res = self.message_deframer.read(rd);
         if let Ok(0) = res {
-            self.peer_eof = true;
+            self.has_seen_eof = true;
         }
         res
     }
@@ -1072,10 +1074,20 @@ impl ConnectionCommon {
         let len = self.received_plaintext.read(buf)?;
 
         if len == 0 && !buf.is_empty() {
-            // No bytes available: if we think we ought to receive more data in the future,
-            // signal `WouldBlock` so that the caller knows it should supply us with more data.
-            if !self.connection_at_eof() {
-                return Err(io::ErrorKind::WouldBlock.into());
+            // No bytes available:
+            match (self.connection_was_cleanly_closed(), self.has_seen_eof) {
+                (true, _) => {
+                    // cleanly closed; don't care about TCP EOF: express this as Ok(0)
+                }
+                (false, true) => {
+                    // unclean closure
+                    return Err(io::ErrorKind::UnexpectedEof.into());
+                }
+                (false, false) => {
+                    // connection still going, but need more data: signal `WouldBlock` so that
+                    // the caller knows this
+                    return Err(io::ErrorKind::WouldBlock.into());
+                }
             }
         }
 

rustls/tests/api.rs

@@ -439,7 +439,8 @@ fn server_close_notify() {
         server.send_close_notify();
 
         transfer(&mut server, &mut client);
-        client.process_new_packets().unwrap();
+        let io_state = client.process_new_packets().unwrap();
+        assert_eq!(io_state.peer_has_closed(), true);
         check_read_and_close(&mut client.reader(), b"from-server!");
 
         transfer(&mut client, &mut server);
@@ -477,7 +478,8 @@ fn client_close_notify() {
         client.send_close_notify();
 
         transfer(&mut client, &mut server);
-        server.process_new_packets().unwrap();
+        let io_state = server.process_new_packets().unwrap();
+        assert_eq!(io_state.peer_has_closed(), true);
         check_read_and_close(&mut server.reader(), b"from-client!");
 
         transfer(&mut server, &mut client);
@@ -486,6 +488,92 @@ fn client_close_notify() {
     }
 }
 
+#[test]
+fn server_closes_uncleanly() {
+    let kt = KeyType::RSA;
+    let server_config = Arc::new(make_server_config(kt));
+
+    for version in rustls::ALL_VERSIONS {
+        let client_config = make_client_config_with_versions(kt, &[version]);
+        let (mut client, mut server) =
+            make_pair_for_arc_configs(&Arc::new(client_config), &server_config);
+        do_handshake(&mut client, &mut server);
+
+        // check that unclean EOF reporting does not overtake appdata
+        assert_eq!(
+            12,
+            server
+                .writer()
+                .write(b"from-server!")
+                .unwrap()
+        );
+        assert_eq!(
+            12,
+            client
+                .writer()
+                .write(b"from-client!")
+                .unwrap()
+        );
+
+        transfer(&mut server, &mut client);
+        transfer_eof(&mut client);
+        let io_state = client.process_new_packets().unwrap();
+        assert_eq!(io_state.peer_has_closed(), false);
+        check_read(&mut client.reader(), b"from-server!");
+
+        assert!(matches!(client.reader().read(&mut [0u8; 1]),
+                         Err(err) if err.kind() == io::ErrorKind::UnexpectedEof));
+
+        // may still transmit pending frames
+        transfer(&mut client, &mut server);
+        server.process_new_packets().unwrap();
+        check_read(&mut server.reader(), b"from-client!");
+    }
+}
+
+#[test]
+fn client_closes_uncleanly() {
+    let kt = KeyType::RSA;
+    let server_config = Arc::new(make_server_config(kt));
+
+    for version in rustls::ALL_VERSIONS {
+        let client_config = make_client_config_with_versions(kt, &[version]);
+        let (mut client, mut server) =
+            make_pair_for_arc_configs(&Arc::new(client_config), &server_config);
+        do_handshake(&mut client, &mut server);
+
+        // check that unclean EOF reporting does not overtake appdata
+        assert_eq!(
+            12,
+            server
+                .writer()
+                .write(b"from-server!")
+                .unwrap()
+        );
+        assert_eq!(
+            12,
+            client
+                .writer()
+                .write(b"from-client!")
+                .unwrap()
+        );
+
+        transfer(&mut client, &mut server);
+        transfer_eof(&mut server);
+        let io_state = server.process_new_packets().unwrap();
+        assert_eq!(io_state.peer_has_closed(), false);
+        check_read(&mut server.reader(), b"from-client!");
+
+        assert!(matches!(server.reader().read(&mut [0u8; 1]),
+                         Err(err) if err.kind() == io::ErrorKind::UnexpectedEof));
+
+        // may still transmit pending frames
+        transfer(&mut server, &mut client);
+        client.process_new_packets().unwrap();
+        check_read(&mut client.reader(), b"from-server!");
+    }
+}
+
 #[derive(Default)]
 struct ServerCheckCertResolve {
     expected_sni: Option<String>,

rustls/tests/common/mod.rs

@@ -126,6 +126,13 @@ pub fn transfer(left: &mut dyn Connection, right: &mut dyn Connection) -> usize
     total
 }
 
+pub fn transfer_eof(conn: &mut dyn Connection) {
+    let empty_buf = [0u8; 0];
+    let empty_cursor: &mut dyn io::Read = &mut &empty_buf[..];
+    let sz = conn.read_tls(empty_cursor).unwrap();
+    assert_eq!(sz, 0);
+}
+
 pub fn transfer_altered<F>(
     left: &mut dyn Connection,
     filter: F,