From 4fce1545664bf3eed93c787762b171b14251c093 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C4=8Cajka?= Date: Tue, 10 Oct 2017 13:43:37 +0200 Subject: [PATCH] fix CVE-2017-15041 and CVE-2017-15042 --- CVE-2017-15041.patch | 127 ++++++++++++++++++++++++++++++++++++++ CVE-2017-15042.patch | 144 +++++++++++++++++++++++++++++++++++++++++++ golang.spec | 11 +++- 3 files changed, 281 insertions(+), 1 deletion(-) create mode 100644 CVE-2017-15041.patch create mode 100644 CVE-2017-15042.patch diff --git a/CVE-2017-15041.patch b/CVE-2017-15041.patch new file mode 100644 index 0000000..4c4747c --- /dev/null +++ b/CVE-2017-15041.patch @@ -0,0 +1,127 @@ +diff -up go/src/cmd/go/get.go.cve go/src/cmd/go/get.go +--- go/src/cmd/go/get.go.cve 2017-05-23 20:35:22.000000000 +0200 ++++ go/src/cmd/go/get.go 2017-10-10 10:25:24.485047705 +0200 +@@ -401,6 +401,11 @@ func downloadPackage(p *Package) error { + p.build.PkgRoot = filepath.Join(list[0], "pkg") + } + root := filepath.Join(p.build.SrcRoot, filepath.FromSlash(rootPath)) ++ ++ if err := checkNestedVCS(vcs, root, p.build.SrcRoot); err != nil { ++ return err ++ } ++ + // If we've considered this repository already, don't do it again. + if downloadRootCache[root] { + return nil +diff -up go/src/cmd/go/go_test.go.cve go/src/cmd/go/go_test.go +--- go/src/cmd/go/go_test.go.cve 2017-05-23 20:35:22.000000000 +0200 ++++ go/src/cmd/go/go_test.go 2017-10-10 10:25:24.485047705 +0200 +@@ -1235,6 +1235,25 @@ func TestGetGitDefaultBranch(t *testing. + tg.grepStdout(`\* another-branch`, "not on correct default branch") + } + ++func TestAccidentalGitCheckout(t *testing.T) { ++ testenv.MustHaveExternalNetwork(t) ++ if _, err := exec.LookPath("git"); err != nil { ++ t.Skip("skipping because git binary not found") ++ } ++ ++ tg := testgo(t) ++ defer tg.cleanup() ++ tg.parallel() ++ tg.tempDir("src") ++ tg.setenv("GOPATH", tg.path(".")) ++ ++ tg.runFail("get", "-u", "vcs-test.golang.org/go/test1-svn-git") ++ tg.grepStderr("src[\\\\/]vcs-test.* uses git, but parent .*src[\\\\/]vcs-test.* uses svn", "get did not fail for right reason") ++ ++ tg.runFail("get", "-u", "vcs-test.golang.org/go/test2-svn-git/test2main") ++ tg.grepStderr("src[\\\\/]vcs-test.* uses git, but parent .*src[\\\\/]vcs-test.* uses svn", "get did not fail for right reason") ++} ++ + func TestErrorMessageForSyntaxErrorInTestGoFileSaysFAIL(t *testing.T) { + tg := testgo(t) + defer tg.cleanup() +diff -up go/src/cmd/go/vcs.go.cve go/src/cmd/go/vcs.go +--- go/src/cmd/go/vcs.go.cve 2017-05-23 20:35:22.000000000 +0200 ++++ go/src/cmd/go/vcs.go 2017-10-10 10:30:52.151621206 +0200 +@@ -479,11 +479,29 @@ func vcsFromDir(dir, srcRoot string) (vc + return nil, "", fmt.Errorf("directory %q is outside source root %q", dir, srcRoot) + } + ++ var vcsRet *vcsCmd ++ var rootRet string ++ + origDir := dir + for len(dir) > len(srcRoot) { + for _, vcs := range vcsList { + if fi, err := os.Stat(filepath.Join(dir, "."+vcs.cmd)); err == nil && fi.IsDir() { +- return vcs, filepath.ToSlash(dir[len(srcRoot)+1:]), nil ++ root := filepath.ToSlash(dir[len(srcRoot)+1:]) ++ // Record first VCS we find, but keep looking, ++ // to detect mistakes like one kind of VCS inside another. ++ if vcsRet == nil { ++ vcsRet = vcs ++ rootRet = root ++ continue ++ } ++ // Allow .git inside .git, which can arise due to submodules. ++ if vcsRet == vcs && vcs.cmd == "git" { ++ continue ++ } ++ // Otherwise, we have one VCS inside a different VCS. ++ return nil, "", fmt.Errorf("directory %q uses %s, but parent %q uses %s", ++ filepath.Join(srcRoot, rootRet), vcsRet.cmd, filepath.Join(srcRoot, root), vcs.cmd) ++ + } + } + +@@ -496,9 +514,48 @@ func vcsFromDir(dir, srcRoot string) (vc + dir = ndir + } + ++ if vcsRet != nil { ++ return vcsRet, rootRet, nil ++ } ++ + return nil, "", fmt.Errorf("directory %q is not using a known version control system", origDir) + } + ++// checkNestedVCS checks for an incorrectly-nested VCS-inside-VCS ++// situation for dir, checking parents up until srcRoot. ++func checkNestedVCS(vcs *vcsCmd, dir, srcRoot string) error { ++ if len(dir) <= len(srcRoot) || dir[len(srcRoot)] != filepath.Separator { ++ return fmt.Errorf("directory %q is outside source root %q", dir, srcRoot) ++ } ++ ++ otherDir := dir ++ for len(otherDir) > len(srcRoot) { ++ for _, otherVCS := range vcsList { ++ if _, err := os.Stat(filepath.Join(dir, "."+otherVCS.cmd)); err == nil { ++ // Allow expected vcs in original dir. ++ if otherDir == dir && otherVCS == vcs { ++ continue ++ } ++ // Allow .git inside .git, which can arise due to submodules. ++ if otherVCS == vcs && vcs.cmd == "git" { ++ continue ++ } ++ // Otherwise, we have one VCS inside a different VCS. ++ return fmt.Errorf("directory %q uses %s, but parent %q uses %s", dir, vcs.cmd, otherDir, otherVCS.cmd) ++ } ++ } ++ // Move to parent. ++ newDir := filepath.Dir(otherDir) ++ if len(newDir) >= len(otherDir) { ++ // Shouldn't happen, but just in case, stop. ++ break ++ } ++ otherDir = newDir ++ } ++ ++ return nil ++} ++ + // repoRoot represents a version control system, a repo, and a root of + // where to put it on disk. + type repoRoot struct { diff --git a/CVE-2017-15042.patch b/CVE-2017-15042.patch new file mode 100644 index 0000000..aca6049 --- /dev/null +++ b/CVE-2017-15042.patch @@ -0,0 +1,144 @@ +From 4be3fc33ef512532b916aa14258087e89eb47347 Mon Sep 17 00:00:00 2001 +From: Russ Cox +Date: Wed, 4 Oct 2017 13:24:49 -0400 +Subject: [PATCH] [release-branch.go1.8] net/smtp: fix PlainAuth to refuse to + send passwords to non-TLS servers + +PlainAuth originally refused to send passwords to non-TLS servers +and was documented as such. + +In 2013, issue #5184 was filed objecting to the TLS requirement, +despite the fact that it is spelled out clearly in RFC 4954. +The only possibly legitimate use case raised was using PLAIN auth +for connections to localhost, and the suggested fix was to let the +server decide: if it advertises that PLAIN auth is OK, believe it. +That approach was adopted in CL 8279043 and released in Go 1.1. + +Unfortunately, this is exactly wrong. The whole point of the TLS +requirement is to make sure not to send the password to the wrong +server or to a man-in-the-middle. Instead of implementing this rule, +CL 8279043 blindly trusts the server, so that if a man-in-the-middle +says "it's OK, you can send me your password," PlainAuth does. +And the documentation was not updated to reflect any of this. + +This CL restores the original TLS check, as required by RFC 4954 +and as promised in the documentation for PlainAuth. +It then carves out a documented exception for connections made +to localhost (defined as "localhost", "127.0.0.1", or "::1"). + +Cherry-pick of CL 68170. + +Change-Id: I1d3729bbd33aa2f11a03f4c000e6bb473164957b +Reviewed-on: https://go-review.googlesource.com/68023 +Run-TryBot: Russ Cox +Reviewed-by: Chris Broadfoot +--- + src/net/smtp/auth.go | 33 ++++++++++++++++++--------------- + src/net/smtp/smtp_test.go | 32 ++++++++++++++++++++++---------- + 2 files changed, 40 insertions(+), 25 deletions(-) + +diff --git a/src/net/smtp/auth.go b/src/net/smtp/auth.go +index 3f1339ebc56..fd1a472f930 100644 +--- a/src/net/smtp/auth.go ++++ b/src/net/smtp/auth.go +@@ -44,26 +44,29 @@ type plainAuth struct { + } + + // PlainAuth returns an Auth that implements the PLAIN authentication +-// mechanism as defined in RFC 4616. +-// The returned Auth uses the given username and password to authenticate +-// on TLS connections to host and act as identity. Usually identity will be +-// left blank to act as username. ++// mechanism as defined in RFC 4616. The returned Auth uses the given ++// username and password to authenticate to host and act as identity. ++// Usually identity should be the empty string, to act as username. ++// ++// PlainAuth will only send the credentials if the connection is using TLS ++// or is connected to localhost. Otherwise authentication will fail with an ++// error, without sending the credentials. + func PlainAuth(identity, username, password, host string) Auth { + return &plainAuth{identity, username, password, host} + } + ++func isLocalhost(name string) bool { ++ return name == "localhost" || name == "127.0.0.1" || name == "::1" ++} ++ + func (a *plainAuth) Start(server *ServerInfo) (string, []byte, error) { +- if !server.TLS { +- advertised := false +- for _, mechanism := range server.Auth { +- if mechanism == "PLAIN" { +- advertised = true +- break +- } +- } +- if !advertised { +- return "", nil, errors.New("unencrypted connection") +- } ++ // Must have TLS, or else localhost server. ++ // Note: If TLS is not true, then we can't trust ANYTHING in ServerInfo. ++ // In particular, it doesn't matter if the server advertises PLAIN auth. ++ // That might just be the attacker saying ++ // "it's ok, you can trust me with your password." ++ if !server.TLS && !isLocalhost(server.Name) { ++ return "", nil, errors.New("unencrypted connection") + } + if server.Name != a.host { + return "", nil, errors.New("wrong host name") +diff --git a/src/net/smtp/smtp_test.go b/src/net/smtp/smtp_test.go +index c48fae6d5ac..15eaca524be 100644 +--- a/src/net/smtp/smtp_test.go ++++ b/src/net/smtp/smtp_test.go +@@ -60,29 +60,41 @@ testLoop: + } + + func TestAuthPlain(t *testing.T) { +- auth := PlainAuth("foo", "bar", "baz", "servername") + + tests := []struct { +- server *ServerInfo +- err string ++ authName string ++ server *ServerInfo ++ err string + }{ + { +- server: &ServerInfo{Name: "servername", TLS: true}, ++ authName: "servername", ++ server: &ServerInfo{Name: "servername", TLS: true}, + }, + { +- // Okay; explicitly advertised by server. +- server: &ServerInfo{Name: "servername", Auth: []string{"PLAIN"}}, ++ // OK to use PlainAuth on localhost without TLS ++ authName: "localhost", ++ server: &ServerInfo{Name: "localhost", TLS: false}, + }, + { +- server: &ServerInfo{Name: "servername", Auth: []string{"CRAM-MD5"}}, +- err: "unencrypted connection", ++ // NOT OK on non-localhost, even if server says PLAIN is OK. ++ // (We don't know that the server is the real server.) ++ authName: "servername", ++ server: &ServerInfo{Name: "servername", Auth: []string{"PLAIN"}}, ++ err: "unencrypted connection", + }, + { +- server: &ServerInfo{Name: "attacker", TLS: true}, +- err: "wrong host name", ++ authName: "servername", ++ server: &ServerInfo{Name: "servername", Auth: []string{"CRAM-MD5"}}, ++ err: "unencrypted connection", ++ }, ++ { ++ authName: "servername", ++ server: &ServerInfo{Name: "attacker", TLS: true}, ++ err: "wrong host name", + }, + } + for i, tt := range tests { ++ auth := PlainAuth("foo", "bar", "baz", tt.authName) + _, _, err := auth.Start(tt.server) + got := "" + if err != nil { diff --git a/golang.spec b/golang.spec index b4b11fb..16ec9c0 100644 --- a/golang.spec +++ b/golang.spec @@ -98,7 +98,7 @@ Name: golang Version: 1.7.6 -Release: 2%{?dist} +Release: 3%{?dist} Summary: The Go Programming Language # source tree includes several copies of Mark.Twain-Tom.Sawyer.txt under Public Domain License: BSD and Public Domain @@ -149,6 +149,9 @@ Patch220: tzdata-fix.patch # https://github.com/golang/go/commit/94aba76639cf4d5e30975d846bb0368db8202269 Patch221: 31bit-OID-asn1.patch +Patch222: CVE-2017-15041.patch +Patch223: CVE-2017-15042.patch + # Having documentation separate was broken Obsoletes: %{name}-docs < 1.1-4 @@ -290,6 +293,9 @@ Requires: %{name} = %{version}-%{release} %patch220 -p1 %patch221 -p1 +%patch222 -p1 +%patch223 -p1 + %build # print out system information uname -a @@ -522,6 +528,9 @@ fi %endif %changelog +* Tue Oct 10 2017 Jakub Čajka - 1.7.6-3 +- fix CVE-2017-15041 and CVE-2017-15042 + * Thu Jun 29 2017 Jakub Čajka - 1.7.6-2 - add race subpackage