[Pkg-golang-commits] [golang] 01/01: Apply backported CVE fixes (CVE-2015-5739, CVE-2015-5740, CVE-2015-5741)
Tianon Gravi
tianon at debian.org
Mon Sep 14 19:17:20 UTC 2015
This is an automated email from the git hooks/post-receive script.
tianon pushed a commit to branch debian-sid
in repository golang.
commit 7a89becc1bcdd09f7834880e3512605529d58e7b
Author: Tianon Gravi <tianon at debian.org>
Date: Mon Sep 14 12:17:08 2015 -0700
Apply backported CVE fixes (CVE-2015-5739, CVE-2015-5740, CVE-2015-5741)
---
debian/changelog | 9 +
debian/patches/cve-2015-5739-5740-5741.patch | 364 +++++++++++++++++++++++++++
debian/patches/series | 1 +
3 files changed, 374 insertions(+)
diff --git a/debian/changelog b/debian/changelog
index fa10914..0482a05 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,12 @@
+golang (2:1.4.2-4) UNRELEASED; urgency=high
+
+ * Apply backported CVE fixes.
+ - CVE-2015-5739: Invalid headers are parsed as valid headers
+ - CVE-2015-5740: RFC 7230 3.3.3 4 violation
+ - CVE-2015-5741: other discoveries of security-relevant RFC 7230 violations
+
+ -- Tianon Gravi <tianon at debian.org> Mon, 14 Sep 2015 12:09:53 -0700
+
golang (2:1.4.2-3) unstable; urgency=medium
* Add missing "prerm" for our new alternatives (thanks piuparts).
diff --git a/debian/patches/cve-2015-5739-5740-5741.patch b/debian/patches/cve-2015-5739-5740-5741.patch
new file mode 100644
index 0000000..2a03d13
--- /dev/null
+++ b/debian/patches/cve-2015-5739-5740-5741.patch
@@ -0,0 +1,364 @@
+Description: CVE-2015-5739 CVE-2015-5740 CVE-2015-5741
+Author: Tianon Gravi <admwiggin at gmail.com>
+Applied-Upstream: 1.5+
+
+Index: golang/src/net/http/header.go
+===================================================================
+--- golang.orig/src/net/http/header.go
++++ golang/src/net/http/header.go
+@@ -168,6 +168,8 @@ func (h Header) WriteSubset(w io.Writer,
+ // letter and any letter following a hyphen to upper case;
+ // the rest are converted to lowercase. For example, the
+ // canonical key for "accept-encoding" is "Accept-Encoding".
++// If s contains a space or invalid header field bytes, it is
++// returned without modifications.
+ func CanonicalHeaderKey(s string) string { return textproto.CanonicalMIMEHeaderKey(s) }
+
+ // hasToken reports whether token appears with v, ASCII
+Index: golang/src/net/textproto/reader.go
+===================================================================
+--- golang.orig/src/net/textproto/reader.go
++++ golang/src/net/textproto/reader.go
+@@ -540,11 +540,16 @@ func (r *Reader) upcomingHeaderNewlines(
+ // the rest are converted to lowercase. For example, the
+ // canonical key for "accept-encoding" is "Accept-Encoding".
+ // MIME header keys are assumed to be ASCII only.
++// If s contains a space or invalid header field bytes, it is
++// returned without modifications.
+ func CanonicalMIMEHeaderKey(s string) string {
+ // Quick check for canonical encoding.
+ upper := true
+ for i := 0; i < len(s); i++ {
+ c := s[i]
++ if !validHeaderFieldByte(c) {
++ return s
++ }
+ if upper && 'a' <= c && c <= 'z' {
+ return canonicalMIMEHeaderKey([]byte(s))
+ }
+@@ -558,19 +563,44 @@ func CanonicalMIMEHeaderKey(s string) st
+
+ const toLower = 'a' - 'A'
+
++// validHeaderFieldByte reports whether b is a valid byte in a header
++// field key. This is actually stricter than RFC 7230, which says:
++// tchar = "!" / "#" / "$" / "%" / "&" / "'" / "*" / "+" / "-" / "." /
++// "^" / "_" / "`" / "|" / "~" / DIGIT / ALPHA
++// token = 1*tchar
++// TODO: revisit in Go 1.6+ and possibly expand this. But note that many
++// servers have historically dropped '_' to prevent ambiguities when mapping
++// to CGI environment variables.
++func validHeaderFieldByte(b byte) bool {
++ return ('A' <= b && b <= 'Z') ||
++ ('a' <= b && b <= 'z') ||
++ ('0' <= b && b <= '9') ||
++ b == '-'
++}
++
+ // canonicalMIMEHeaderKey is like CanonicalMIMEHeaderKey but is
+ // allowed to mutate the provided byte slice before returning the
+ // string.
++//
++// For invalid inputs (if a contains spaces or non-token bytes), a
++// is unchanged and a string copy is returned.
+ func canonicalMIMEHeaderKey(a []byte) string {
++ // See if a looks like a header key. If not, return it unchanged.
++ for _, c := range a {
++ if validHeaderFieldByte(c) {
++ continue
++ }
++ // Don't canonicalize.
++ return string(a)
++ }
++
+ upper := true
+ for i, c := range a {
+ // Canonicalize: first letter upper case
+ // and upper case after each dash.
+ // (Host, User-Agent, If-Modified-Since).
+ // MIME headers are ASCII only, so no Unicode issues.
+- if c == ' ' {
+- c = '-'
+- } else if upper && 'a' <= c && c <= 'z' {
++ if upper && 'a' <= c && c <= 'z' {
+ c -= toLower
+ } else if !upper && 'A' <= c && c <= 'Z' {
+ c += toLower
+Index: golang/src/net/textproto/reader_test.go
+===================================================================
+--- golang.orig/src/net/textproto/reader_test.go
++++ golang/src/net/textproto/reader_test.go
+@@ -24,11 +24,14 @@ var canonicalHeaderKeyTests = []canonica
+ {"uSER-aGENT", "User-Agent"},
+ {"user-agent", "User-Agent"},
+ {"USER-AGENT", "User-Agent"},
+- {"üser-agenT", "üser-Agent"}, // non-ASCII unchanged
++
++ // Non-ASCII or anything with spaces or non-token chars is unchanged:
++ {"üser-agenT", "üser-agenT"},
++ {"a B", "a B"},
+
+ // This caused a panic due to mishandling of a space:
+- {"C Ontent-Transfer-Encoding", "C-Ontent-Transfer-Encoding"},
+- {"foo bar", "Foo-Bar"},
++ {"C Ontent-Transfer-Encoding", "C Ontent-Transfer-Encoding"},
++ {"foo bar", "foo bar"},
+ }
+
+ func TestCanonicalMIMEHeaderKey(t *testing.T) {
+@@ -185,7 +188,7 @@ func TestReadMIMEHeaderNonCompliant(t *t
+ "Foo": {"bar"},
+ "Content-Language": {"en"},
+ "Sid": {"0"},
+- "Audio-Mode": {"None"},
++ "Audio Mode": {"None"},
+ "Privilege": {"127"},
+ }
+ if !reflect.DeepEqual(m, want) || err != nil {
+Index: golang/src/net/http/readrequest_test.go
+===================================================================
+--- golang.orig/src/net/http/readrequest_test.go
++++ golang/src/net/http/readrequest_test.go
+@@ -9,6 +9,7 @@ import (
+ "bytes"
+ "fmt"
+ "io"
++ "io/ioutil"
+ "net/url"
+ "reflect"
+ "strings"
+@@ -177,6 +178,36 @@ var reqTests = []reqTest{
+ noError,
+ },
+
++ // Tests chunked body and a bogus Content-Length which should be deleted.
++ {
++ "POST / HTTP/1.1\r\n" +
++ "Host: foo.com\r\n" +
++ "Transfer-Encoding: chunked\r\n" +
++ "Content-Length: 9999\r\n\r\n" + // to be removed.
++ "3\r\nfoo\r\n" +
++ "3\r\nbar\r\n" +
++ "0\r\n" +
++ "\r\n",
++ &Request{
++ Method: "POST",
++ URL: &url.URL{
++ Path: "/",
++ },
++ TransferEncoding: []string{"chunked"},
++ Proto: "HTTP/1.1",
++ ProtoMajor: 1,
++ ProtoMinor: 1,
++ Header: Header{},
++ ContentLength: -1,
++ Host: "foo.com",
++ RequestURI: "/",
++ },
++
++ "foobar",
++ noTrailer,
++ noError,
++ },
++
+ // CONNECT request with domain name:
+ {
+ "CONNECT www.google.com:443 HTTP/1.1\r\n\r\n",
+@@ -323,6 +354,32 @@ var reqTests = []reqTest{
+ noTrailer,
+ noError,
+ },
++
++ // HEAD with Content-Length 0. Make sure this is permitted,
++ // since I think we used to send it.
++ {
++ "HEAD / HTTP/1.1\r\nHost: issue8261.com\r\nConnection: close\r\nContent-Length: 0\r\n\r\n",
++ &Request{
++ Method: "HEAD",
++ URL: &url.URL{
++ Path: "/",
++ },
++ Header: Header{
++ "Connection": []string{"close"},
++ "Content-Length": []string{"0"},
++ },
++ Host: "issue8261.com",
++ Proto: "HTTP/1.1",
++ ProtoMajor: 1,
++ ProtoMinor: 1,
++ Close: true,
++ RequestURI: "/",
++ },
++
++ noBody,
++ noTrailer,
++ noError,
++ },
+ }
+
+ func TestReadRequest(t *testing.T) {
+@@ -356,3 +413,34 @@ func TestReadRequest(t *testing.T) {
+ }
+ }
+ }
++
++// reqBytes treats req as a request (with \n delimiters) and returns it with \r\n delimiters,
++// ending in \r\n\r\n
++func reqBytes(req string) []byte {
++ return []byte(strings.Replace(strings.TrimSpace(req), "\n", "\r\n", -1) + "\r\n\r\n")
++}
++
++var badRequestTests = []struct {
++ name string
++ req []byte
++}{
++ {"bad_connect_host", reqBytes("CONNECT []%20%48%54%54%50%2f%31%2e%31%0a%4d%79%48%65%61%64%65%72%3a%20%31%32%33%0a%0a HTTP/1.0")},
++ {"smuggle_two_contentlen", reqBytes(`POST / HTTP/1.1
++Content-Length: 3
++Content-Length: 4
++
++abc`)},
++ {"smuggle_content_len_head", reqBytes(`HEAD / HTTP/1.1
++Host: foo
++Content-Length: 5`)},
++}
++
++func TestReadRequest_Bad(t *testing.T) {
++ for _, tt := range badRequestTests {
++ got, err := ReadRequest(bufio.NewReader(bytes.NewReader(tt.req)))
++ if err == nil {
++ all, err := ioutil.ReadAll(got.Body)
++ t.Errorf("%s: got unexpected request = %#v\n Body = %q, %v", tt.name, got, all, err)
++ }
++ }
++}
+Index: golang/src/net/http/transfer.go
+===================================================================
+--- golang.orig/src/net/http/transfer.go
++++ golang/src/net/http/transfer.go
+@@ -143,6 +143,9 @@ func (t *transferWriter) shouldSendConte
+ return true
+ }
+ if t.ContentLength == 0 && isIdentity(t.TransferEncoding) {
++ if t.Method == "GET" || t.Method == "HEAD" {
++ return false
++ }
+ return true
+ }
+
+@@ -310,6 +313,7 @@ func readTransfer(msg interface{}, r *bu
+ }
+ case *Request:
+ t.Header = rr.Header
++ t.RequestMethod = rr.Method
+ t.ProtoMajor = rr.ProtoMajor
+ t.ProtoMinor = rr.ProtoMinor
+ // Transfer semantics for Requests are exactly like those for
+@@ -325,7 +329,7 @@ func readTransfer(msg interface{}, r *bu
+ }
+
+ // Transfer encoding, content length
+- t.TransferEncoding, err = fixTransferEncoding(t.RequestMethod, t.Header)
++ t.TransferEncoding, err = fixTransferEncoding(isResponse, t.RequestMethod, t.Header)
+ if err != nil {
+ return err
+ }
+@@ -413,12 +417,11 @@ func chunked(te []string) bool { return
+ func isIdentity(te []string) bool { return len(te) == 1 && te[0] == "identity" }
+
+ // Sanitize transfer encoding
+-func fixTransferEncoding(requestMethod string, header Header) ([]string, error) {
++func fixTransferEncoding(isResponse bool, requestMethod string, header Header) ([]string, error) {
+ raw, present := header["Transfer-Encoding"]
+ if !present {
+ return nil, nil
+ }
+-
+ delete(header, "Transfer-Encoding")
+
+ encodings := strings.Split(raw[0], ",")
+@@ -443,9 +446,22 @@ func fixTransferEncoding(requestMethod s
+ return nil, &badStringError{"too many transfer encodings", strings.Join(te, ",")}
+ }
+ if len(te) > 0 {
+- // Chunked encoding trumps Content-Length. See RFC 2616
+- // Section 4.4. Currently len(te) > 0 implies chunked
+- // encoding.
++ // RFC 7230 3.3.2 says "A sender MUST NOT send a
++ // Content-Length header field in any message that
++ // contains a Transfer-Encoding header field."
++ //
++ // but also:
++ // "If a message is received with both a
++ // Transfer-Encoding and a Content-Length header
++ // field, the Transfer-Encoding overrides the
++ // Content-Length. Such a message might indicate an
++ // attempt to perform request smuggling (Section 9.5)
++ // or response splitting (Section 9.4) and ought to be
++ // handled as an error. A sender MUST remove the
++ // received Content-Length field prior to forwarding
++ // such a message downstream."
++ //
++ // Reportedly, these appear in the wild.
+ delete(header, "Content-Length")
+ return te, nil
+ }
+@@ -457,9 +473,17 @@ func fixTransferEncoding(requestMethod s
+ // function is not a method, because ultimately it should be shared by
+ // ReadResponse and ReadRequest.
+ func fixLength(isResponse bool, status int, requestMethod string, header Header, te []string) (int64, error) {
+-
++ contentLens := header["Content-Length"]
++ isRequest := !isResponse
+ // Logic based on response type or status
+ if noBodyExpected(requestMethod) {
++ // For HTTP requests, as part of hardening against request
++ // smuggling (RFC 7230), don't allow a Content-Length header for
++ // methods which don't permit bodies. As an exception, allow
++ // exactly one Content-Length header if its value is "0".
++ if isRequest && len(contentLens) > 0 && !(len(contentLens) == 1 && contentLens[0] == "0") {
++ return 0, fmt.Errorf("http: method cannot contain a Content-Length; got %q", contentLens)
++ }
+ return 0, nil
+ }
+ if status/100 == 1 {
+@@ -470,13 +494,21 @@ func fixLength(isResponse bool, status i
+ return 0, nil
+ }
+
++ if len(contentLens) > 1 {
++ // harden against HTTP request smuggling. See RFC 7230.
++ return 0, errors.New("http: message cannot contain multiple Content-Length headers")
++ }
++
+ // Logic based on Transfer-Encoding
+ if chunked(te) {
+ return -1, nil
+ }
+
+ // Logic based on Content-Length
+- cl := strings.TrimSpace(header.get("Content-Length"))
++ var cl string
++ if len(contentLens) == 1 {
++ cl = strings.TrimSpace(contentLens[0])
++ }
+ if cl != "" {
+ n, err := parseContentLength(cl)
+ if err != nil {
+@@ -487,11 +519,14 @@ func fixLength(isResponse bool, status i
+ header.Del("Content-Length")
+ }
+
+- if !isResponse && requestMethod == "GET" {
+- // RFC 2616 doesn't explicitly permit nor forbid an
++ if !isResponse {
++ // RFC 2616 neither explicitly permits nor forbids an
+ // entity-body on a GET request so we permit one if
+ // declared, but we default to 0 here (not -1 below)
+ // if there's no mention of a body.
++ // Likewise, all other request methods are assumed to have
++ // no body if neither Transfer-Encoding chunked nor a
++ // Content-Length are set.
+ return 0, nil
+ }
+
diff --git a/debian/patches/series b/debian/patches/series
index e69de29..4e3b467 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -0,0 +1 @@
+cve-2015-5739-5740-5741.patch
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-golang/golang.git
More information about the pkg-golang-commits
mailing list