Skip to content

Commit f40c049

Browse files
committed
[release-branch.go1.10] all: merge release-branch.go1.10-security into release-branch.go1.10
Change-Id: I7048387350b683030d9f3106979370b0d096ea38
2 parents 1ae7397 + 25ca8f4 commit f40c049

File tree

8 files changed

+386
-60
lines changed

8 files changed

+386
-60
lines changed

VERSION

+1-1
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
go1.10.5
1+
go1.10.6

doc/devel/release.html

+7
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,13 @@ <h3 id="go1.10.minor">Minor revisions</h3>
7272
1.10.5 milestone</a> on our issue tracker for details.
7373
</p>
7474

75+
<p>
76+
go1.10.6 (released 2018/12/12) includes three security fixes to "go get" and
77+
the <code>crypto/x509</code> package.
78+
See the <a href="https://github.com/golang/go/issues?q=milestone%3AGo1.10.6">Go
79+
1.10.6 milestone</a> on our issue tracker for details.
80+
</p>
81+
7582
<h2 id="go1.9">go1.9 (released 2017/08/24)</h2>
7683

7784
<p>

src/cmd/go/internal/get/get.go

+4
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,10 @@ func downloadPackage(p *load.Package) error {
376376
security = web.Insecure
377377
}
378378

379+
if err := CheckImportPath(p.ImportPath); err != nil {
380+
return fmt.Errorf("%s: invalid import path: %v", p.ImportPath, err)
381+
}
382+
379383
if p.Internal.Build.SrcRoot != "" {
380384
// Directory exists. Look for checkout along path to src.
381385
vcs, rootPath, err = vcsFromDir(p.Dir, p.Internal.Build.SrcRoot)

src/cmd/go/internal/get/path.go

+192
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,192 @@
1+
// Copyright 2018 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package get
6+
7+
import (
8+
"fmt"
9+
"strings"
10+
"unicode"
11+
"unicode/utf8"
12+
)
13+
14+
// The following functions are copied verbatim from cmd/go/internal/module/module.go,
15+
// with a change to additionally reject Windows short-names,
16+
// and one to accept arbitrary letters (golang.org/issue/29101).
17+
//
18+
// TODO(bcmills): After the call site for this function is backported,
19+
// consolidate this back down to a single copy.
20+
//
21+
// NOTE: DO NOT MERGE THESE UNTIL WE DECIDE ABOUT ARBITRARY LETTERS IN MODULE MODE.
22+
23+
// CheckImportPath checks that an import path is valid.
24+
func CheckImportPath(path string) error {
25+
if err := checkPath(path, false); err != nil {
26+
return fmt.Errorf("malformed import path %q: %v", path, err)
27+
}
28+
return nil
29+
}
30+
31+
// checkPath checks that a general path is valid.
32+
// It returns an error describing why but not mentioning path.
33+
// Because these checks apply to both module paths and import paths,
34+
// the caller is expected to add the "malformed ___ path %q: " prefix.
35+
// fileName indicates whether the final element of the path is a file name
36+
// (as opposed to a directory name).
37+
func checkPath(path string, fileName bool) error {
38+
if !utf8.ValidString(path) {
39+
return fmt.Errorf("invalid UTF-8")
40+
}
41+
if path == "" {
42+
return fmt.Errorf("empty string")
43+
}
44+
if strings.Contains(path, "..") {
45+
return fmt.Errorf("double dot")
46+
}
47+
if strings.Contains(path, "//") {
48+
return fmt.Errorf("double slash")
49+
}
50+
if path[len(path)-1] == '/' {
51+
return fmt.Errorf("trailing slash")
52+
}
53+
elemStart := 0
54+
for i, r := range path {
55+
if r == '/' {
56+
if err := checkElem(path[elemStart:i], fileName); err != nil {
57+
return err
58+
}
59+
elemStart = i + 1
60+
}
61+
}
62+
if err := checkElem(path[elemStart:], fileName); err != nil {
63+
return err
64+
}
65+
return nil
66+
}
67+
68+
// checkElem checks whether an individual path element is valid.
69+
// fileName indicates whether the element is a file name (not a directory name).
70+
func checkElem(elem string, fileName bool) error {
71+
if elem == "" {
72+
return fmt.Errorf("empty path element")
73+
}
74+
if strings.Count(elem, ".") == len(elem) {
75+
return fmt.Errorf("invalid path element %q", elem)
76+
}
77+
if elem[0] == '.' && !fileName {
78+
return fmt.Errorf("leading dot in path element")
79+
}
80+
if elem[len(elem)-1] == '.' {
81+
return fmt.Errorf("trailing dot in path element")
82+
}
83+
84+
charOK := pathOK
85+
if fileName {
86+
charOK = fileNameOK
87+
}
88+
for _, r := range elem {
89+
if !charOK(r) {
90+
return fmt.Errorf("invalid char %q", r)
91+
}
92+
}
93+
94+
// Windows disallows a bunch of path elements, sadly.
95+
// See https://docs.microsoft.com/en-us/windows/desktop/fileio/naming-a-file
96+
short := elem
97+
if i := strings.Index(short, "."); i >= 0 {
98+
short = short[:i]
99+
}
100+
for _, bad := range badWindowsNames {
101+
if strings.EqualFold(bad, short) {
102+
return fmt.Errorf("disallowed path element %q", elem)
103+
}
104+
}
105+
106+
// Reject path components that look like Windows short-names.
107+
// Those usually end in a tilde followed by one or more ASCII digits.
108+
if tilde := strings.LastIndexByte(short, '~'); tilde >= 0 && tilde < len(short)-1 {
109+
suffix := short[tilde+1:]
110+
suffixIsDigits := true
111+
for _, r := range suffix {
112+
if r < '0' || r > '9' {
113+
suffixIsDigits = false
114+
break
115+
}
116+
}
117+
if suffixIsDigits {
118+
return fmt.Errorf("trailing tilde and digits in path element")
119+
}
120+
}
121+
122+
return nil
123+
}
124+
125+
// pathOK reports whether r can appear in an import path element.
126+
//
127+
// NOTE: This function DIVERGES from module mode pathOK by accepting Unicode letters.
128+
func pathOK(r rune) bool {
129+
if r < utf8.RuneSelf {
130+
return r == '+' || r == '-' || r == '.' || r == '_' || r == '~' ||
131+
'0' <= r && r <= '9' ||
132+
'A' <= r && r <= 'Z' ||
133+
'a' <= r && r <= 'z'
134+
}
135+
return unicode.IsLetter(r)
136+
}
137+
138+
// fileNameOK reports whether r can appear in a file name.
139+
// For now we allow all Unicode letters but otherwise limit to pathOK plus a few more punctuation characters.
140+
// If we expand the set of allowed characters here, we have to
141+
// work harder at detecting potential case-folding and normalization collisions.
142+
// See note about "safe encoding" below.
143+
func fileNameOK(r rune) bool {
144+
if r < utf8.RuneSelf {
145+
// Entire set of ASCII punctuation, from which we remove characters:
146+
// ! " # $ % & ' ( ) * + , - . / : ; < = > ? @ [ \ ] ^ _ ` { | } ~
147+
// We disallow some shell special characters: " ' * < > ? ` |
148+
// (Note that some of those are disallowed by the Windows file system as well.)
149+
// We also disallow path separators / : and \ (fileNameOK is only called on path element characters).
150+
// We allow spaces (U+0020) in file names.
151+
const allowed = "!#$%&()+,-.=@[]^_{}~ "
152+
if '0' <= r && r <= '9' || 'A' <= r && r <= 'Z' || 'a' <= r && r <= 'z' {
153+
return true
154+
}
155+
for i := 0; i < len(allowed); i++ {
156+
if rune(allowed[i]) == r {
157+
return true
158+
}
159+
}
160+
return false
161+
}
162+
// It may be OK to add more ASCII punctuation here, but only carefully.
163+
// For example Windows disallows < > \, and macOS disallows :, so we must not allow those.
164+
return unicode.IsLetter(r)
165+
}
166+
167+
// badWindowsNames are the reserved file path elements on Windows.
168+
// See https://docs.microsoft.com/en-us/windows/desktop/fileio/naming-a-file
169+
var badWindowsNames = []string{
170+
"CON",
171+
"PRN",
172+
"AUX",
173+
"NUL",
174+
"COM1",
175+
"COM2",
176+
"COM3",
177+
"COM4",
178+
"COM5",
179+
"COM6",
180+
"COM7",
181+
"COM8",
182+
"COM9",
183+
"LPT1",
184+
"LPT2",
185+
"LPT3",
186+
"LPT4",
187+
"LPT5",
188+
"LPT6",
189+
"LPT7",
190+
"LPT8",
191+
"LPT9",
192+
}

src/cmd/go/internal/get/vcs.go

+6-2
Original file line numberDiff line numberDiff line change
@@ -970,10 +970,14 @@ func matchGoImport(imports []metaImport, importPath string) (metaImport, error)
970970

971971
// expand rewrites s to replace {k} with match[k] for each key k in match.
972972
func expand(match map[string]string, s string) string {
973+
// We want to replace each match exactly once, and the result of expansion
974+
// must not depend on the iteration order through the map.
975+
// A strings.Replacer has exactly the properties we're looking for.
976+
oldNew := make([]string, 0, 2*len(match))
973977
for k, v := range match {
974-
s = strings.Replace(s, "{"+k+"}", v, -1)
978+
oldNew = append(oldNew, "{"+k+"}", v)
975979
}
976-
return s
980+
return strings.NewReplacer(oldNew...).Replace(s)
977981
}
978982

979983
// vcsPaths defines the meaning of import paths referring to

src/crypto/x509/cert_pool.go

+6-22
Original file line numberDiff line numberDiff line change
@@ -38,32 +38,16 @@ func SystemCertPool() (*CertPool, error) {
3838
return loadSystemRoots()
3939
}
4040

41-
// findVerifiedParents attempts to find certificates in s which have signed the
42-
// given certificate. If any candidates were rejected then errCert will be set
43-
// to one of them, arbitrarily, and err will contain the reason that it was
44-
// rejected.
45-
func (s *CertPool) findVerifiedParents(cert *Certificate) (parents []int, errCert *Certificate, err error) {
41+
// findPotentialParents returns the indexes of certificates in s which might
42+
// have signed cert. The caller must not modify the returned slice.
43+
func (s *CertPool) findPotentialParents(cert *Certificate) []int {
4644
if s == nil {
47-
return
45+
return nil
4846
}
49-
var candidates []int
50-
5147
if len(cert.AuthorityKeyId) > 0 {
52-
candidates = s.bySubjectKeyId[string(cert.AuthorityKeyId)]
53-
}
54-
if len(candidates) == 0 {
55-
candidates = s.byName[string(cert.RawIssuer)]
48+
return s.bySubjectKeyId[string(cert.AuthorityKeyId)]
5649
}
57-
58-
for _, c := range candidates {
59-
if err = cert.CheckSignatureFrom(s.certs[c]); err == nil {
60-
parents = append(parents, c)
61-
} else {
62-
errCert = s.certs[c]
63-
}
64-
}
65-
66-
return
50+
return s.byName[string(cert.RawIssuer)]
6751
}
6852

6953
func (s *CertPool) contains(cert *Certificate) bool {

src/crypto/x509/verify.go

+51-35
Original file line numberDiff line numberDiff line change
@@ -765,7 +765,7 @@ func (c *Certificate) Verify(opts VerifyOptions) (chains [][]*Certificate, err e
765765
if opts.Roots.contains(c) {
766766
candidateChains = append(candidateChains, []*Certificate{c})
767767
} else {
768-
if candidateChains, err = c.buildChains(make(map[int][][]*Certificate), []*Certificate{c}, &opts); err != nil {
768+
if candidateChains, err = c.buildChains(nil, []*Certificate{c}, nil, &opts); err != nil {
769769
return nil, err
770770
}
771771
}
@@ -802,58 +802,74 @@ func appendToFreshChain(chain []*Certificate, cert *Certificate) []*Certificate
802802
return n
803803
}
804804

805-
func (c *Certificate) buildChains(cache map[int][][]*Certificate, currentChain []*Certificate, opts *VerifyOptions) (chains [][]*Certificate, err error) {
806-
possibleRoots, failedRoot, rootErr := opts.Roots.findVerifiedParents(c)
807-
nextRoot:
808-
for _, rootNum := range possibleRoots {
809-
root := opts.Roots.certs[rootNum]
805+
// maxChainSignatureChecks is the maximum number of CheckSignatureFrom calls
806+
// that an invocation of buildChains will (tranistively) make. Most chains are
807+
// less than 15 certificates long, so this leaves space for multiple chains and
808+
// for failed checks due to different intermediates having the same Subject.
809+
const maxChainSignatureChecks = 100
810810

811+
func (c *Certificate) buildChains(cache map[*Certificate][][]*Certificate, currentChain []*Certificate, sigChecks *int, opts *VerifyOptions) (chains [][]*Certificate, err error) {
812+
var (
813+
hintErr error
814+
hintCert *Certificate
815+
)
816+
817+
considerCandidate := func(certType int, candidate *Certificate) {
811818
for _, cert := range currentChain {
812-
if cert.Equal(root) {
813-
continue nextRoot
819+
if cert.Equal(candidate) {
820+
return
814821
}
815822
}
816823

817-
err = root.isValid(rootCertificate, currentChain, opts)
818-
if err != nil {
819-
continue
824+
if sigChecks == nil {
825+
sigChecks = new(int)
826+
}
827+
*sigChecks++
828+
if *sigChecks > maxChainSignatureChecks {
829+
err = errors.New("x509: signature check attempts limit reached while verifying certificate chain")
830+
return
820831
}
821-
chains = append(chains, appendToFreshChain(currentChain, root))
822-
}
823832

824-
possibleIntermediates, failedIntermediate, intermediateErr := opts.Intermediates.findVerifiedParents(c)
825-
nextIntermediate:
826-
for _, intermediateNum := range possibleIntermediates {
827-
intermediate := opts.Intermediates.certs[intermediateNum]
828-
for _, cert := range currentChain {
829-
if cert.Equal(intermediate) {
830-
continue nextIntermediate
833+
if err := c.CheckSignatureFrom(candidate); err != nil {
834+
if hintErr == nil {
835+
hintErr = err
836+
hintCert = candidate
831837
}
838+
return
832839
}
833-
err = intermediate.isValid(intermediateCertificate, currentChain, opts)
840+
841+
err = candidate.isValid(certType, currentChain, opts)
834842
if err != nil {
835-
continue
843+
return
836844
}
837-
var childChains [][]*Certificate
838-
childChains, ok := cache[intermediateNum]
839-
if !ok {
840-
childChains, err = intermediate.buildChains(cache, appendToFreshChain(currentChain, intermediate), opts)
841-
cache[intermediateNum] = childChains
845+
846+
switch certType {
847+
case rootCertificate:
848+
chains = append(chains, appendToFreshChain(currentChain, candidate))
849+
case intermediateCertificate:
850+
if cache == nil {
851+
cache = make(map[*Certificate][][]*Certificate)
852+
}
853+
childChains, ok := cache[candidate]
854+
if !ok {
855+
childChains, err = candidate.buildChains(cache, appendToFreshChain(currentChain, candidate), sigChecks, opts)
856+
cache[candidate] = childChains
857+
}
858+
chains = append(chains, childChains...)
842859
}
843-
chains = append(chains, childChains...)
860+
}
861+
862+
for _, rootNum := range opts.Roots.findPotentialParents(c) {
863+
considerCandidate(rootCertificate, opts.Roots.certs[rootNum])
864+
}
865+
for _, intermediateNum := range opts.Intermediates.findPotentialParents(c) {
866+
considerCandidate(intermediateCertificate, opts.Intermediates.certs[intermediateNum])
844867
}
845868

846869
if len(chains) > 0 {
847870
err = nil
848871
}
849-
850872
if len(chains) == 0 && err == nil {
851-
hintErr := rootErr
852-
hintCert := failedRoot
853-
if hintErr == nil {
854-
hintErr = intermediateErr
855-
hintCert = failedIntermediate
856-
}
857873
err = UnknownAuthorityError{c, hintErr, hintCert}
858874
}
859875

0 commit comments

Comments
 (0)