http2: switch to ASCII equivalents of string functions
The current implementation uses UTF-aware functions
like strings.EqualFold and strings.ToLower.
This could, in some cases, cause http smuggling.
Change-Id: I7250b24bbefe2143b61cc8dbbe2853a66499d829
Reviewed-on: https://go-review.googlesource.com/c/net/+/313489
Trust: Roberto Clapis <roberto@golang.org>
Trust: Katie Hockman <katie@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
diff --git a/http2/ascii.go b/http2/ascii.go
new file mode 100644
index 0000000..0c58d72
--- /dev/null
+++ b/http2/ascii.go
@@ -0,0 +1,49 @@
+// Copyright 2021 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package http2
+
+import "strings"
+
+// asciiEqualFold is strings.EqualFold, ASCII only. It reports whether s and t
+// are equal, ASCII-case-insensitively.
+func asciiEqualFold(s, t string) bool {
+ if len(s) != len(t) {
+ return false
+ }
+ for i := 0; i < len(s); i++ {
+ if lower(s[i]) != lower(t[i]) {
+ return false
+ }
+ }
+ return true
+}
+
+// lower returns the ASCII lowercase version of b.
+func lower(b byte) byte {
+ if 'A' <= b && b <= 'Z' {
+ return b + ('a' - 'A')
+ }
+ return b
+}
+
+// isASCIIPrint returns whether s is ASCII and printable according to
+// https://tools.ietf.org/html/rfc20#section-4.2.
+func isASCIIPrint(s string) bool {
+ for i := 0; i < len(s); i++ {
+ if s[i] < ' ' || s[i] > '~' {
+ return false
+ }
+ }
+ return true
+}
+
+// asciiToLower returns the lowercase version of s if s is ASCII and printable,
+// and whether or not it was.
+func asciiToLower(s string) (lower string, ok bool) {
+ if !isASCIIPrint(s) {
+ return "", false
+ }
+ return strings.ToLower(s), true
+}
diff --git a/http2/ascii_test.go b/http2/ascii_test.go
new file mode 100644
index 0000000..0c4e4ab
--- /dev/null
+++ b/http2/ascii_test.go
@@ -0,0 +1,95 @@
+// Copyright 2021 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package http2
+
+import "testing"
+
+func TestASCIIEqualFold(t *testing.T) {
+ var tests = []struct {
+ name string
+ a, b string
+ want bool
+ }{
+ {
+ name: "empty",
+ want: true,
+ },
+ {
+ name: "simple match",
+ a: "CHUNKED",
+ b: "chunked",
+ want: true,
+ },
+ {
+ name: "same string",
+ a: "chunked",
+ b: "chunked",
+ want: true,
+ },
+ {
+ name: "Unicode Kelvin symbol",
+ a: "chunKed", // This "K" is 'KELVIN SIGN' (\u212A)
+ b: "chunked",
+ want: false,
+ },
+ }
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ if got := asciiEqualFold(tt.a, tt.b); got != tt.want {
+ t.Errorf("AsciiEqualFold(%q,%q): got %v want %v", tt.a, tt.b, got, tt.want)
+ }
+ })
+ }
+}
+
+func TestIsASCIIPrint(t *testing.T) {
+ var tests = []struct {
+ name string
+ in string
+ want bool
+ }{
+ {
+ name: "empty",
+ want: true,
+ },
+ {
+ name: "ASCII low",
+ in: "This is a space: ' '",
+ want: true,
+ },
+ {
+ name: "ASCII high",
+ in: "This is a tilde: '~'",
+ want: true,
+ },
+ {
+ name: "ASCII low non-print",
+ in: "This is a unit separator: \x1F",
+ want: false,
+ },
+ {
+ name: "Ascii high non-print",
+ in: "This is a Delete: \x7F",
+ want: false,
+ },
+ {
+ name: "Unicode letter",
+ in: "Today it's 280K outside: it's freezing!", // This "K" is 'KELVIN SIGN' (\u212A)
+ want: false,
+ },
+ {
+ name: "Unicode emoji",
+ in: "Gophers like 🧀",
+ want: false,
+ },
+ }
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ if got := isASCIIPrint(tt.in); got != tt.want {
+ t.Errorf("IsASCIIPrint(%q): got %v want %v", tt.in, got, tt.want)
+ }
+ })
+ }
+}
diff --git a/http2/headermap.go b/http2/headermap.go
index c3ff3fa..9e12941 100644
--- a/http2/headermap.go
+++ b/http2/headermap.go
@@ -6,7 +6,6 @@
import (
"net/http"
- "strings"
"sync"
)
@@ -79,10 +78,10 @@
}
}
-func lowerHeader(v string) string {
+func lowerHeader(v string) (lower string, ascii bool) {
buildCommonHeaderMapsOnce()
if s, ok := commonLowerHeader[v]; ok {
- return s
+ return s, true
}
- return strings.ToLower(v)
+ return asciiToLower(v)
}
diff --git a/http2/server.go b/http2/server.go
index 5ad3a2d..09bc705 100644
--- a/http2/server.go
+++ b/http2/server.go
@@ -2783,8 +2783,12 @@
// but PUSH_PROMISE requests cannot have a body.
// http://tools.ietf.org/html/rfc7540#section-8.2
// Also disallow Host, since the promised URL must be absolute.
- switch strings.ToLower(k) {
- case "content-length", "content-encoding", "trailer", "te", "expect", "host":
+ if asciiEqualFold(k, "content-length") ||
+ asciiEqualFold(k, "content-encoding") ||
+ asciiEqualFold(k, "trailer") ||
+ asciiEqualFold(k, "te") ||
+ asciiEqualFold(k, "expect") ||
+ asciiEqualFold(k, "host") {
return fmt.Errorf("promised request headers cannot include %q", k)
}
}
diff --git a/http2/transport.go b/http2/transport.go
index f89369e..7bd4b9c 100644
--- a/http2/transport.go
+++ b/http2/transport.go
@@ -995,7 +995,7 @@
if vv := req.Header["Transfer-Encoding"]; len(vv) > 0 && (len(vv) > 1 || vv[0] != "" && vv[0] != "chunked") {
return fmt.Errorf("http2: invalid Transfer-Encoding request header: %q", vv)
}
- if vv := req.Header["Connection"]; len(vv) > 0 && (len(vv) > 1 || vv[0] != "" && !strings.EqualFold(vv[0], "close") && !strings.EqualFold(vv[0], "keep-alive")) {
+ if vv := req.Header["Connection"]; len(vv) > 0 && (len(vv) > 1 || vv[0] != "" && !asciiEqualFold(vv[0], "close") && !asciiEqualFold(vv[0], "keep-alive")) {
return fmt.Errorf("http2: invalid Connection request header: %q", vv)
}
return nil
@@ -1521,19 +1521,21 @@
var didUA bool
for k, vv := range req.Header {
- if strings.EqualFold(k, "host") || strings.EqualFold(k, "content-length") {
+ if asciiEqualFold(k, "host") || asciiEqualFold(k, "content-length") {
// Host is :authority, already sent.
// Content-Length is automatic, set below.
continue
- } else if strings.EqualFold(k, "connection") || strings.EqualFold(k, "proxy-connection") ||
- strings.EqualFold(k, "transfer-encoding") || strings.EqualFold(k, "upgrade") ||
- strings.EqualFold(k, "keep-alive") {
+ } else if asciiEqualFold(k, "connection") ||
+ asciiEqualFold(k, "proxy-connection") ||
+ asciiEqualFold(k, "transfer-encoding") ||
+ asciiEqualFold(k, "upgrade") ||
+ asciiEqualFold(k, "keep-alive") {
// Per 8.1.2.2 Connection-Specific Header
// Fields, don't send connection-specific
// fields. We have already checked if any
// are error-worthy so just ignore the rest.
continue
- } else if strings.EqualFold(k, "user-agent") {
+ } else if asciiEqualFold(k, "user-agent") {
// Match Go's http1 behavior: at most one
// User-Agent. If set to nil or empty string,
// then omit it. Otherwise if not mentioned,
@@ -1546,7 +1548,7 @@
if vv[0] == "" {
continue
}
- } else if strings.EqualFold(k, "cookie") {
+ } else if asciiEqualFold(k, "cookie") {
// Per 8.1.2.5 To allow for better compression efficiency, the
// Cookie header field MAY be split into separate header fields,
// each with one or more cookie-pairs.
@@ -1605,7 +1607,12 @@
// Header list size is ok. Write the headers.
enumerateHeaders(func(name, value string) {
- name = strings.ToLower(name)
+ name, ascii := asciiToLower(name)
+ if !ascii {
+ // Skip writing invalid headers. Per RFC 7540, Section 8.1.2, header
+ // field names have to be ASCII characters (just as in HTTP/1.x).
+ return
+ }
cc.writeHeader(name, value)
if traceHeaders {
traceWroteHeaderField(trace, name, value)
@@ -1653,9 +1660,14 @@
}
for k, vv := range req.Trailer {
+ lowKey, ascii := asciiToLower(k)
+ if !ascii {
+ // Skip writing invalid headers. Per RFC 7540, Section 8.1.2, header
+ // field names have to be ASCII characters (just as in HTTP/1.x).
+ continue
+ }
// Transfer-Encoding, etc.. have already been filtered at the
// start of RoundTrip
- lowKey := strings.ToLower(k)
for _, v := range vv {
cc.writeHeader(lowKey, v)
}
diff --git a/http2/transport_test.go b/http2/transport_test.go
index 7b13928..750813b 100644
--- a/http2/transport_test.go
+++ b/http2/transport_test.go
@@ -2219,6 +2219,11 @@
},
{
key: "Transfer-Encoding",
+ value: []string{"chunKed"}, // Kelvin sign
+ want: "ERROR: http2: invalid Transfer-Encoding request header: [\"chunKed\"]",
+ },
+ {
+ key: "Transfer-Encoding",
value: []string{"chunked", "other"},
want: "ERROR: http2: invalid Transfer-Encoding request header: [\"chunked\" \"other\"]",
},
diff --git a/http2/write.go b/http2/write.go
index 3849bc2..33f6139 100644
--- a/http2/write.go
+++ b/http2/write.go
@@ -341,7 +341,12 @@
}
for _, k := range keys {
vv := h[k]
- k = lowerHeader(k)
+ k, ascii := lowerHeader(k)
+ if !ascii {
+ // Skip writing invalid headers. Per RFC 7540, Section 8.1.2, header
+ // field names have to be ASCII characters (just as in HTTP/1.x).
+ continue
+ }
if !validWireHeaderFieldName(k) {
// Skip it as backup paranoia. Per
// golang.org/issue/14048, these should
