From fe0c53ec439940635864a9d1bc4687245dbe4373 Mon Sep 17 00:00:00 2001 From: Rahul Somasundaram Date: Tue, 20 Sep 2022 21:16:52 +0530 Subject: [PATCH 01/13] adding TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA to safe cipher suite --- internal/aghtls/aghtls.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/aghtls/aghtls.go b/internal/aghtls/aghtls.go index 5dc7a382..2e286735 100644 --- a/internal/aghtls/aghtls.go +++ b/internal/aghtls/aghtls.go @@ -17,7 +17,6 @@ func SaferCipherSuites() (safe []uint16) { tls.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA, tls.TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA, tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, - tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA, tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256, tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256: // Less safe 3DES and CBC suites, go on. From 91bbb744dc94469210d254bd5219d35b43b3d3b5 Mon Sep 17 00:00:00 2001 From: Rahul Somasundaram Date: Thu, 22 Sep 2022 07:53:39 +0530 Subject: [PATCH 02/13] Revert "adding TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA to safe cipher suite" This reverts commit fe0c53ec439940635864a9d1bc4687245dbe4373. --- internal/aghtls/aghtls.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/aghtls/aghtls.go b/internal/aghtls/aghtls.go index 2e286735..5dc7a382 100644 --- a/internal/aghtls/aghtls.go +++ b/internal/aghtls/aghtls.go @@ -17,6 +17,7 @@ func SaferCipherSuites() (safe []uint16) { tls.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA, tls.TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA, tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, + tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA, tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256, tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256: // Less safe 3DES and CBC suites, go on. From 59d18c6598ac857da4810cd12fb6625b56a67155 Mon Sep 17 00:00:00 2001 From: Rahul Somasundaram Date: Thu, 22 Sep 2022 08:28:46 +0530 Subject: [PATCH 03/13] added support for User prefered Ciphers --- internal/aghtls/aghtls.go | 18 +++++++++++++++++- internal/dnsforward/config.go | 3 +++ internal/home/home.go | 1 + internal/home/web.go | 12 +++++++++++- 4 files changed, 32 insertions(+), 2 deletions(-) diff --git a/internal/aghtls/aghtls.go b/internal/aghtls/aghtls.go index 5dc7a382..213eb8fe 100644 --- a/internal/aghtls/aghtls.go +++ b/internal/aghtls/aghtls.go @@ -1,7 +1,12 @@ // Package aghtls contains utilities for work with TLS. package aghtls -import "crypto/tls" +import ( + "crypto/tls" + + "github.com/AdguardTeam/golibs/log" + "golang.org/x/exp/slices" +) // SaferCipherSuites returns a set of default cipher suites with vulnerable and // weak cipher suites removed. @@ -28,3 +33,14 @@ func SaferCipherSuites() (safe []uint16) { return safe } + +func UserPreferedCipherSuites(ciphers []string) (userCiphers []uint16) { + for _, s := range tls.CipherSuites() { + if slices.Contains(ciphers, s.Name) { + userCiphers = append(userCiphers, s.ID) + log.Debug("user specified cipher : %s, ID : %d", s.Name, s.ID) + } + } + + return userCiphers +} diff --git a/internal/dnsforward/config.go b/internal/dnsforward/config.go index 747767c4..29563725 100644 --- a/internal/dnsforward/config.go +++ b/internal/dnsforward/config.go @@ -165,6 +165,9 @@ type TLSConfig struct { cert tls.Certificate // DNS names from certificate (SAN) or CN value from Subject dnsNames []string + + // ciphers specified by user + TLSCiphers []string `yaml:"tls_ciphers" json:"-"` } // DNSCryptConfig is the DNSCrypt server configuration struct. diff --git a/internal/home/home.go b/internal/home/home.go index 4b200bc1..f7acd2dd 100644 --- a/internal/home/home.go +++ b/internal/home/home.go @@ -366,6 +366,7 @@ func initWeb(args options, clientBuildFS fs.FS) (web *Web, err error) { clientFS: clientFS, clientBetaFS: clientBetaFS, + tlsCiphers: config.TLS.TLSCiphers, } web = CreateWeb(&webConf) diff --git a/internal/home/web.go b/internal/home/web.go index 2052df55..b7157a1a 100644 --- a/internal/home/web.go +++ b/internal/home/web.go @@ -58,6 +58,9 @@ type webConfig struct { WriteTimeout time.Duration firstRun bool + + // ciphers specified by user + tlsCiphers []string } // HTTPSServer - HTTPS Server @@ -269,6 +272,13 @@ func (web *Web) tlsServerLoop() { web.httpsServer.cond.L.Unlock() + var cipher []uint16 + + if len(web.conf.tlsCiphers) == 0 { + cipher = aghtls.SaferCipherSuites() + } else { + cipher = aghtls.UserPreferedCipherSuites(web.conf.tlsCiphers) + } // prepare HTTPS server address := netutil.JoinHostPort(web.conf.BindHost.String(), web.conf.PortHTTPS) web.httpsServer.server = &http.Server{ @@ -277,7 +287,7 @@ func (web *Web) tlsServerLoop() { TLSConfig: &tls.Config{ Certificates: []tls.Certificate{web.httpsServer.cert}, RootCAs: Context.tlsRoots, - CipherSuites: aghtls.SaferCipherSuites(), + CipherSuites: cipher, MinVersion: tls.VersionTLS12, }, Handler: withMiddlewares(Context.mux, limitRequestBody), From 690deb1c05cca5834b67d67c9a342b343422d7f4 Mon Sep 17 00:00:00 2001 From: Rahul Somasundaram Date: Thu, 22 Sep 2022 08:44:43 +0530 Subject: [PATCH 04/13] spelling corrected UserPreferredCipherSuites --- internal/aghtls/aghtls.go | 2 +- internal/home/web.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/aghtls/aghtls.go b/internal/aghtls/aghtls.go index 213eb8fe..209bef1d 100644 --- a/internal/aghtls/aghtls.go +++ b/internal/aghtls/aghtls.go @@ -34,7 +34,7 @@ func SaferCipherSuites() (safe []uint16) { return safe } -func UserPreferedCipherSuites(ciphers []string) (userCiphers []uint16) { +func UserPreferredCipherSuites(ciphers []string) (userCiphers []uint16) { for _, s := range tls.CipherSuites() { if slices.Contains(ciphers, s.Name) { userCiphers = append(userCiphers, s.ID) diff --git a/internal/home/web.go b/internal/home/web.go index b7157a1a..510d04f4 100644 --- a/internal/home/web.go +++ b/internal/home/web.go @@ -277,7 +277,7 @@ func (web *Web) tlsServerLoop() { if len(web.conf.tlsCiphers) == 0 { cipher = aghtls.SaferCipherSuites() } else { - cipher = aghtls.UserPreferedCipherSuites(web.conf.tlsCiphers) + cipher = aghtls.UserPreferredCipherSuites(web.conf.tlsCiphers) } // prepare HTTPS server address := netutil.JoinHostPort(web.conf.BindHost.String(), web.conf.PortHTTPS) From 9d59be4269f256bfa460c51c7573e1da46743b30 Mon Sep 17 00:00:00 2001 From: Ainar Garipov Date: Wed, 21 Sep 2022 15:02:35 +0300 Subject: [PATCH 05/13] Pull request: imp-stalebot Merge in DNS/adguard-home from imp-stalebot to master Squashed commit of the following: commit d1fb5c6da25eeb168c53abfc7af714827a5242cd Author: Ainar Garipov Date: Wed Sep 21 14:31:50 2022 +0300 all: imp stalebot --- .github/stale.yml | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/.github/stale.yml b/.github/stale.yml index 6ed9a7df..6042fc60 100644 --- a/.github/stale.yml +++ b/.github/stale.yml @@ -4,15 +4,17 @@ 'daysUntilClose': 15 # Issues with these labels will never be considered stale. 'exemptLabels': -- 'bug' -- 'documentation' -- 'enhancement' -- 'feature request' -- 'help wanted' -- 'localization' -- 'needs investigation' -- 'recurrent' -- 'research' + - 'bug' + - 'documentation' + - 'enhancement' + - 'feature request' + - 'help wanted' + - 'localization' + - 'needs investigation' + - 'recurrent' + - 'research' +# Set to true to ignore issues in a milestone. +'exemptMilestones': true # Label to use when marking an issue as stale. 'staleLabel': 'wontfix' # Comment to post when marking an issue as stale. Set to `false` to disable. @@ -22,3 +24,5 @@ for your contributions. # Comment to post when closing a stale issue. Set to `false` to disable. 'closeComment': false +# Limit the number of actions per hour. +'limitPerRun': 1 From 6e7964c9e7e8e1d999a2b402c0f60496d0cf7801 Mon Sep 17 00:00:00 2001 From: Ainar Garipov Date: Wed, 21 Sep 2022 19:21:13 +0300 Subject: [PATCH 06/13] Pull request: imp-scripts Merge in DNS/adguard-home from imp-scripts to master Squashed commit of the following: commit ab63a8a2dd1b64287e00a2a6f747fd48b530709e Author: Ainar Garipov Date: Wed Sep 21 19:15:06 2022 +0300 all: imp scripts; upd tools; doc --- internal/aghnet/ipset_linux.go | 23 +++++++---------------- internal/tools/go.mod | 10 +++++----- internal/tools/go.sum | 20 ++++++++++---------- scripts/make/go-lint.sh | 5 +---- scripts/make/version.sh | 6 +----- 5 files changed, 24 insertions(+), 40 deletions(-) diff --git a/internal/aghnet/ipset_linux.go b/internal/aghnet/ipset_linux.go index d1376b52..1c970f53 100644 --- a/internal/aghnet/ipset_linux.go +++ b/internal/aghnet/ipset_linux.go @@ -18,27 +18,18 @@ import ( // How to test on a real Linux machine: // -// 1. Run: +// 1. Run "sudo ipset create example_set hash:ip family ipv4". // -// sudo ipset create example_set hash:ip family ipv4 +// 2. Run "sudo ipset list example_set". The Members field should be empty. // -// 2. Run: +// 3. Add the line "example.com/example_set" to your AdGuardHome.yaml. // -// sudo ipset list example_set +// 4. Start AdGuardHome. // -// The Members field should be empty. +// 5. Make requests to example.com and its subdomains. // -// 3. Add the line "example.com/example_set" to your AdGuardHome.yaml. -// -// 4. Start AdGuardHome. -// -// 5. Make requests to example.com and its subdomains. -// -// 6. Run: -// -// sudo ipset list example_set -// -// The Members field should contain the resolved IP addresses. +// 6. Run "sudo ipset list example_set". The Members field should contain the +// resolved IP addresses. // newIpsetMgr returns a new Linux ipset manager. func newIpsetMgr(ipsetConf []string) (set IpsetManager, err error) { diff --git a/internal/tools/go.mod b/internal/tools/go.mod index c4f78914..4f813b69 100644 --- a/internal/tools/go.mod +++ b/internal/tools/go.mod @@ -9,8 +9,8 @@ require ( github.com/kisielk/errcheck v1.6.2 github.com/kyoh86/looppointer v0.1.7 github.com/securego/gosec/v2 v2.13.1 - golang.org/x/tools v0.1.13-0.20220803210227-8b9a1fbdf5c3 - golang.org/x/vuln v0.0.0-20220912202342-0ed43f12cb05 + golang.org/x/tools v0.1.13-0.20220921142454-16b974289fe5 + golang.org/x/vuln v0.0.0-20220921153644-d9be10b6cc84 honnef.co/go/tools v0.3.3 mvdan.cc/gofumpt v0.3.1 mvdan.cc/unparam v0.0.0-20220831102321-2fc90a84c7ec @@ -25,10 +25,10 @@ require ( github.com/kyoh86/nolint v0.0.1 // indirect github.com/nbutton23/zxcvbn-go v0.0.0-20210217022336-fa2cb2858354 // indirect github.com/xo/terminfo v0.0.0-20210125001918-ca9a967f8778 // indirect - golang.org/x/exp v0.0.0-20220909182711-5c715a9e8561 // indirect + golang.org/x/exp v0.0.0-20220921023135-46d9e7742f1e // indirect golang.org/x/exp/typeparams v0.0.0-20220827204233-334a2380cb91 // indirect - golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 // indirect + golang.org/x/mod v0.6.0-dev.0.20220907135952-02c991387e35 // indirect golang.org/x/sync v0.0.0-20220819030929-7fc1605a5dde // indirect - golang.org/x/sys v0.0.0-20220909162455-aba9fc2a8ff2 // indirect + golang.org/x/sys v0.0.0-20220919091848-fb04ddd9f9c8 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect ) diff --git a/internal/tools/go.sum b/internal/tools/go.sum index 367d3d53..d93d2b10 100644 --- a/internal/tools/go.sum +++ b/internal/tools/go.sum @@ -55,15 +55,15 @@ golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACk golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= -golang.org/x/exp v0.0.0-20220909182711-5c715a9e8561 h1:MDc5xs78ZrZr3HMQugiXOAkSZtfTpbJLDr/lwfgO53E= -golang.org/x/exp v0.0.0-20220909182711-5c715a9e8561/go.mod h1:cyybsKvd6eL0RnXn6p/Grxp8F5bW7iYuBgsNCOHpMYE= +golang.org/x/exp v0.0.0-20220921023135-46d9e7742f1e h1:Ctm9yurWsg7aWwIpH9Bnap/IdSVxixymIb3MhiMEQQA= +golang.org/x/exp v0.0.0-20220921023135-46d9e7742f1e/go.mod h1:cyybsKvd6eL0RnXn6p/Grxp8F5bW7iYuBgsNCOHpMYE= golang.org/x/exp/typeparams v0.0.0-20220827204233-334a2380cb91 h1:Ic/qN6TEifvObMGQy72k0n1LlJr7DjWWEi+MOsDOiSk= golang.org/x/exp/typeparams v0.0.0-20220827204233-334a2380cb91/go.mod h1:AbB0pIl9nAr9wVwH+Z2ZpaocVmF5I4GyWCDIsVjR0bk= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3/go.mod h1:3p9vT2HGsQu2K1YbXdKPJLVgG5VJdoTa1poYQBtP1AY= -golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 h1:6zppjxzCulZykYSLyVDYbneBfbaBIQPYMevg0bEwv2s= -golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= +golang.org/x/mod v0.6.0-dev.0.20220907135952-02c991387e35 h1:CZP0Rbk/s1EIiUMx5DS2MhK2ct52xpQxqddVD0FmF+o= +golang.org/x/mod v0.6.0-dev.0.20220907135952-02c991387e35/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20200625001655-4c5254603344/go.mod h1:/O7V0waA8r7cgGh81Ro3o1hOxt32SMVPicZroKQ2sZA= @@ -86,8 +86,8 @@ golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20211019181941-9d821ace8654/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20220909162455-aba9fc2a8ff2 h1:wM1k/lXfpc5HdkJJyW9GELpd8ERGdnh8sMGL6Gzq3Ho= -golang.org/x/sys v0.0.0-20220909162455-aba9fc2a8ff2/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220919091848-fb04ddd9f9c8 h1:h+EGohizhe9XlX18rfpa8k8RAc5XyaeamM+0VHRd4lc= +golang.org/x/sys v0.0.0-20220919091848-fb04ddd9f9c8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= @@ -100,10 +100,10 @@ golang.org/x/tools v0.0.0-20200710042808-f1c4188a97a1/go.mod h1:njjCfa9FT2d7l9Bc golang.org/x/tools v0.0.0-20201007032633-0806396f153e/go.mod h1:z6u4i615ZeAfBE4XtMziQW1fSVJXACjjbWkB/mvPzlU= golang.org/x/tools v0.1.5/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.10/go.mod h1:Uh6Zz+xoGYZom868N8YTex3t7RhtHDBrE8Gzo9bV56E= -golang.org/x/tools v0.1.13-0.20220803210227-8b9a1fbdf5c3 h1:aE4T3aJwdCNz+s35ScSQYUzeGu7BOLDHZ1bBHVurqqY= -golang.org/x/tools v0.1.13-0.20220803210227-8b9a1fbdf5c3/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= -golang.org/x/vuln v0.0.0-20220912202342-0ed43f12cb05 h1:NWQHMTdThZhCArzUbnu1Bh+l3LdwUfjZws+ivBR2sxM= -golang.org/x/vuln v0.0.0-20220912202342-0ed43f12cb05/go.mod h1:7tDfEDtOLlzHQRi4Yzfg5seVBSvouUIjyPzBx4q5CxQ= +golang.org/x/tools v0.1.13-0.20220921142454-16b974289fe5 h1:o1LhIiY5L+hLK9DWqfFlilCrpZnw/s7WU4iCUkb/bao= +golang.org/x/tools v0.1.13-0.20220921142454-16b974289fe5/go.mod h1:VsjNM1dMo+Ofkp5d7y7fOdQZD8MTXSQ4w3EPk65AvKU= +golang.org/x/vuln v0.0.0-20220921153644-d9be10b6cc84 h1:L0qUjdplndgX880fozFRGC242wAtfsViyRXWGlpZQ54= +golang.org/x/vuln v0.0.0-20220921153644-d9be10b6cc84/go.mod h1:7tDfEDtOLlzHQRi4Yzfg5seVBSvouUIjyPzBx4q5CxQ= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/scripts/make/go-lint.sh b/scripts/make/go-lint.sh index f6f09592..2cdcc90d 100644 --- a/scripts/make/go-lint.sh +++ b/scripts/make/go-lint.sh @@ -219,10 +219,7 @@ exit_on_output gofumpt --extra -e -l . "$GO" vet ./... -# TODO(a.garipov): Reenable this once https://github.com/golang/go/issues/55035 -# is fixed. -# -# govulncheck ./... +govulncheck ./... # Apply more lax standards to the code we haven't properly refactored yet. gocyclo --over 17 ./internal/querylog/ diff --git a/scripts/make/version.sh b/scripts/make/version.sh index 903be7bf..68e84e9c 100644 --- a/scripts/make/version.sh +++ b/scripts/make/version.sh @@ -85,11 +85,7 @@ in # num_commits_since_minor is the number of commits since the last new # minor release. If the current commit is the new minor release, # num_commits_since_minor is zero. - num_commits_since_minor="$( git rev-list "${last_minor_zero}..HEAD" | wc -l )" - - # The output of darwin's implementation of wc needs to be trimmed from - # redundant spaces. - num_commits_since_minor="$( echo "$num_commits_since_minor" | tr -d '[:space:]' )" + num_commits_since_minor="$( git rev-list --count "${last_minor_zero}..HEAD" )" readonly num_commits_since_minor # next_minor is the next minor release version. From 24eb3476db53d9f5dc69b927238ee8ac1196e3c3 Mon Sep 17 00:00:00 2001 From: Rahul Somasundaram Date: Tue, 4 Oct 2022 09:51:55 +0530 Subject: [PATCH 07/13] added ciphers for h3 --- internal/home/web.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/home/web.go b/internal/home/web.go index d1193885..d10e8dd5 100644 --- a/internal/home/web.go +++ b/internal/home/web.go @@ -318,7 +318,7 @@ func (web *Web) tlsServerLoop() { printHTTPAddresses(aghhttp.SchemeHTTPS) if web.conf.serveHTTP3 { - go web.mustStartHTTP3(addr) + go web.mustStartHTTP3(addr, cipher) } log.Debug("web: starting https server") @@ -330,7 +330,7 @@ func (web *Web) tlsServerLoop() { } } -func (web *Web) mustStartHTTP3(address string) { +func (web *Web) mustStartHTTP3(address string, ciphers []uint16) { defer log.OnPanic("web: http3") web.httpsServer.server3 = &http3.Server{ @@ -340,7 +340,7 @@ func (web *Web) mustStartHTTP3(address string) { TLSConfig: &tls.Config{ Certificates: []tls.Certificate{web.httpsServer.cert}, RootCAs: Context.tlsRoots, - CipherSuites: aghtls.SaferCipherSuites(), + CipherSuites: ciphers, MinVersion: tls.VersionTLS12, }, Handler: withMiddlewares(Context.mux, limitRequestBody), From 15b19ff7268da689933dd369862930afc98fcc8e Mon Sep 17 00:00:00 2001 From: Rahul Somasundaram Date: Wed, 5 Oct 2022 00:12:53 +0530 Subject: [PATCH 08/13] changes done as per review comments --- CHANGELOG.md | 13 +++++++++++-- internal/aghtls/aghtls.go | 5 ++++- internal/dnsforward/config.go | 6 ++++-- internal/home/home.go | 15 ++++++++++++++- internal/home/web.go | 24 ++++++++---------------- 5 files changed, 41 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 71b93339..35b3a259 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,9 +22,18 @@ and this project adheres to See also the [v0.107.16 GitHub milestone][ms-v0.107.15]. -[ms-v0.107.16]: https://github.com/AdguardTeam/AdGuardHome/milestone/52?closed=1 ---> +[ms-v0.107.16]: https://github.com/AdguardTeam/AdGuardHome/milestone/52?closed= +### Added + +- The new optional `tls.override_tls_ciphers` property list, which can be set in + the configuration file. It allows overriding TLS Ciphers that are used for + https listeners ([#4925]) + +[#4925]: https://github.com/AdguardTeam/AdGuardHome/issues/4925 + + +--> ## [v0.107.15] - 2022-10-03 diff --git a/internal/aghtls/aghtls.go b/internal/aghtls/aghtls.go index 209bef1d..4bd20a01 100644 --- a/internal/aghtls/aghtls.go +++ b/internal/aghtls/aghtls.go @@ -34,11 +34,14 @@ func SaferCipherSuites() (safe []uint16) { return safe } -func UserPreferredCipherSuites(ciphers []string) (userCiphers []uint16) { +// ParseCipherIDs returns a set of cipher suites with the cipher names provided +func ParseCipherIDs(ciphers []string) (userCiphers []uint16) { for _, s := range tls.CipherSuites() { if slices.Contains(ciphers, s.Name) { userCiphers = append(userCiphers, s.ID) log.Debug("user specified cipher : %s, ID : %d", s.Name, s.ID) + } else { + log.Error("unknown cipher : %s ", s) } } diff --git a/internal/dnsforward/config.go b/internal/dnsforward/config.go index 3a70bbeb..9994f1cb 100644 --- a/internal/dnsforward/config.go +++ b/internal/dnsforward/config.go @@ -166,8 +166,10 @@ type TLSConfig struct { // DNS names from certificate (SAN) or CN value from Subject dnsNames []string - // ciphers specified by user - TLSCiphers []string `yaml:"tls_ciphers" json:"-"` + // OverrideTLSCiphers holds the cipher names. If the slice is empty + // default set of ciphers are used for https listener, else this is + // considered. + OverrideTLSCiphers []string `yaml:"override_tls_ciphers" json:"-"` } // DNSCryptConfig is the DNSCrypt server configuration struct. diff --git a/internal/home/home.go b/internal/home/home.go index 0b325298..fad2c695 100644 --- a/internal/home/home.go +++ b/internal/home/home.go @@ -383,7 +383,7 @@ func initWeb(args options, clientBuildFS fs.FS) (web *Web, err error) { clientBetaFS: clientBetaFS, serveHTTP3: config.DNS.ServeHTTP3, - tlsCiphers: config.TLS.TLSCiphers, + tlsCiphers: getTLSCiphers(), } web = newWeb(&webConf) @@ -888,3 +888,16 @@ type jsonError struct { // Message is the error message, an opaque string. Message string `json:"message"` } + +// getTLSCiphers check for overriden tls ciphers, if the slice is +// empty, then default safe ciphers are used +func getTLSCiphers() []uint16 { + var cipher []uint16 + + if len(config.TLS.OverrideTLSCiphers) == 0 { + cipher = aghtls.SaferCipherSuites() + } else { + cipher = aghtls.ParseCipherIDs(config.TLS.OverrideTLSCiphers) + } + return cipher +} diff --git a/internal/home/web.go b/internal/home/web.go index d10e8dd5..5ece2819 100644 --- a/internal/home/web.go +++ b/internal/home/web.go @@ -11,7 +11,6 @@ import ( "github.com/AdguardTeam/AdGuardHome/internal/aghhttp" "github.com/AdguardTeam/AdGuardHome/internal/aghnet" - "github.com/AdguardTeam/AdGuardHome/internal/aghtls" "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/log" "github.com/AdguardTeam/golibs/netutil" @@ -34,6 +33,10 @@ const ( ) type webConfig struct { + + // Ciphers that are used for https listener + tlsCiphers []uint16 + clientFS fs.FS clientBetaFS fs.FS @@ -57,9 +60,6 @@ type webConfig struct { firstRun bool serveHTTP3 bool - - // ciphers specified by user - tlsCiphers []string } // httpsServer contains the data for the HTTPS server. @@ -291,14 +291,6 @@ func (web *Web) tlsServerLoop() { web.httpsServer.cond.L.Unlock() - var cipher []uint16 - - if len(web.conf.tlsCiphers) == 0 { - cipher = aghtls.SaferCipherSuites() - } else { - cipher = aghtls.UserPreferredCipherSuites(web.conf.tlsCiphers) - } - addr := netutil.JoinHostPort(web.conf.BindHost.String(), web.conf.PortHTTPS) web.httpsServer.server = &http.Server{ ErrorLog: log.StdLog("web: https", log.DEBUG), @@ -306,7 +298,7 @@ func (web *Web) tlsServerLoop() { TLSConfig: &tls.Config{ Certificates: []tls.Certificate{web.httpsServer.cert}, RootCAs: Context.tlsRoots, - CipherSuites: cipher, + CipherSuites: web.conf.tlsCiphers, MinVersion: tls.VersionTLS12, }, Handler: withMiddlewares(Context.mux, limitRequestBody), @@ -318,7 +310,7 @@ func (web *Web) tlsServerLoop() { printHTTPAddresses(aghhttp.SchemeHTTPS) if web.conf.serveHTTP3 { - go web.mustStartHTTP3(addr, cipher) + go web.mustStartHTTP3(addr) } log.Debug("web: starting https server") @@ -330,7 +322,7 @@ func (web *Web) tlsServerLoop() { } } -func (web *Web) mustStartHTTP3(address string, ciphers []uint16) { +func (web *Web) mustStartHTTP3(address string) { defer log.OnPanic("web: http3") web.httpsServer.server3 = &http3.Server{ @@ -340,7 +332,7 @@ func (web *Web) mustStartHTTP3(address string, ciphers []uint16) { TLSConfig: &tls.Config{ Certificates: []tls.Certificate{web.httpsServer.cert}, RootCAs: Context.tlsRoots, - CipherSuites: ciphers, + CipherSuites: web.conf.tlsCiphers, MinVersion: tls.VersionTLS12, }, Handler: withMiddlewares(Context.mux, limitRequestBody), From 7cac010573a46a8815baf5bf91b44e40f366b671 Mon Sep 17 00:00:00 2001 From: Rahul Somasundaram Date: Thu, 6 Oct 2022 21:37:15 +0530 Subject: [PATCH 09/13] changed based on review 1. exit AG is user defined cipher is invalid 2. updated changelog 3. golang naming tweaks --- CHANGELOG.md | 18 +++++++++--------- internal/aghtls/aghtls.go | 29 +++++++++++++++++++---------- internal/home/home.go | 19 +++++++++++-------- internal/home/web.go | 1 - 4 files changed, 39 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 35b3a259..06535d8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,15 @@ and this project adheres to ## [Unreleased] + +### Added + +- The new optional `tls.override_tls_ciphers` property list, which can be set in + the configuration file. It allows overriding TLS Ciphers that are used for + https listeners ([#4925]) + +[#4925]: https://github.com/AdguardTeam/AdGuardHome/issues/4925 + @@ -24,15 +33,6 @@ See also the [v0.107.16 GitHub milestone][ms-v0.107.15]. [ms-v0.107.16]: https://github.com/AdguardTeam/AdGuardHome/milestone/52?closed= -### Added - -- The new optional `tls.override_tls_ciphers` property list, which can be set in - the configuration file. It allows overriding TLS Ciphers that are used for - https listeners ([#4925]) - -[#4925]: https://github.com/AdguardTeam/AdGuardHome/issues/4925 - - --> diff --git a/internal/aghtls/aghtls.go b/internal/aghtls/aghtls.go index 4bd20a01..708d6599 100644 --- a/internal/aghtls/aghtls.go +++ b/internal/aghtls/aghtls.go @@ -3,9 +3,7 @@ package aghtls import ( "crypto/tls" - - "github.com/AdguardTeam/golibs/log" - "golang.org/x/exp/slices" + "fmt" ) // SaferCipherSuites returns a set of default cipher suites with vulnerable and @@ -35,15 +33,26 @@ func SaferCipherSuites() (safe []uint16) { } // ParseCipherIDs returns a set of cipher suites with the cipher names provided -func ParseCipherIDs(ciphers []string) (userCiphers []uint16) { - for _, s := range tls.CipherSuites() { - if slices.Contains(ciphers, s.Name) { - userCiphers = append(userCiphers, s.ID) - log.Debug("user specified cipher : %s, ID : %d", s.Name, s.ID) +func ParseCipherIDs(ciphers []string) (userCiphers []uint16, err error) { + for _, cipher := range ciphers { + exists, cipherID := CipherExists(cipher) + if exists { + userCiphers = append(userCiphers, cipherID) } else { - log.Error("unknown cipher : %s ", s) + return nil, fmt.Errorf("unknown cipher : %s ", cipher) } } - return userCiphers + return userCiphers, nil +} + +// CipherExists returns cipherid if exists, else return false in boolean +func CipherExists(cipher string) (exists bool, cipherID uint16) { + for _, s := range tls.CipherSuites() { + if s.Name == cipher { + return true, s.ID + } + } + + return false, 0 } diff --git a/internal/home/home.go b/internal/home/home.go index fad2c695..49e348a5 100644 --- a/internal/home/home.go +++ b/internal/home/home.go @@ -369,6 +369,11 @@ func initWeb(args options, clientBuildFS fs.FS) (web *Web, err error) { } } + tlsCiphers, err := getTLSCiphers() + if err != nil { + return nil, err + } + webConf := webConfig{ firstRun: Context.firstRun, BindHost: config.BindHost, @@ -383,7 +388,7 @@ func initWeb(args options, clientBuildFS fs.FS) (web *Web, err error) { clientBetaFS: clientBetaFS, serveHTTP3: config.DNS.ServeHTTP3, - tlsCiphers: getTLSCiphers(), + tlsCiphers: tlsCiphers, } web = newWeb(&webConf) @@ -889,15 +894,13 @@ type jsonError struct { Message string `json:"message"` } -// getTLSCiphers check for overriden tls ciphers, if the slice is +// getTLSCiphers check for overridden tls ciphers, if the slice is // empty, then default safe ciphers are used -func getTLSCiphers() []uint16 { - var cipher []uint16 - +func getTLSCiphers() (cipherIds []uint16, err error) { if len(config.TLS.OverrideTLSCiphers) == 0 { - cipher = aghtls.SaferCipherSuites() + return aghtls.SaferCipherSuites(), nil } else { - cipher = aghtls.ParseCipherIDs(config.TLS.OverrideTLSCiphers) + log.Info("Overriding TLS Ciphers : %s", config.TLS.OverrideTLSCiphers) + return aghtls.ParseCipherIDs(config.TLS.OverrideTLSCiphers) } - return cipher } diff --git a/internal/home/web.go b/internal/home/web.go index 5ece2819..9a94bead 100644 --- a/internal/home/web.go +++ b/internal/home/web.go @@ -33,7 +33,6 @@ const ( ) type webConfig struct { - // Ciphers that are used for https listener tlsCiphers []uint16 From c0c9d8adb0209f53c945172be7fcfd9d1cca1747 Mon Sep 17 00:00:00 2001 From: Rahul Somasundaram Date: Thu, 6 Oct 2022 21:44:43 +0530 Subject: [PATCH 10/13] fixed formatting --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 06535d8d..ff1129e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,7 +32,6 @@ and this project adheres to See also the [v0.107.16 GitHub milestone][ms-v0.107.15]. [ms-v0.107.16]: https://github.com/AdguardTeam/AdGuardHome/milestone/52?closed= - --> From a126f514ff60aab1587c714f8fee8fbd6c59318b Mon Sep 17 00:00:00 2001 From: Rahul Somasundaram Date: Thu, 6 Oct 2022 21:48:22 +0530 Subject: [PATCH 11/13] updated changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ff1129e0..51adea62 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,7 +31,7 @@ and this project adheres to See also the [v0.107.16 GitHub milestone][ms-v0.107.15]. -[ms-v0.107.16]: https://github.com/AdguardTeam/AdGuardHome/milestone/52?closed= +[ms-v0.107.16]: https://github.com/AdguardTeam/AdGuardHome/milestone/52?closed=1 --> From 5ae826d8a99c0906a09e595459fe8418926c63fc Mon Sep 17 00:00:00 2001 From: Ainar Garipov Date: Fri, 14 Oct 2022 20:14:07 +0300 Subject: [PATCH 12/13] home: refactor override --- internal/dnsforward/config.go | 30 +++++++++++++----------------- internal/home/config.go | 27 +++++++++++++++++++++++++++ internal/home/home.go | 23 +++++------------------ internal/home/web.go | 7 ++----- 4 files changed, 47 insertions(+), 40 deletions(-) diff --git a/internal/dnsforward/config.go b/internal/dnsforward/config.go index 9994f1cb..caad6547 100644 --- a/internal/dnsforward/config.go +++ b/internal/dnsforward/config.go @@ -12,7 +12,6 @@ import ( "github.com/AdguardTeam/AdGuardHome/internal/aghalg" "github.com/AdguardTeam/AdGuardHome/internal/aghhttp" - "github.com/AdguardTeam/AdGuardHome/internal/aghtls" "github.com/AdguardTeam/AdGuardHome/internal/filtering" "github.com/AdguardTeam/dnsproxy/proxy" "github.com/AdguardTeam/dnsproxy/upstream" @@ -166,10 +165,9 @@ type TLSConfig struct { // DNS names from certificate (SAN) or CN value from Subject dnsNames []string - // OverrideTLSCiphers holds the cipher names. If the slice is empty - // default set of ciphers are used for https listener, else this is - // considered. - OverrideTLSCiphers []string `yaml:"override_tls_ciphers" json:"-"` + // OverrideTLSCiphers, when set, contains the names of the cipher suites to + // use. If the slice is empty, the default safe suites are used. + OverrideTLSCiphers []string `yaml:"override_tls_ciphers,omitempty" json:"-"` } // DNSCryptConfig is the DNSCrypt server configuration struct. @@ -198,7 +196,9 @@ type ServerConfig struct { UpstreamTimeout time.Duration TLSv12Roots *x509.CertPool // list of root CAs for TLSv1.2 - TLSCiphers []uint16 // list of TLS ciphers to use + + // TLSCiphers are the IDs of TLS cipher suites to use. + TLSCiphers []uint16 // Called when the configuration is changed by HTTP request ConfigModified func() @@ -353,17 +353,13 @@ func UpstreamHTTPVersions(http3 bool) (v []upstream.HTTPVersion) { // prepareUpstreamSettings - prepares upstream DNS server settings func (s *Server) prepareUpstreamSettings() error { - // We're setting a customized set of RootCAs - // The reason is that Go default mechanism of loading TLS roots - // does not always work properly on some routers so we're - // loading roots manually and pass it here. - // See "util.LoadSystemRootCAs" + // We're setting a customized set of RootCAs. The reason is that Go default + // mechanism of loading TLS roots does not always work properly on some + // routers so we're loading roots manually and pass it here. + // + // See [aghtls.SystemRootCAs]. upstream.RootCAs = s.conf.TLSv12Roots - - // See util.InitTLSCiphers -- removed unsafe ciphers - if len(s.conf.TLSCiphers) > 0 { - upstream.CipherSuites = s.conf.TLSCiphers - } + upstream.CipherSuites = s.conf.TLSCiphers // Load upstreams either from the file, or from the settings var upstreams []string @@ -499,7 +495,7 @@ func (s *Server) prepareTLS(proxyConfig *proxy.Config) error { proxyConfig.TLSConfig = &tls.Config{ GetCertificate: s.onGetCertificate, - CipherSuites: aghtls.SaferCipherSuites(), + CipherSuites: s.conf.TLSCiphers, MinVersion: tls.VersionTLS12, } diff --git a/internal/home/config.go b/internal/home/config.go index 7df4f853..c7198d93 100644 --- a/internal/home/config.go +++ b/internal/home/config.go @@ -9,6 +9,7 @@ import ( "sync" "github.com/AdguardTeam/AdGuardHome/internal/aghalg" + "github.com/AdguardTeam/AdGuardHome/internal/aghtls" "github.com/AdguardTeam/AdGuardHome/internal/dhcpd" "github.com/AdguardTeam/AdGuardHome/internal/dnsforward" "github.com/AdguardTeam/AdGuardHome/internal/filtering" @@ -380,6 +381,7 @@ func parseConfig() (err error) { // we add support for HTTP/3 for web admin interface. addPorts(udpPorts, udpPort(config.TLS.PortDNSOverQUIC)) } + if err = tcpPorts.Validate(); err != nil { return fmt.Errorf("validating tcp ports: %w", err) } else if err = udpPorts.Validate(); err != nil { @@ -394,6 +396,11 @@ func parseConfig() (err error) { config.DNS.UpstreamTimeout = timeutil.Duration{Duration: dnsforward.DefaultTimeout} } + err = setContextTLSCipherIDs() + if err != nil { + return err + } + return nil } @@ -496,3 +503,23 @@ func (c *configuration) write() (err error) { return nil } + +// setContextTLSCipherIDs sets the TLS cipher suite IDs to use. +func setContextTLSCipherIDs() (err error) { + if len(config.TLS.OverrideTLSCiphers) == 0 { + log.Info("tls: using default ciphers") + + Context.tlsCipherIDs = aghtls.SaferCipherSuites() + + return nil + } + + log.Info("tls: overriding ciphers: %s", config.TLS.OverrideTLSCiphers) + + Context.tlsCipherIDs, err = aghtls.ParseCiphers(config.TLS.OverrideTLSCiphers) + if err != nil { + return fmt.Errorf("parsing override ciphers: %w", err) + } + + return nil +} diff --git a/internal/home/home.go b/internal/home/home.go index c8bd992e..5d3582e9 100644 --- a/internal/home/home.go +++ b/internal/home/home.go @@ -84,6 +84,10 @@ type homeContext struct { transport *http.Transport client *http.Client appSignalChannel chan os.Signal // Channel for receiving OS signals by the console app + + // tlsCipherIDs are the ID of the cipher suites that AdGuard Home must use. + tlsCipherIDs []uint16 + // runningAsService flag is set to true when options are passed from the service runner runningAsService bool } @@ -153,7 +157,7 @@ func setupContext(opts options) { Proxy: getHTTPProxy, TLSClientConfig: &tls.Config{ RootCAs: Context.tlsRoots, - CipherSuites: aghtls.SaferCipherSuites(), + CipherSuites: Context.tlsCipherIDs, MinVersion: tls.VersionTLS12, }, } @@ -386,11 +390,6 @@ func initWeb(opts options, clientBuildFS fs.FS) (web *Web, err error) { } } - tlsCiphers, err := getTLSCiphers() - if err != nil { - return nil, err - } - webConf := webConfig{ firstRun: Context.firstRun, BindHost: config.BindHost, @@ -405,7 +404,6 @@ func initWeb(opts options, clientBuildFS fs.FS) (web *Web, err error) { clientBetaFS: clientBetaFS, serveHTTP3: config.DNS.ServeHTTP3, - tlsCiphers: tlsCiphers, } web = newWeb(&webConf) @@ -916,14 +914,3 @@ type jsonError struct { // Message is the error message, an opaque string. Message string `json:"message"` } - -// getTLSCiphers check for overridden tls ciphers, if the slice is -// empty, then default safe ciphers are used -func getTLSCiphers() (cipherIds []uint16, err error) { - if len(config.TLS.OverrideTLSCiphers) == 0 { - return aghtls.SaferCipherSuites(), nil - } else { - log.Info("Overriding TLS Ciphers : %s", config.TLS.OverrideTLSCiphers) - return aghtls.ParseCiphers(config.TLS.OverrideTLSCiphers) - } -} diff --git a/internal/home/web.go b/internal/home/web.go index 86700357..7836355f 100644 --- a/internal/home/web.go +++ b/internal/home/web.go @@ -33,9 +33,6 @@ const ( ) type webConfig struct { - // Ciphers that are used for https listener - tlsCiphers []uint16 - clientFS fs.FS clientBetaFS fs.FS @@ -300,7 +297,7 @@ func (web *Web) tlsServerLoop() { TLSConfig: &tls.Config{ Certificates: []tls.Certificate{web.httpsServer.cert}, RootCAs: Context.tlsRoots, - CipherSuites: web.conf.tlsCiphers, + CipherSuites: Context.tlsCipherIDs, MinVersion: tls.VersionTLS12, }, Handler: withMiddlewares(Context.mux, limitRequestBody), @@ -334,7 +331,7 @@ func (web *Web) mustStartHTTP3(address string) { TLSConfig: &tls.Config{ Certificates: []tls.Certificate{web.httpsServer.cert}, RootCAs: Context.tlsRoots, - CipherSuites: web.conf.tlsCiphers, + CipherSuites: Context.tlsCipherIDs, MinVersion: tls.VersionTLS12, }, Handler: withMiddlewares(Context.mux, limitRequestBody), From ab79168b13a5680557147de098c740bca974392e Mon Sep 17 00:00:00 2001 From: Ainar Garipov Date: Fri, 14 Oct 2022 20:19:25 +0300 Subject: [PATCH 13/13] all: fix chlog --- CHANGELOG.md | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bbeec92c..e8af7f0a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,20 +54,10 @@ and this project adheres to See also the [v0.107.17 GitHub milestone][ms-v0.107.17]. -<<<<<<< HEAD -[ms-v0.107.16]: https://github.com/AdguardTeam/AdGuardHome/milestone/52?closed=1 -||||||| bf792b83f -[ms-v0.107.16]: https://github.com/AdguardTeam/AdGuardHome/milestone/52?closed=1 -======= -[ms-v0.107.17]: https://github.com/AdguardTeam/AdGuardHome/milestone/52?closed=1 ->>>>>>> master +[ms-v0.107.17]: https://github.com/AdguardTeam/AdGuardHome/milestone/52?closed=1 --> -<<<<<<< HEAD -||||||| bf792b83f - -======= ## [v0.107.16] - 2022-10-07 @@ -84,7 +74,6 @@ were resolved. ->>>>>>> master ## [v0.107.15] - 2022-10-03 See also the [v0.107.15 GitHub milestone][ms-v0.107.15].