From 313135d74f40004b966b887babd4b83bca152e85 Mon Sep 17 00:00:00 2001 From: Fedor Setrakov Date: Thu, 23 Apr 2026 14:28:50 +0000 Subject: [PATCH] Pull request 2614: AGDNS-3799-h2c-vuln-test Squashed commit of the following: commit 43c0748600f8d69ffd126991c17c9cb972a7961c Merge: 1a9c78d8b 35f910186 Author: f.setrakov Date: Thu Apr 23 17:21:12 2026 +0300 Merge branch 'master' into AGDNS-3799-h2c-vuln-test commit 1a9c78d8bfc219b54305d13e9d5abe6e8f5a6517 Author: f.setrakov Date: Thu Apr 23 17:09:55 2026 +0300 home: imp docs commit 729d41ab1484acf94b8367ea1e7c0427d35cd481 Author: f.setrakov Date: Thu Apr 23 14:18:26 2026 +0300 home: rm temp logs commit 4ad68ef711b38eac03a08040445719ca6fe6c557 Merge: 4e33babc8 b08e58766 Author: f.setrakov Date: Thu Apr 23 14:04:37 2026 +0300 Merge branch 'master' into AGDNS-3799-h2c-vuln-test commit 4e33babc809098c802b114eac497256967525ad9 Author: f.setrakov Date: Thu Apr 23 13:41:13 2026 +0300 home: print stack commit 7bcd54314a972885ea27901add69ff5c96a2eb12 Author: f.setrakov Date: Thu Apr 23 13:33:06 2026 +0300 home: add temp log commit cb21e8128b19166cbeef8626ebaf871c06a38f8a Author: f.setrakov Date: Tue Apr 21 18:10:30 2026 +0300 home: enable h2c test logs commit f0785d97298c36c7d96860f08c613876a201e46d Author: f.setrakov Date: Thu Apr 16 17:59:40 2026 +0300 home: rm querylog, imp code commit 98fea60334a3370131d6299edef91aa8e045d0bc Author: f.setrakov Date: Wed Apr 15 19:00:38 2026 +0300 home: use static files for healthcheck commit b01973e31109d1fa34667510e6a1b84f049a309e Author: f.setrakov Date: Wed Apr 15 11:56:26 2026 +0300 home: disable conn reuse commit 4b7ce72cd49db56745f5fac88c04f33f45f2558e Author: f.setrakov Date: Tue Apr 14 23:42:12 2026 +0300 home: increase timeout commit 7ffbc64d6b9e9b1b0ab5a6b2b41c01f0891a4ead Author: f.setrakov Date: Tue Apr 14 23:27:02 2026 +0300 home: use /control/status for healthcheck commit bc45ee874a006c4b4b6d11d3c1e63941af85d47c Author: f.setrakov Date: Tue Apr 14 18:39:28 2026 +0300 web: fix req body, increase timeouts commit 913044292ddbe89d27ef2ec0a3ce167511381797 Author: f.setrakov Date: Thu Apr 9 20:30:30 2026 +0300 home: fix ci, imp style commit be70c8f5ea45819ed6abffef1e2bd211a3c01761 Author: f.setrakov Date: Mon Apr 6 15:51:42 2026 +0300 home: imp test commit 0bff1300aea08182d70cc633f3290b5e6e10a860 Author: f.setrakov Date: Thu Apr 2 19:08:44 2026 +0300 home: impl upgrade requests commit 483ed1ff149e489f9b0a8ac3f1a438e04c1087fb Author: f.setrakov Date: Fri Mar 27 13:07:07 2026 +0300 home: h2c vuln test draft --- internal/home/clientshttp_internal_test.go | 4 - internal/home/home_internal_test.go | 35 +++ internal/home/tls_internal_test.go | 33 -- internal/home/web_internal_test.go | 349 +++++++++++++++++++++ 4 files changed, 384 insertions(+), 37 deletions(-) create mode 100644 internal/home/web_internal_test.go diff --git a/internal/home/clientshttp_internal_test.go b/internal/home/clientshttp_internal_test.go index 37b4daaa6..f088d346e 100644 --- a/internal/home/clientshttp_internal_test.go +++ b/internal/home/clientshttp_internal_test.go @@ -11,7 +11,6 @@ import ( "net/url" "slices" "testing" - "time" "github.com/AdguardTeam/AdGuardHome/internal/client" "github.com/AdguardTeam/AdGuardHome/internal/filtering" @@ -22,9 +21,6 @@ import ( "github.com/stretchr/testify/require" ) -// testTimeout is the common timeout for tests and contexts. -const testTimeout = 1 * time.Second - const ( testClientIP1 = "1.1.1.1" testClientIP2 = "2.2.2.2" diff --git a/internal/home/home_internal_test.go b/internal/home/home_internal_test.go index 78b03154d..e4c172d6a 100644 --- a/internal/home/home_internal_test.go +++ b/internal/home/home_internal_test.go @@ -5,6 +5,7 @@ import ( "net/http" "net/netip" "testing" + "time" "github.com/AdguardTeam/AdGuardHome/internal/agh" "github.com/AdguardTeam/AdGuardHome/internal/aghhttp" @@ -14,6 +15,9 @@ import ( "github.com/stretchr/testify/require" ) +// testTimeout is the common timeout for tests and contexts. +const testTimeout = 1 * time.Second + // testLogger is a common logger for tests. var testLogger = slogutil.NewDiscardLogger() @@ -52,7 +56,38 @@ func newTestWeb( return web } +// storeGlobals is a test helper function that saves global variables and +// restores them once the test is complete. +// +// The global variables are: +// - [config] +// - [glFilePrefix] +// - [globalContext.clients.storage] +// - [globalContext.dnsServer] +// - [globalContext.web] +// +// TODO(s.chzhen): Remove this once the TLS manager no longer accesses global +// variables. Make tests that use this helper concurrent. +func storeGlobals(tb testing.TB) { + tb.Helper() + + prevConfig := config + prefGLFilePrefix := glFilePrefix + storage := globalContext.clients.storage + dnsServer := globalContext.dnsServer + web := globalContext.web + + tb.Cleanup(func() { + config = prevConfig + glFilePrefix = prefGLFilePrefix + globalContext.clients.storage = storage + globalContext.dnsServer = dnsServer + globalContext.web = web + }) +} + func TestMain(m *testing.M) { initCmdLineOpts() + testutil.DiscardLogOutput(m) } diff --git a/internal/home/tls_internal_test.go b/internal/home/tls_internal_test.go index 80e6172b1..161d43f94 100644 --- a/internal/home/tls_internal_test.go +++ b/internal/home/tls_internal_test.go @@ -133,39 +133,6 @@ func TestValidateCertificates(t *testing.T) { }) } -// storeGlobals is a test helper function that saves global variables and -// restores them once the test is complete. -// -// The global variables are: -// - [config] -// - [glFilePrefix] -// - [globalContext.auth] -// - [globalContext.clients.storage] -// - [globalContext.dnsServer] -// - [globalContext.firstRun] -// - [globalContext.mux] -// - [globalContext.web] -// -// TODO(s.chzhen): Remove this once the TLS manager no longer accesses global -// variables. Make tests that use this helper concurrent. -func storeGlobals(tb testing.TB) { - tb.Helper() - - prevConfig := config - prefGLFilePrefix := glFilePrefix - storage := globalContext.clients.storage - dnsServer := globalContext.dnsServer - web := globalContext.web - - tb.Cleanup(func() { - config = prevConfig - glFilePrefix = prefGLFilePrefix - globalContext.clients.storage = storage - globalContext.dnsServer = dnsServer - globalContext.web = web - }) -} - // newCertWithoutIP generates a CA certificate, a leaf certificate without an IP // address, and the PEM-encoded leaf private key. func newCertWithoutIP(tb testing.TB) ( diff --git a/internal/home/web_internal_test.go b/internal/home/web_internal_test.go new file mode 100644 index 000000000..cf153da90 --- /dev/null +++ b/internal/home/web_internal_test.go @@ -0,0 +1,349 @@ +package home + +import ( + "bufio" + "bytes" + "fmt" + "net" + "net/http" + "net/url" + "path" + "strconv" + "testing" + "testing/fstest" + + "github.com/AdguardTeam/AdGuardHome/internal/aghhttp" + "github.com/AdguardTeam/AdGuardHome/internal/aghos" + "github.com/AdguardTeam/AdGuardHome/internal/aghuser" + "github.com/AdguardTeam/golibs/netutil" + "github.com/AdguardTeam/golibs/netutil/urlutil" + "github.com/AdguardTeam/golibs/testutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/crypto/bcrypt" + "golang.org/x/net/http2" + "golang.org/x/net/http2/hpack" +) + +const ( + // clientPreface is the message sent to the server as a final confirmation + // of HTTP2 usage. + clientPreface = "PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n" + + // testSettings is a common value of the HTTP2-Settings header for tests. + testSettings = "AAEAABAAAAIAAAABAAQAAP__AAUAAEAAAAgAAAAAAAMAAABkAAYAAQAA" + + // testHpackMaxDynamicTableSize is the common HPACK max dynamic table size + // value for tests. + testHPACKMaxDynamicTableSize = 4096 + + // testTargetStreamID is a common HTTP2 stream ID for sending requests after + // an upgrade. + // + // NOTE: The upgrade request implicitly uses Stream ID 1, so the first + // client-side valid ID is 3. + testTargetStreamID = 3 +) + +// h2c upgrade headers. +// +// TODO(a.garipov): Add to httphdr. +const ( + headerConnection = "Connection" + headerUpgrade = "Upgrade" + headerHTTP2Settings = "HTTP2-Settings" +) + +// h2c upgrade header values for tests. +const ( + testHeaderValueConnection = "Upgrade, HTTP2-Settings" + testHeaderValueUpgrade = "h2c" +) + +// testDecoder implements HTTP2 HPACK-encoded headers decoding for tests. +type testDecoder struct { + decoder *hpack.Decoder + status int +} + +// newTestDecoder returns a properly initialized *testDecoder. +func newTestDecoder(tb testing.TB) (d *testDecoder) { + tb.Helper() + + d = &testDecoder{} + d.decoder = hpack.NewDecoder(testHPACKMaxDynamicTableSize, func(f hpack.HeaderField) { + if f.Name != ":status" { + return + } + + status64, err := strconv.ParseInt(f.Value, 10, 64) + require.NoError(tb, err) + + d.status = int(status64) + }) + + return d +} + +// decodeStatus decodes an HPACK-encoded header block and returns the HTTP +// status code. +func (d *testDecoder) decodeStatus(tb testing.TB, b []byte) (status int) { + tb.Helper() + + d.status = 0 + + _, err := d.decoder.Write(b) + require.NoError(tb, err) + + return d.status +} + +func TestWebAPI_h2cVulnerability(t *testing.T) { + storeGlobals(t) + + stop := make(chan struct{}) + t.Cleanup(func() { + testutil.RequireReceive(t, stop, testTimeout) + }) + + password := "password" + passwordHash, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.MinCost) + require.NoError(t, err) + + fs := fstest.MapFS{ + "build/static/login.html": &fstest.MapFile{ + Data: []byte("foo"), + Mode: aghos.DefaultPermFile, + }, + } + + user := webUser{ + Name: "foo", + PasswordHash: string(passwordHash), + UserID: aghuser.MustNewUserID(), + } + + mux := http.NewServeMux() + auth, err := newAuth(testutil.ContextWithTimeout(t, testTimeout), &authConfig{ + baseLogger: testLogger, + rateLimiter: emptyRateLimiter{}, + trustedProxies: testTrustedProxies, + dbFilename: path.Join(t.TempDir(), "sessions.db"), + users: []webUser{user}, + sessionTTL: testTimeout, + isGLiNet: false, + mux: mux, + }) + require.NoError(t, err) + + t.Cleanup(func() { + ctx := testutil.ContextWithTimeout(t, testTimeout) + auth.close(ctx) + }) + + mw := &webMw{} + registrar := aghhttp.NewDefaultRegistrar(mux, mw.wrap) + web := newTestWeb(t, &webConfig{ + baseLogger: testLogger, + auth: auth, + mux: mux, + httpReg: registrar, + clientBuildFS: fs, + }) + + mw.set(web) + globalContext.web = web + + port := config.HTTPConfig.Address.Port() + host := fmt.Sprintf("%s:%d", netutil.IPv4Localhost(), port) + + go func() { + ctx := testutil.ContextWithTimeout(t, testTimeout) + web.start(ctx) + close(stop) + }() + + t.Cleanup(func() { + ctx := testutil.ContextWithTimeout(t, testTimeout) + web.close(ctx) + }) + + waitForWebAPIReady(t, host) + performH2CUpgradeAttack(t, host) +} + +// waitForWebAPIReady waits until the [webAPI] server has started and is ready +// to accept connections. +func waitForWebAPIReady(tb testing.TB, host string) { + tb.Helper() + + u := (&url.URL{ + Scheme: urlutil.SchemeHTTP, + Host: host, + Path: "/login.html", + }).String() + + require.EventuallyWithT(tb, func(c *assert.CollectT) { + ctx := testutil.ContextWithTimeout(tb, testTimeout) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, u, nil) + require.NoError(c, err) + + resp, err := http.DefaultClient.Do(req) + require.NoError(c, err) + assert.Equal(c, http.StatusOK, resp.StatusCode) + }, testTimeout, testTimeout/10) +} + +// performH2CUpgradeAttack establishes a TCP connection to the specified host, +// performs an HTTP2 protocol upgrade, and attempts to access a protected +// endpoint without proper authentication, verifying that the server responds +// with [http.StatusUnauthorized]. +func performH2CUpgradeAttack(tb testing.TB, host string) { + tb.Helper() + + dialer := &net.Dialer{} + ctx := testutil.ContextWithTimeout(tb, testTimeout) + + conn, err := dialer.DialContext(ctx, "tcp", host) + require.NoError(tb, err) + testutil.CleanupAndRequireSuccess(tb, conn.Close) + + writer := bufio.NewWriter(conn) + reader := bufio.NewReader(conn) + + u := &url.URL{ + Scheme: urlutil.SchemeHTTP, + Host: host, + Path: "/control/login", + } + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, u.String(), nil) + require.NoError(tb, err) + + req.Header.Set(headerConnection, testHeaderValueConnection) + req.Header.Set(headerUpgrade, testHeaderValueUpgrade) + req.Header.Set(headerHTTP2Settings, testSettings) + + err = req.Write(writer) + require.NoError(tb, err) + require.NoError(tb, writer.Flush()) + + resp, err := http.ReadResponse(reader, req) + require.NoError(tb, err) + require.Equal(tb, http.StatusSwitchingProtocols, resp.StatusCode) + testutil.CleanupAndRequireSuccess(tb, resp.Body.Close) + + _, err = writer.Write([]byte(clientPreface)) + require.NoError(tb, err) + + framer := http2.NewFramer(writer, reader) + decoder := newTestDecoder(tb) + performH2CSettingsExchange(tb, framer, writer, decoder) + sendH2CRequest(tb, framer, host) + require.NoError(tb, writer.Flush()) + + readH2CResponse(tb, framer, decoder) +} + +// performH2CSettingsExchange performs the HTTP2 settings exchange handshake. It +// sends empty client settings, waits for acknowledgement, and then receives the +// server settings and responds with acknowledgement. framer, writer and +// decoder must not be nil. +func performH2CSettingsExchange( + tb testing.TB, + framer *http2.Framer, + writer *bufio.Writer, + decoder *testDecoder, +) { + tb.Helper() + + err := framer.WriteSettings() + require.NoError(tb, err) + require.NoError(tb, writer.Flush()) + + var ( + gotServerSettings bool + gotSettingsAck bool + ) + for !gotServerSettings || !gotSettingsAck { + var frame http2.Frame + frame, err = framer.ReadFrame() + require.NoError(tb, err) + + switch f := frame.(type) { + case *http2.HeadersFrame: + // NOTE: The decoder must process all headers frames because the + // client and server share the same HPACK dynamic table. Skipping + // frames causes index desynchronization. + decoder.decodeStatus(tb, f.HeaderBlockFragment()) + case *http2.SettingsFrame: + if f.IsAck() { + gotSettingsAck = true + + continue + } + + err = framer.WriteSettingsAck() + require.NoError(tb, err) + require.NoError(tb, writer.Flush()) + + gotServerSettings = true + } + } +} + +// sendH2CRequest writes a request to a protected endpoint into the framer. +// framer must not be nil. +func sendH2CRequest(tb testing.TB, framer *http2.Framer, host string) { + tb.Helper() + + var headerBlockFragment bytes.Buffer + enc := hpack.NewEncoder(&headerBlockFragment) + headers := []hpack.HeaderField{ + {Name: ":method", Value: http.MethodGet}, + {Name: ":path", Value: "/control/status"}, + {Name: ":scheme", Value: urlutil.SchemeHTTP}, + {Name: ":authority", Value: host}, + } + + for _, h := range headers { + require.NoError(tb, enc.WriteField(h)) + } + + err := framer.WriteHeaders(http2.HeadersFrameParam{ + StreamID: testTargetStreamID, + BlockFragment: headerBlockFragment.Bytes(), + EndHeaders: true, + EndStream: true, + }) + require.NoError(tb, err) +} + +// readH2CResponse reads the response from an h2c connection and asserts that +// the server responds with [http.StatusUnauthorized]. framer and decoder must +// not be nil. +func readH2CResponse(tb testing.TB, framer *http2.Framer, decoder *testDecoder) { + tb.Helper() + + for { + frame, err := framer.ReadFrame() + require.NoError(tb, err) + + if frame.Header().StreamID != testTargetStreamID { + headerFrame, ok := frame.(*http2.HeadersFrame) + if ok { + decoder.decodeStatus(tb, headerFrame.HeaderBlockFragment()) + } + + continue + } + + headerFrame := testutil.RequireTypeAssert[*http2.HeadersFrame](tb, frame) + require.True(tb, headerFrame.StreamEnded()) + + status := decoder.decodeStatus(tb, headerFrame.HeaderBlockFragment()) + assert.Equal(tb, http.StatusUnauthorized, status) + + break + } +}