http2: don't allow writing non-zero padding unless Framer.AllowIllegalWrites

Fixes golang/go#18809

Change-Id: Ib1014f3ebe5a57dde30b4eaf287a2cbff3c1179c
Reviewed-on: https://go-review.googlesource.com/36118
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Tom Bergan <tombergan@google.com>

http2/frame.go

@@ -600,6 +600,7 @@ var (
 	errStreamID    = errors.New("invalid stream ID")
 	errDepStreamID = errors.New("invalid dependent stream ID")
 	errPadLength   = errors.New("pad length too large")
+	errPadBytes    = errors.New("padding bytes must all be zeros unless AllowIllegalWrites is enabled")
 )
 
 func validStreamIDOrZero(streamID uint32) bool {
@@ -623,6 +624,7 @@ func (f *Framer) WriteData(streamID uint32, endStream bool, data []byte) error {
 //
 // If pad is nil, the padding bit is not sent.
 // The length of pad must not exceed 255 bytes.
+// The bytes of pad must all be zero, unless f.AllowIllegalWrites is set.
 //
 // It will perform exactly one Write to the underlying Writer.
 // It is the caller's responsibility not to violate the maximum frame size
@@ -631,8 +633,18 @@ func (f *Framer) WriteDataPadded(streamID uint32, endStream bool, data, pad []by
 	if !validStreamID(streamID) && !f.AllowIllegalWrites {
 		return errStreamID
 	}
-	if len(pad) > 255 {
-		return errPadLength
+	if len(pad) > 0 {
+		if len(pad) > 255 {
+			return errPadLength
+		}
+		if !f.AllowIllegalWrites {
+			for _, b := range pad {
+				if b != 0 {
+					// "Padding octets MUST be set to zero when sending."
+					return errPadBytes
+				}
+			}
+		}
 	}
 	var flags Flags
 	if endStream {

http2/frame_test.go

@@ -141,7 +141,7 @@ func TestWriteDataPadded(t *testing.T) {
 			streamID:  1,
 			endStream: false,
 			data:      []byte("foo"),
-			pad:       []byte("bar"),
+			pad:       []byte{0, 0, 0},
 			wantHeader: FrameHeader{
 				Type:     FrameData,
 				Flags:    FlagDataPadded,

http2/server_test.go

@@ -1192,7 +1192,7 @@ func TestServer_Handler_Sends_WindowUpdate_Padding(t *testing.T) {
 		EndStream:     false,
 		EndHeaders:    true,
 	})
-	st.writeDataPadded(1, false, []byte("abcdef"), []byte("1234"))
+	st.writeDataPadded(1, false, []byte("abcdef"), []byte{0, 0, 0, 0})
 
 	// Expect to immediately get our 5 bytes of padding back for
 	// both the connection and stream (4 bytes of padding + 1 byte of length)

http2/transport_test.go

@@ -2406,7 +2406,7 @@ func TestTransportReturnsDataPaddingFlowControl(t *testing.T) {
 			EndStream:     false,
 			BlockFragment: buf.Bytes(),
 		})
-		pad := []byte("12345")
+		pad := make([]byte, 5)
 		ct.fr.WriteDataPadded(hf.StreamID, false, make([]byte, 5000), pad) // without ending stream
 
 		f, err := ct.readNonSettingsFrame()