Merge pull request #2508 from tunglambk/fix/website-http-to-https-2261

fix: answer plaintext HTTP sent to a TLS listener with a 400
2026-10-22 06:12:39 +00:00 · 2026-10-13 15:33:19 -07:00
parent 82bbb69eb0 ea4492505d
commit ca4c8d30f5
2 changed files with 301 additions and 2 deletions
@@ -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
@@ -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")
+	}
+}