diff options
author | Dmitri Shuralyov <dmitshur@golang.org> | 2020-01-30 19:14:03 -0500 |
---|---|---|
committer | Dmitri Shuralyov <dmitshur@golang.org> | 2020-01-30 19:14:28 -0500 |
commit | 249c282caf075fda88658ef6e11fb90067f238b0 (patch) | |
tree | c42763636bc2fcd3e8535fd68cca5a86258a1f2d | |
parent | 4af1337d1e9eb9e7b766c9deb787c78413bb25c4 (diff) | |
parent | deac3221fc4cd365fb40d269dd56551e9d354356 (diff) | |
download | go-249c282caf075fda88658ef6e11fb90067f238b0.tar.gz go-249c282caf075fda88658ef6e11fb90067f238b0.zip |
[release-branch.go1.12] all: merge release-branch.go1.12-security into release-branch.go1.12
Change-Id: Ic8ed07ad2c77042a67d7e1d4e9c0d5953610cf07
-rw-r--r-- | VERSION | 2 | ||||
-rw-r--r-- | doc/devel/release.html | 7 | ||||
-rw-r--r-- | src/crypto/x509/root_windows.go | 20 | ||||
-rw-r--r-- | src/internal/x/crypto/cryptobyte/asn1.go | 5 | ||||
-rw-r--r-- | src/internal/x/crypto/cryptobyte/asn1_test.go | 4 | ||||
-rw-r--r-- | src/internal/x/crypto/cryptobyte/string.go | 7 |
6 files changed, 34 insertions, 11 deletions
@@ -1 +1 @@ -go1.12.15
\ No newline at end of file +go1.12.16
\ No newline at end of file diff --git a/doc/devel/release.html b/doc/devel/release.html index 487367c35f..38a9040d5e 100644 --- a/doc/devel/release.html +++ b/doc/devel/release.html @@ -139,6 +139,13 @@ the <a href="https://github.com/golang/go/issues?q=milestone%3AGo1.12.15+label%3 1.12.15 milestone</a> on our issue tracker for details. </p> +<p> +go1.12.16 (released 2020/01/28) includes two security fixes to +the <code>crypto/x509</code> package. See the +<a href="https://github.com/golang/go/issues?q=milestone%3AGo1.12.16+label%3ACherryPickApproved">Go +1.12.16 milestone</a> on our issue tracker for details. +</p> + <h2 id="go1.11">go1.11 (released 2018/08/24)</h2> <p> diff --git a/src/crypto/x509/root_windows.go b/src/crypto/x509/root_windows.go index 74d395df70..3da3d06e73 100644 --- a/src/crypto/x509/root_windows.go +++ b/src/crypto/x509/root_windows.go @@ -219,10 +219,26 @@ func (c *Certificate) systemVerify(opts *VerifyOptions) (chains [][]*Certificate if err != nil { return nil, err } + if len(chain) < 1 { + return nil, errors.New("x509: internal error: system verifier returned an empty chain") + } - chains = append(chains, chain) + // Mitigate CVE-2020-0601, where the Windows system verifier might be + // tricked into using custom curve parameters for a trusted root, by + // double-checking all ECDSA signatures. If the system was tricked into + // using spoofed parameters, the signature will be invalid for the correct + // ones we parsed. (We don't support custom curves ourselves.) + for i, parent := range chain[1:] { + if parent.PublicKeyAlgorithm != ECDSA { + continue + } + if err := parent.CheckSignature(chain[i].SignatureAlgorithm, + chain[i].RawTBSCertificate, chain[i].Signature); err != nil { + return nil, err + } + } - return chains, nil + return [][]*Certificate{chain}, nil } func loadSystemRoots() (*CertPool, error) { diff --git a/src/internal/x/crypto/cryptobyte/asn1.go b/src/internal/x/crypto/cryptobyte/asn1.go index 2d40680ddd..758ac3a649 100644 --- a/src/internal/x/crypto/cryptobyte/asn1.go +++ b/src/internal/x/crypto/cryptobyte/asn1.go @@ -470,7 +470,8 @@ func (s *String) ReadASN1GeneralizedTime(out *time.Time) bool { // It reports whether the read was successful. func (s *String) ReadASN1BitString(out *encoding_asn1.BitString) bool { var bytes String - if !s.ReadASN1(&bytes, asn1.BIT_STRING) || len(bytes) == 0 { + if !s.ReadASN1(&bytes, asn1.BIT_STRING) || len(bytes) == 0 || + len(bytes)*8/8 != len(bytes) { return false } @@ -740,7 +741,7 @@ func (s *String) readASN1(out *String, outTag *asn1.Tag, skipHeader bool) bool { length = headerLen + len32 } - if uint32(int(length)) != length || !s.ReadBytes((*[]byte)(out), int(length)) { + if int(length) < 0 || !s.ReadBytes((*[]byte)(out), int(length)) { return false } if skipHeader && !out.Skip(int(headerLen)) { diff --git a/src/internal/x/crypto/cryptobyte/asn1_test.go b/src/internal/x/crypto/cryptobyte/asn1_test.go index ca28e3bcfb..f90d768edb 100644 --- a/src/internal/x/crypto/cryptobyte/asn1_test.go +++ b/src/internal/x/crypto/cryptobyte/asn1_test.go @@ -31,6 +31,10 @@ var readASN1TestData = []readASN1Test{ {"non-minimal length", append([]byte{0x30, 0x82, 0, 0x80}, make([]byte, 0x80)...), 0x30, false, nil}, {"invalid tag", []byte{0xa1, 3, 0x4, 1, 1}, 31, false, nil}, {"high tag", []byte{0x1f, 0x81, 0x80, 0x01, 2, 1, 2}, 0xff /* actually 0x4001, but tag is uint8 */, false, nil}, + {"2**31 - 1 length", []byte{0x30, 0x84, 0x7f, 0xff, 0xff, 0xff}, 0x30, false, nil}, + {"2**32 - 1 length", []byte{0x30, 0x84, 0xff, 0xff, 0xff, 0xff}, 0x30, false, nil}, + {"2**63 - 1 length", []byte{0x30, 0x88, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, 0x30, false, nil}, + {"2**64 - 1 length", []byte{0x30, 0x88, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, 0x30, false, nil}, } func TestReadASN1(t *testing.T) { diff --git a/src/internal/x/crypto/cryptobyte/string.go b/src/internal/x/crypto/cryptobyte/string.go index bd2ed2e207..a3ecf63828 100644 --- a/src/internal/x/crypto/cryptobyte/string.go +++ b/src/internal/x/crypto/cryptobyte/string.go @@ -24,7 +24,7 @@ type String []byte // read advances a String by n bytes and returns them. If less than n bytes // remain, it returns nil. func (s *String) read(n int) []byte { - if len(*s) < n { + if len(*s) < n || n < 0 { return nil } v := (*s)[:n] @@ -105,11 +105,6 @@ func (s *String) readLengthPrefixed(lenLen int, outChild *String) bool { length = length << 8 length = length | uint32(b) } - if int(length) < 0 { - // This currently cannot overflow because we read uint24 at most, but check - // anyway in case that changes in the future. - return false - } v := s.read(int(length)) if v == nil { return false |