diff --git a/internal/netutil/multi_listener.go b/internal/netutil/multi_listener.go index a4f8d7c7..1068155e 100644 --- a/internal/netutil/multi_listener.go +++ b/internal/netutil/multi_listener.go @@ -19,11 +19,14 @@ import ( "crypto/x509" "errors" "fmt" + "io" "net" + "net/http" "os" "path/filepath" "strings" "sync" + "time" ) // MultiListener implements net.Listener and accepts connections from multiple @@ -352,7 +355,7 @@ func NewMultiAddrTLSListenerWithOptions(network, address string, tlsOpts TLSOpti return nil, fmt.Errorf("failed to set permissions on socket %s: %w", address, err) } } - return NewMultiListener(tls.NewListener(ln, config)), nil + return NewMultiListener(newTLSListener(ln, config)), nil } addrs, err := resolveHostnameAddrs(address) @@ -369,12 +372,136 @@ func NewMultiAddrTLSListenerWithOptions(network, address string, tlsOpts TLSOpti } return nil, fmt.Errorf("failed to bind TLS listener %s: %w", addr, err) } - listeners = append(listeners, tls.NewListener(ln, config)) + listeners = append(listeners, newTLSListener(ln, config)) } return NewMultiListener(listeners...), nil } +// newTLSListener is tls.NewListener with one addition: a client that sends a +// plaintext HTTP request to a TLS port gets a 400 that says so, instead of the +// empty reply it would otherwise see (curl exit code 52). +func newTLSListener(ln net.Listener, config *tls.Config) net.Listener { + return tls.NewListener(&plaintextHTTPListener{Listener: ln}, config) +} + +// plaintextHTTPListener lets the connection under the TLS layer look at the +// first bytes before the handshake consumes them. +type plaintextHTTPListener struct { + net.Listener +} + +func (l *plaintextHTTPListener) Accept() (net.Conn, error) { + conn, err := l.Listener.Accept() + if err != nil { + return nil, err + } + return &plaintextHTTPConn{Conn: conn}, nil +} + +// errPlaintextHTTPOnTLS aborts the handshake once the client has been told it +// spoke plaintext HTTP to a TLS port. +var errPlaintextHTTPOnTLS = errors.New("plaintext HTTP request received on a TLS listener") + +// plaintextHTTPDrainTimeout bounds how long the reply waits for the client to +// finish sending, so a client that keeps the connection open can't hold the +// server goroutine. +const plaintextHTTPDrainTimeout = 500 * time.Millisecond + +// plaintextHTTPConn holds the first five bytes of a connection back from the +// TLS handshake. A TLS ClientHello starts with a record type byte (0x16 for +// TLS 1.0 through 1.3), so a request line in those bytes means the client spoke +// plaintext HTTP. Without this, the handshake fails and the server, which only +// sees a read error, closes the connection without a response. +type plaintextHTTPConn struct { + net.Conn + prefix []byte + sniffed bool +} + +func (c *plaintextHTTPConn) Read(p []byte) (int, error) { + if !c.sniffed { + c.sniffed = true + + var hdr [5]byte + n, err := io.ReadFull(c.Conn, hdr[:]) + if n == len(hdr) && looksLikeHTTPRequest(hdr) { + writePlaintextHTTPResponse(c.Conn, hdr) + return 0, errPlaintextHTTPOnTLS + } + + // Hand the sniffed bytes to the handshake. After a short read, the + // next read of the connection returns the error again. + c.prefix = hdr[:n] + if err != nil && n == 0 { + return 0, err + } + } + + if len(c.prefix) > 0 { + n := copy(p, c.prefix) + c.prefix = c.prefix[n:] + return n, nil + } + + return c.Conn.Read(p) +} + +// writePlaintextHTTPResponse answers a client that sent plaintext HTTP to a TLS +// port. The body is deliberately endpoint-agnostic: the same listener backs the +// S3, admin, IAM, website and web UI servers, whose error bodies are XML, HTML +// and JSON, and none of them has seen the request yet. +func writePlaintextHTTPResponse(conn net.Conn, hdr [5]byte) { + body := "Client sent an HTTP request to an HTTPS server.\n" + + "This listener serves TLS; retry with https:// instead of http://.\n" + head := "HTTP/1.1 400 Bad Request\r\n" + + "Date: " + time.Now().UTC().Format(http.TimeFormat) + "\r\n" + + "Content-Type: text/plain; charset=utf-8\r\n" + + fmt.Sprintf("Content-Length: %d\r\n", len(body)) + + "Connection: close\r\n\r\n" + + if string(hdr[:]) == "HEAD " { + // A response to HEAD carries no body, but the length it would + // have had. + _, _ = io.WriteString(conn, head) + } else { + _, _ = io.WriteString(conn, head+body) + } + closePlaintextHTTP(conn) +} + +// closePlaintextHTTP half-closes and drains briefly before closing. Closing with +// the rest of the client's request still unread sends a RST, which can reach the +// client before the response that was just written. +func closePlaintextHTTP(conn net.Conn) { + if halfCloser, ok := conn.(interface{ CloseWrite() error }); ok { + _ = halfCloser.CloseWrite() + } + if err := conn.SetReadDeadline(time.Now().Add(plaintextHTTPDrainTimeout)); err == nil { + _, _ = io.Copy(io.Discard, conn) + } + _ = conn.Close() +} + +// looksLikeHTTPRequest reports whether the five bytes crypto/tls took for a +// record header are the start of an HTTP request line. A TLS handshake record +// begins with a record type byte (0x16 for TLS 1.0 through 1.3), so an HTTP +// method token can't collide with one. +func looksLikeHTTPRequest(hdr [5]byte) bool { + for i, b := range hdr { + if b == ' ' { + // The space ends the method token. + return i >= 3 + } + if b < 'A' || b > 'Z' { + return false + } + } + // Five uppercase letters are a method longer than four characters, + // such as DELETE, OPTIONS or CONNECT. + return true +} + // RequireSecureTransport enforces mTLS or unix socket a unix socket // is always acceptable (the filesystem is the trust boundary), // but a TCP address is only acceptable when mTLS (a server cert plus diff --git a/internal/netutil/tls_plaintext_http_test.go b/internal/netutil/tls_plaintext_http_test.go new file mode 100644 index 00000000..ad8be57d --- /dev/null +++ b/internal/netutil/tls_plaintext_http_test.go @@ -0,0 +1,172 @@ +// Copyright 2026 Versity Software +// This file is licensed under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package netutil + +import ( + "crypto/tls" + "io" + "net" + "net/http" + "strings" + "testing" + "time" +) + +// serveTestTLS accepts from ln and runs each connection the way a server built +// on net.Conn does: hand the handshake to the connection first, then read. The +// bytes read are echoed back so the TLS case can observe a round trip. +func serveTestTLS(ln net.Listener) { + for { + conn, err := ln.Accept() + if err != nil { + return + } + go func(c net.Conn) { + defer c.Close() + + if hs, ok := c.(interface{ Handshake() error }); ok { + _ = hs.Handshake() + } + + buf := make([]byte, 512) + n, err := c.Read(buf) + if err != nil { + return + } + _, _ = c.Write(buf[:n]) + }(conn) + } +} + +func newTestTLSListener(t *testing.T) net.Listener { + t.Helper() + + cert := issueTestCert(t, generateTestCA(t), "localhost") + ln, err := NewMultiAddrTLSListener("tcp", "127.0.0.1:0", func(*tls.ClientHelloInfo) (*tls.Certificate, error) { + return &cert, nil + }, ListenerOptions{}) + if err != nil { + t.Fatalf("NewMultiAddrTLSListener() error = %v", err) + } + t.Cleanup(func() { _ = ln.Close() }) + + go serveTestTLS(ln) + + return ln +} + +// TestMultiAddrTLSListenerAnswersPlaintextHTTP is the regression test for +// versity/versitygw#2261: a plaintext HTTP request to a TLS listener has to +// come back with a 400 that names the mistake, not an empty reply +// (curl exit code 52). +func TestMultiAddrTLSListenerAnswersPlaintextHTTP(t *testing.T) { + client := &http.Client{Timeout: 5 * time.Second} + + t.Run("http client", func(t *testing.T) { + ln := newTestTLSListener(t) + + resp, err := client.Get("http://" + ln.Addr().String() + "/") + if err != nil { + t.Fatalf("plaintext HTTP request to the TLS listener: %v", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusBadRequest { + t.Fatalf("status = %d, want %d", resp.StatusCode, http.StatusBadRequest) + } + + body, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatalf("read response body: %v", err) + } + if !strings.Contains(string(body), "HTTPS server") { + t.Errorf("body = %q, want it to mention the HTTPS server", body) + } + }) + + t.Run("absolute form request line", func(t *testing.T) { + ln := newTestTLSListener(t) + + conn, err := net.DialTimeout("tcp", ln.Addr().String(), 5*time.Second) + if err != nil { + t.Fatalf("dial: %v", err) + } + defer conn.Close() + + if err := conn.SetDeadline(time.Now().Add(5 * time.Second)); err != nil { + t.Fatalf("set deadline: %v", err) + } + + request := "GET http://" + ln.Addr().String() + "/ HTTP/1.1\r\nHost: " + ln.Addr().String() + "\r\n\r\n" + if _, err := io.WriteString(conn, request); err != nil { + t.Fatalf("write request: %v", err) + } + + raw, err := io.ReadAll(conn) + if err != nil { + t.Fatalf("read response: %v", err) + } + if !strings.HasPrefix(string(raw), "HTTP/1.1 400 Bad Request") { + t.Errorf("response = %q, want an HTTP 400", raw) + } + }) + + t.Run("head has no body", func(t *testing.T) { + ln := newTestTLSListener(t) + + resp, err := client.Head("http://" + ln.Addr().String() + "/") + if err != nil { + t.Fatalf("plaintext HEAD to the TLS listener: %v", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusBadRequest { + t.Fatalf("status = %d, want %d", resp.StatusCode, http.StatusBadRequest) + } + body, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatalf("read response body: %v", err) + } + if len(body) != 0 { + t.Errorf("HEAD body = %q, want none", body) + } + }) +} + +// TestMultiAddrTLSListenerStillServesTLS guards the other side of the change: +// a real TLS client must still complete the handshake and round-trip data. +func TestMultiAddrTLSListenerStillServesTLS(t *testing.T) { + ln := newTestTLSListener(t) + + conn, err := tls.Dial("tcp", ln.Addr().String(), &tls.Config{InsecureSkipVerify: true}) //nolint:gosec // test certificate + if err != nil { + t.Fatalf("TLS dial: %v", err) + } + defer conn.Close() + + if err := conn.SetDeadline(time.Now().Add(5 * time.Second)); err != nil { + t.Fatalf("set deadline: %v", err) + } + if _, err := conn.Write([]byte("ping")); err != nil { + t.Fatalf("write: %v", err) + } + + buf := make([]byte, 4) + if _, err := io.ReadFull(conn, buf); err != nil { + t.Fatalf("read echo: %v", err) + } + if string(buf) != "ping" { + t.Errorf("echo = %q, want %q", buf, "ping") + } +}