diff --git a/117ddcb83d7f42d6aa72241240af99ded81118e9.patch b/117ddcb83d7f42d6aa72241240af99ded81118e9.patch new file mode 100644 index 0000000..bc8eb0b --- /dev/null +++ b/117ddcb83d7f42d6aa72241240af99ded81118e9.patch @@ -0,0 +1,135 @@ +commit 117ddcb83d7f42d6aa72241240af99ded81118e9 +Author: Brad Fitzpatrick +Date: Tue Jun 30 09:22:41 2015 -0700 + + net/textproto: don't treat spaces as hyphens in header keys + + This was originally done in https://codereview.appspot.com/5690059 + (Feb 2012) to deal with bad response headers coming back from webcams, + but it presents a potential security problem with HTTP request + smuggling for request headers containing "Content Length" instead of + "Content-Length". + + Part of overall HTTP hardening for request smuggling. See RFC 7230. + + Thanks to Régis Leroy for the report. + + Change-Id: I92b17fb637c9171c5774ea1437979ae2c17ca88a + Reviewed-on: https://go-review.googlesource.com/11772 + Reviewed-by: Russ Cox + Run-TryBot: Brad Fitzpatrick + TryBot-Result: Gobot Gobot + +diff --git a/src/net/http/header.go b/src/net/http/header.go +index 153b943..d847b13 100644 +--- a/src/net/http/header.go ++++ b/src/net/http/header.go +@@ -168,6 +168,8 @@ func (h Header) WriteSubset(w io.Writer, exclude map[string]bool) error { + // 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 +diff --git a/src/net/textproto/reader.go b/src/net/textproto/reader.go +index e4b8f6b..91303fe 100644 +--- a/src/net/textproto/reader.go ++++ b/src/net/textproto/reader.go +@@ -547,11 +547,16 @@ func (r *Reader) upcomingHeaderNewlines() (n int) { + // 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)) + } +@@ -565,19 +570,44 @@ func CanonicalMIMEHeaderKey(s string) string { + + 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 +diff --git a/src/net/textproto/reader_test.go b/src/net/textproto/reader_test.go +index 6bbd993..8fce7dd 100644 +--- a/src/net/textproto/reader_test.go ++++ b/src/net/textproto/reader_test.go +@@ -24,11 +24,14 @@ var canonicalHeaderKeyTests = []canonicalHeaderKeyTest{ + {"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) { +@@ -194,7 +197,7 @@ func TestReadMIMEHeaderNonCompliant(t *testing.T) { + "Foo": {"bar"}, + "Content-Language": {"en"}, + "Sid": {"0"}, +- "Audio-Mode": {"None"}, ++ "Audio Mode": {"None"}, + "Privilege": {"127"}, + } + if !reflect.DeepEqual(m, want) || err != nil { diff --git a/143822585e32449860e624cace9d2e521deee62e.patch b/143822585e32449860e624cace9d2e521deee62e.patch new file mode 100644 index 0000000..b11edc6 --- /dev/null +++ b/143822585e32449860e624cace9d2e521deee62e.patch @@ -0,0 +1,112 @@ +commit 143822585e32449860e624cace9d2e521deee62e +Author: Brad Fitzpatrick +Date: Tue Jul 7 13:19:44 2015 -0600 + + net/http: revert overly-strict part of earlier smuggling defense + + The recent https://golang.org/cl/11810 is reportedly a bit too + aggressive. + + Apparently some HTTP requests in the wild do contain both a + Transfer-Encoding along with a bogus Content-Length. Instead of + returning a 400 Bad Request error, we should just ignore the + Content-Length like we did before. + + Change-Id: I0001be90d09f8293a34f04691f608342875ff5c4 + Reviewed-on: https://go-review.googlesource.com/11962 + Reviewed-by: Andrew Gerrand + Run-TryBot: Brad Fitzpatrick + TryBot-Result: Gobot Gobot + +diff --git a/src/net/http/readrequest_test.go b/src/net/http/readrequest_test.go +index 1a3cf91..60e2be4 100644 +--- a/src/net/http/readrequest_test.go ++++ b/src/net/http/readrequest_test.go +@@ -178,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", +@@ -400,11 +430,6 @@ Content-Length: 3 + Content-Length: 4 + + abc`)}, +- {"smuggle_chunked_and_len", reqBytes(`POST / HTTP/1.1 +-Transfer-Encoding: chunked +-Content-Length: 3 +- +-abc`)}, + {"smuggle_content_len_head", reqBytes(`HEAD / HTTP/1.1 + Host: foo + Content-Length: 5`)}, +diff --git a/src/net/http/transfer.go b/src/net/http/transfer.go +index 3c868bd..fbbbf24 100644 +--- a/src/net/http/transfer.go ++++ b/src/net/http/transfer.go +@@ -430,7 +430,6 @@ func fixTransferEncoding(isResponse bool, requestMethod string, header Header) ( + if !present { + return nil, nil + } +- isRequest := !isResponse + delete(header, "Transfer-Encoding") + + encodings := strings.Split(raw[0], ",") +@@ -458,12 +457,20 @@ func fixTransferEncoding(isResponse bool, requestMethod string, header Header) ( + // 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." +- if len(header["Content-Length"]) > 0 { +- if isRequest { +- return nil, errors.New("http: invalid Content-Length with Transfer-Encoding") +- } +- delete(header, "Content-Length") +- } ++ // ++ // 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 + } + diff --git a/300d9a21583e7cf0149a778a0611e76ff7c6680f.patch b/300d9a21583e7cf0149a778a0611e76ff7c6680f.patch new file mode 100644 index 0000000..1027032 --- /dev/null +++ b/300d9a21583e7cf0149a778a0611e76ff7c6680f.patch @@ -0,0 +1,225 @@ +commit 300d9a21583e7cf0149a778a0611e76ff7c6680f +Author: Brad Fitzpatrick +Date: Tue Jun 30 14:21:15 2015 -0700 + + net/http: harden Server against request smuggling + + See RFC 7230. + + Thanks to Régis Leroy for the report. + + Change-Id: Ic1779bc2180900430d4d7a4938cac04ed73c304c + Reviewed-on: https://go-review.googlesource.com/11810 + Reviewed-by: Russ Cox + Run-TryBot: Brad Fitzpatrick + +diff --git a/src/net/http/readrequest_test.go b/src/net/http/readrequest_test.go +index e930d99..1a3cf91 100644 +--- a/src/net/http/readrequest_test.go ++++ b/src/net/http/readrequest_test.go +@@ -9,6 +9,7 @@ import ( + "bytes" + "fmt" + "io" ++ "io/ioutil" + "net/url" + "reflect" + "strings" +@@ -323,6 +324,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 +383,39 @@ 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_chunked_and_len", reqBytes(`POST / HTTP/1.1 ++Transfer-Encoding: chunked ++Content-Length: 3 ++ ++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) ++ } ++ } ++} +diff --git a/src/net/http/transfer.go b/src/net/http/transfer.go +index 5205003..3887604 100644 +--- a/src/net/http/transfer.go ++++ b/src/net/http/transfer.go +@@ -143,6 +143,9 @@ func (t *transferWriter) shouldSendContentLength() bool { + 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 *bufio.Reader) (err error) { + } + 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 *bufio.Reader) (err error) { + } + + // 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,12 @@ func chunked(te []string) bool { return len(te) > 0 && te[0] == "chunked" } + 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 + } +- ++ isRequest := !isResponse + delete(header, "Transfer-Encoding") + + encodings := strings.Split(raw[0], ",") +@@ -443,10 +447,15 @@ func fixTransferEncoding(requestMethod string, header Header) ([]string, error) + 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. +- delete(header, "Content-Length") ++ // 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." ++ if len(header["Content-Length"]) > 0 { ++ if isRequest { ++ return nil, errors.New("http: invalid Content-Length with Transfer-Encoding") ++ } ++ delete(header, "Content-Length") ++ } + return te, nil + } + +@@ -457,9 +466,17 @@ func fixTransferEncoding(requestMethod string, header Header) ([]string, error) + // 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 +487,21 @@ func fixLength(isResponse bool, status int, requestMethod string, header Header, + 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 +512,14 @@ func fixLength(isResponse bool, status int, requestMethod string, header Header, + 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/golang.spec b/golang.spec index 33cff31..dd8281d 100644 --- a/golang.spec +++ b/golang.spec @@ -40,7 +40,7 @@ Name: golang Version: 1.4.2 -Release: 2%{?dist} +Release: 3%{?dist} Summary: The Go Programming Language License: BSD @@ -68,6 +68,12 @@ Patch0: golang-1.2-verbose-build.patch # https://bugzilla.redhat.com/show_bug.cgi?id=1038683 Patch1: golang-1.2-remove-ECC-p224.patch +# TODO this should be removed with go1.4.3 +# https://bugzilla.redhat.com/show_bug.cgi?id=1250352 +Patch100: 300d9a21583e7cf0149a778a0611e76ff7c6680f.patch +Patch101: 117ddcb83d7f42d6aa72241240af99ded81118e9.patch +Patch102: 143822585e32449860e624cace9d2e521deee62e.patch + # Having documentation separate was broken Obsoletes: %{name}-docs < 1.1-4 @@ -324,6 +330,11 @@ end # remove the P224 curve %patch1 -p1 +# bz1250352 +%patch100 -p1 +%patch101 -p1 +%patch102 -p1 + %build # set up final install location export GOROOT_FINAL=%{goroot} @@ -739,6 +750,9 @@ fi %changelog +* Wed Aug 05 2015 Vincent Batts - 1.4.2-3 +- bz1250352 + * Wed Mar 18 2015 Vincent Batts - 1.4.2-2 - obsoleting deprecated packages