[master] 053c93c Enforce sequence requirement of header block frames

Dag Haavi Finstad daghf at varnish-software.com
Tue Oct 3 13:43:05 UTC 2017


commit 053c93cd12f79a00d9801a5d0f9e67ea3aa2fcbd
Author: Dag Haavi Finstad <daghf at varnish-software.com>
Date:   Fri Sep 29 16:05:31 2017 +0200

    Enforce sequence requirement of header block frames
    
    This commit enforces that a HEADERS frame without an END_HEADERS flag
    must be followed by a CONTINUATION frame on the same stream.
    
    Sending interleaved/out of sequence header or continuation frames will
    now be treated as a PROTOCOL_ERROR, as per rfc7540.
    
    This also fixes the panics seen in #2387.
    
    Fixes: #2387

diff --git a/bin/varnishd/http2/cache_http2_proto.c b/bin/varnishd/http2/cache_http2_proto.c
index 3c50d20..06353d8 100644
--- a/bin/varnishd/http2/cache_http2_proto.c
+++ b/bin/varnishd/http2/cache_http2_proto.c
@@ -508,7 +508,7 @@ h2_do_req(struct worker *wrk, void *priv)
 }
 
 static h2_error
-h2_end_headers(struct worker *wrk, const struct h2_sess *h2,
+h2_end_headers(struct worker *wrk, struct h2_sess *h2,
     struct req *req, struct h2_req *r2)
 {
 	h2_error h2e;
@@ -518,6 +518,7 @@ h2_end_headers(struct worker *wrk, const struct h2_sess *h2,
 	h2e = h2h_decode_fini(h2, r2->decode);
 	FREE_OBJ(r2->decode);
 	r2->state = H2_S_CLOS_REM;
+	h2->new_req = NULL;
 	if (h2e != NULL) {
 		Lck_Lock(&h2->sess->mtx);
 		VSLb(h2->vsl, SLT_Debug, "HPACK/FINI %s", h2e->name);
@@ -614,9 +615,7 @@ h2_rx_headers(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2)
 	return (0);
 }
 
-/**********************************************************************
- * XXX: Check hard sequence req. for Cont.
- */
+/**********************************************************************/
 
 static h2_error __match_proto__(h2_frame_f)
 h2_rx_continuation(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2)
@@ -831,6 +830,11 @@ h2_procframe(struct worker *wrk, struct h2_sess *h2,
 		AN(r2);
 	}
 
+	if (h2->new_req != NULL &&
+	    !(h2->new_req == r2->req && h2f == H2_F_CONTINUATION))
+		return (H2CE_PROTOCOL_ERROR);	// rfc7540,l,1859,1863
+
+
 	h2e = h2f->rxfunc(wrk, h2, r2);
 	if (h2e == 0)
 		return (0);
diff --git a/bin/varnishtest/tests/r02387.vtc b/bin/varnishtest/tests/r02387.vtc
new file mode 100644
index 0000000..34863a0
--- /dev/null
+++ b/bin/varnishtest/tests/r02387.vtc
@@ -0,0 +1,36 @@
+varnishtest "2387: Crash on out of order header blocks"
+
+server s1 {
+	rxreq
+	txresp
+} -start
+
+varnish v1 -vcl+backend {} -start
+
+varnish v1 -cliok "param.set feature +http2"
+varnish v1 -cliok "param.set debug +syncvsl"
+
+
+barrier b1 cond 2
+barrier b2 cond 2
+
+client c1 {
+	stream 1 {
+		txreq -nohdrend
+		barrier b2 sync
+		barrier b1 sync
+		txcont -hdr "bar" "foo"
+	} -start
+	stream 3 {
+		barrier b2 sync
+		txreq -nohdrend
+		barrier b1 sync
+		txcont -hdr "bar" "foo"
+
+	} -run
+	stream 0 {
+		rxgoaway
+		expect goaway.laststream == "3"
+		expect goaway.err == PROTOCOL_ERROR
+	} -run
+} -run


More information about the varnish-commit mailing list