rpms
/
golang
2
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

fix CVE-2017-15041 and CVE-2017-15042

This commit is contained in:
Jakub Čajka 2017-10-13 14:13:08 +02:00
parent 5c3bfeff0a
commit dc8fe7c9ce
3 changed files with 275 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

127
CVE-2017-15041.patch Normal file
View File

@ -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 {

144
CVE-2017-15042.patch Normal file
View File

@ -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 {

5
golang.spec
View File

@ -48,7 +48,7 @@
Name: golang
Version: 1.7.6
Release: 1%{?dist}
Release: 2%{?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
@ -443,6 +443,9 @@ fi
%endif
%changelog
* Fri Oct 13 2017 Jakub Čajka <jcajka@fedoraproject.org> - 1.7.4-2
- fix CVE-2017-15041 and CVE-2017-15042
* Thu Jun 15 2017 Jakub Čajka <jcajka@fedoraproject.org> - 1.7.4-1
- Bump to 1.7.6
- Fix for CVE-2017-8932