From abc48584d7ca95b2fc8aa5c4dc981e0b051d8501 Mon Sep 17 00:00:00 2001 From: Maksim Kazantsev Date: Tue, 9 Jun 2026 10:44:30 +0000 Subject: [PATCH 1/2] Pull request 2672: AGDNS-4085-imp-updater-logs-and-errors Squashed commit of the following: commit 2730536faa012017e3d27f838824f9266dd170f7 Merge: ff3253bd4 8d4fd14b2 Author: Maksim Kazantsev Date: Mon Jun 8 18:10:03 2026 +0300 Merge branch 'master' into AGDNS-4085-imp-updater-logs-and-errors commit ff3253bd4c4197737f4b0dc8f11b471d88ecd1eb Author: Maksim Kazantsev Date: Fri Jun 5 12:34:51 2026 +0300 updater: fix invalid usage of validate; commit 4d8e8e7be84f8a270dea32f57cc8759d9f87706e Author: Maksim Kazantsev Date: Wed Jun 3 19:23:46 2026 +0300 all: upd docs; imp code; upd chlog; commit 8a88930c5c3eaef43eb9e6a4ef253f68cea65402 Author: Maksim Kazantsev Date: Wed Jun 3 17:45:08 2026 +0300 updater: imp logs and errs; --- CHANGELOG.md | 6 +++++ internal/home/controlupdate.go | 6 +++-- internal/updater/check.go | 47 +++++++++++++++++++++++++++++----- 3 files changed, 50 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 00f447fdbb0..7e7c7f40afe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,12 @@ See also the [v0.107.78 GitHub milestone][ms-v0.107.78]. NOTE: Add new changes BELOW THIS COMMENT. --> +### Added + +- Improved updater logging to give users more insight into the problem with version updating ([#8410]). + +[#8410]: https://github.com/AdguardTeam/AdGuardHome/issues/8410 + ### Security - The size of rulelists is limited. This is necessary to prevent a user's machine from becoming overloaded if the filter source misbehaves. diff --git a/internal/home/controlupdate.go b/internal/home/controlupdate.go index cb14ab62c93..d0c080f2eb1 100644 --- a/internal/home/controlupdate.go +++ b/internal/home/controlupdate.go @@ -77,7 +77,7 @@ func (web *webAPI) handleVersionJSON(w http.ResponseWriter, r *http.Request) { } // requestVersionInfo sets the VersionInfo field of resp if it can reach the -// update server. +// update server. resp must not be nil. func (web *webAPI) requestVersionInfo( ctx context.Context, resp *versionResponse, @@ -109,6 +109,8 @@ func (web *webAPI) requestVersionInfo( } if err != nil { + web.logger.WarnContext(ctx, "getting version info", slogutil.KeyError, err) + return fmt.Errorf("getting version info: %w", err) } @@ -128,7 +130,7 @@ func (web *webAPI) handleUpdate(w http.ResponseWriter, r *http.Request) { r, w, http.StatusBadRequest, - "/update request isn't allowed now", + "update request isn't allowed now", ) return diff --git a/internal/updater/check.go b/internal/updater/check.go index bd6325ab51d..bd5a41d4563 100644 --- a/internal/updater/check.go +++ b/internal/updater/check.go @@ -13,6 +13,7 @@ import ( "github.com/AdguardTeam/AdGuardHome/internal/aghalg" "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/ioutil" + "github.com/AdguardTeam/golibs/validate" "github.com/c2h5oh/datasize" ) @@ -42,6 +43,8 @@ func (u *Updater) VersionInfo(ctx context.Context, forceRecheck bool) (vi Versio now := time.Now() recheckTime := u.prevCheckTime.Add(versionCheckPeriod) if !forceRecheck && now.Before(recheckTime) { + u.logger.DebugContext(ctx, "version info recheck is not required yet") + return u.prevCheckResult, u.prevCheckError } @@ -55,10 +58,18 @@ func (u *Updater) VersionInfo(ctx context.Context, forceRecheck bool) (vi Versio resp, err := u.client.Do(req) if err != nil { - return VersionInfo{}, fmt.Errorf("requesting %s: %w", vcu, err) + return VersionInfo{}, fmt.Errorf("sending http request to %s: %w", vcu, err) } defer func() { err = errors.WithDeferred(err, resp.Body.Close()) }() + if resp.StatusCode != http.StatusOK { + return VersionInfo{}, fmt.Errorf( + "got status code %d, want %d", + resp.StatusCode, + http.StatusOK, + ) + } + r := ioutil.LimitReader(resp.Body, maxVersionRespSize.Bytes()) // This use of ReadAll is safe, because we just limited the appropriate @@ -74,7 +85,12 @@ func (u *Updater) VersionInfo(ctx context.Context, forceRecheck bool) (vi Versio return u.prevCheckResult, u.prevCheckError } -func (u *Updater) parseVersionResponse(ctx context.Context, data []byte) (VersionInfo, error) { +// parseVersionResponse parses version-related data and unmarshals it into the +// [VersionInfo] structure. +func (u *Updater) parseVersionResponse( + ctx context.Context, + data []byte, +) (vi VersionInfo, err error) { info := VersionInfo{ CanAutoUpdate: aghalg.NBFalse, } @@ -83,14 +99,15 @@ func (u *Updater) parseVersionResponse(ctx context.Context, data []byte) (Versio "announcement": "", "announcement_url": "", } - err := json.Unmarshal(data, &versionJSON) + err = json.Unmarshal(data, &versionJSON) if err != nil { return info, fmt.Errorf("version.json: %w", err) } for k, v := range versionJSON { - if v == "" { - return info, fmt.Errorf("version.json: bad data: value for key %q is empty", k) + err = validate.NotEmpty("version_json_value", v) + if err != nil { + return info, fmt.Errorf("bad value for %q key: %w", k, err) } } @@ -100,10 +117,26 @@ func (u *Updater) parseVersionResponse(ctx context.Context, data []byte) (Versio packageURL, key, found := u.downloadURL(ctx, versionJSON) if !found { - return info, fmt.Errorf("version.json: no package URL: key %q not found in object", key) + return info, fmt.Errorf("version.json: bad key %q: %w", key, errors.ErrNoValue) + } + + isNewVersion := info.NewVersion != u.version + if isNewVersion { + u.logger.InfoContext( + ctx, + "a new version is available", + "current_version", u.version, + "new_version", info.NewVersion, + ) + } else { + u.logger.DebugContext( + ctx, + "the current version is up-to-date", + "current_version", u.version, + ) } - info.CanAutoUpdate = aghalg.BoolToNullBool(info.NewVersion != u.version) + info.CanAutoUpdate = aghalg.BoolToNullBool(isNewVersion) u.newVersion = info.NewVersion u.packageURL = packageURL From 36600da54921e2e9441e0304e6eaa8850764754c Mon Sep 17 00:00:00 2001 From: Fedor Setrakov Date: Tue, 9 Jun 2026 11:16:57 +0000 Subject: [PATCH 2/2] Pull request 2668: AGDNS-4038-rm-h2c-pkg Squashed commit of the following: commit 1ec1589130190478d867ae4d683fa8cae139bb07 Merge: 997f81cf8 abc48584d Author: f.setrakov Date: Tue Jun 9 14:11:45 2026 +0300 Merge branch 'master' into AGDNS-4038-rm-h2c-pkg commit 997f81cf833f612a8319cab0c44c6bf5799248ce Merge: 7caeddd7d 8d4fd14b2 Author: f.setrakov Date: Tue Jun 9 10:57:30 2026 +0300 Merge branch 'master' into AGDNS-4038-rm-h2c-pkg commit 7caeddd7d71cc22b0dcec8fba31a0cc937945c2c Author: f.setrakov Date: Wed Jun 3 16:50:50 2026 +0300 all: imp changelog commit 60d77937ce11e38a1c2db58ee620589e4bbd5f0d Author: f.setrakov Date: Tue Jun 2 20:17:17 2026 +0300 home: add docs commit f24550e69f47fd4d52b8e60321738ca612909efb Merge: 4948ea004 c7b89c77d Author: f.setrakov Date: Tue Jun 2 20:12:42 2026 +0300 Merge branch 'master' into AGDNS-4038-rm-h2c-pkg commit 4948ea0047463d1cc3cdebde2c95f38bbe32c1b0 Author: f.setrakov Date: Mon Jun 1 23:21:54 2026 +0300 all: rm h2c pkg --- CHANGELOG.md | 4 + internal/home/web.go | 16 +- internal/home/web_internal_test.go | 349 ----------------------------- 3 files changed, 9 insertions(+), 360 deletions(-) delete mode 100644 internal/home/web_internal_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e7c7f40afe..6a336b9e3f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,8 @@ NOTE: Add new changes BELOW THIS COMMENT. ### Security +- The H2C connection establishment via HTTP/1.1 request upgrade is no longer supported. See [RFC 9113][rfc9113]. + - The size of rulelists is limited. This is necessary to prevent a user's machine from becoming overloaded if the filter source misbehaves. ### Changed @@ -38,6 +40,8 @@ NOTE: Add new changes BELOW THIS COMMENT. - Blocked services check on the Custom filtering rules page does not work properly without specifying of a client. +[rfc9113]: https://datatracker.ietf.org/doc/html/rfc9113 + diff --git a/internal/home/web.go b/internal/home/web.go index 1862eff0486..29ab155e19b 100644 --- a/internal/home/web.go +++ b/internal/home/web.go @@ -24,10 +24,6 @@ import ( "github.com/AdguardTeam/golibs/osutil/executil" "github.com/NYTimes/gziphandler" "github.com/quic-go/quic-go/http3" - "golang.org/x/net/http2" - - //lint:ignore SA1019 See AGDNS-4038. - "golang.org/x/net/http2/h2c" ) // TODO(a.garipov): Make configurable. @@ -277,18 +273,16 @@ func (web *webAPI) start(ctx context.Context) { hdlr = web.auth.middleware().Wrap(hdlr) - // Use an h2c handler to support unencrypted HTTP/2, e.g. for proxies. - // - // NOTE: The auth middleware must be inside the h2c handler to ensure - // it applies to upgraded HTTP/2 connections as well. See AG-51779. - // - //lint:ignore SA1019 See AGDNS-4038. - hdlr = h2c.NewHandler(hdlr, &http2.Server{}) + // Enable unencrypted HTTP/2, e.g. for proxies. + protocols := &http.Protocols{} + protocols.SetUnencryptedHTTP2(true) + protocols.SetHTTP1(true) // Create a new instance, because the Web is not usable after Shutdown. web.httpServer = &http.Server{ Addr: web.conf.BindAddr.String(), Handler: hdlr, + Protocols: protocols, ReadTimeout: web.conf.ReadTimeout, ReadHeaderTimeout: web.conf.ReadHeaderTimeout, WriteTimeout: web.conf.WriteTimeout, diff --git a/internal/home/web_internal_test.go b/internal/home/web_internal_test.go deleted file mode 100644 index cf153da909c..00000000000 --- a/internal/home/web_internal_test.go +++ /dev/null @@ -1,349 +0,0 @@ -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 - } -}