fix CVE-2017-15041 and CVE-2017-15042
This commit is contained in:
parent
fbbbf12bd3
commit
4fce154566
|
@ -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 {
|
|
@ -0,0 +1,144 @@
|
|||
From 4be3fc33ef512532b916aa14258087e89eb47347 Mon Sep 17 00:00:00 2001
|
||||
From: Russ Cox <rsc@golang.org>
|
||||
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 <rsc@golang.org>
|
||||
Reviewed-by: Chris Broadfoot <cbro@golang.org>
|
||||
---
|
||||
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 {
|
11
golang.spec
11
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 <jcajka@redhat.com> - 1.7.6-3
|
||||
- fix CVE-2017-15041 and CVE-2017-15042
|
||||
|
||||
* Thu Jun 29 2017 Jakub Čajka <jcajka@redhat.com> - 1.7.6-2
|
||||
- add race subpackage
|
||||
|
||||
|
|
Loading…
Reference in New Issue