feat(http/detect)!: error when the socket is closed (#3721)

* refactor(http): consolidate HTTP protocol detection

Linkerd's HTTP protocol detection logic is spread across a few crates: the
linkerd-detect crate is generic over the actual protocol detection logic, and
the linkerd-proxy-http crate provides an implementation. There are no other
implemetations of the Detect interface. This leads to gnarly type signatures in
the form `Result<Option<http::Variant>, DetectTimeoutError>`: simultaneously
verbose and not particularly informative (what does the None case mean exactly).

This commit introduces a new crate, `linkerd-http-detect`, consolidating this
logic and removes the prior implementations. The admin, inbound, and outbound
stacks are updated to use these new types. This work is done in anticipation of
introducing metrics that report HTTP detection behavior.

There are no functional changes.

* feat(http/detect)!: error when the socket is closed

When a proxy does protocol detection, the initial read may indicate that the
connection was closed by the client with no data being written to the socket. In
such a case, the proxy continues to process the connection as if may be proxied,
but we expect this to fail immediately. This can lead to unexpected proxy
behavior: for example, inbound proxies may report policy denials.

To address this, this change surfaces an error (as if the read call failed).
This could, theoretically, impact some bizarre clients that initiate half-open
connections. These corner cases can use explicit opaque policies to bypass
detection.

linkerd/app/admin/src/stack.rs

@@ -166,7 +166,7 @@ impl Config {
                         }
                         // If the connection failed HTTP detection, check if we detected TLS for
                         // another target. This might indicate that the client is confused/stale.
-                        http::Detection::Empty | http::Detection::NotHttp => match tcp.tls {
+                        http::Detection::NotHttp => match tcp.tls {
                             tls::ConditionalServerTls::Some(tls::ServerTls::Passthru { sni }) => {
                                 Err(UnexpectedSni(sni, tcp.client).into())
                             }

linkerd/app/inbound/src/detect.rs

@@ -109,9 +109,7 @@ impl Inbound<svc::ArcNewTcp<Http, io::BoxedIo>> {
                     |(detected, Detect { tls, .. })| -> Result<_, Infallible> {
                         match detected {
                             http::Detection::Http(http) => Ok(svc::Either::A(Http { http, tls })),
-                            http::Detection::Empty | http::Detection::NotHttp => {
-                                Ok(svc::Either::B(tls))
-                            }
+                            http::Detection::NotHttp => Ok(svc::Either::B(tls)),
                             // When HTTP detection fails, forward the connection to the application as
                             // an opaque TCP stream.
                             http::Detection::ReadTimeout(timeout) => {

linkerd/http/detect/src/lib.rs

@@ -18,7 +18,6 @@ pub struct DetectParams {
 
 #[derive(Debug, Clone)]
 pub enum Detection {
-    Empty,
     NotHttp,
     Http(Variant),
     ReadTimeout(time::Duration),
@@ -154,7 +153,7 @@ async fn detect<I: io::AsyncRead + Send + Unpin + 'static>(
 ) -> io::Result<Detection> {
     debug_assert!(buf.capacity() > 0, "buffer must have capacity");
 
-    trace!(capacity = READ_CAPACITY, timeout = ?read_timeout, "Reading");
+    trace!(capacity = buf.capacity(), timeout = ?read_timeout, "Reading");
     let sz = match time::timeout(read_timeout, io.read_buf(buf)).await {
         Ok(res) => res?,
         Err(_) => return Ok(Detection::ReadTimeout(read_timeout)),
@@ -162,7 +161,10 @@ async fn detect<I: io::AsyncRead + Send + Unpin + 'static>(
 
     trace!(sz, "Read");
     if sz == 0 {
-        return Ok(Detection::Empty);
+        return Err(io::Error::new(
+            io::ErrorKind::UnexpectedEof,
+            "socket closed before protocol detection",
+        ));
     }
 
     // HTTP/2 checking is faster because it's a simple string match. If we
@@ -300,8 +302,8 @@ mod tests {
         };
         let mut buf = BytesMut::with_capacity(1024);
         let mut io = io::Builder::new().build();
-        let kind = detect(params, &mut io, &mut buf).await.unwrap();
-        assert!(matches!(kind, Detection::Empty), "{kind:?}");
+        let err = detect(params, &mut io, &mut buf).await.unwrap_err();
+        assert_eq!(err.kind(), std::io::ErrorKind::UnexpectedEof, "{err:?}");
         assert_eq!(&buf[..], b"");
     }
 }