From a1bb48770c939dcd21951545f674af098ae60c69 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 6 Sep 2015 23:11:07 +0900 Subject: [PATCH] nghttpx: Add tests for PROXY protocol handling --- integration-tests/nghttpx_http2_test.go | 321 ++++++++++++++++++++++++ src/shrpx_client_handler.cc | 32 ++- src/shrpx_client_handler.h | 1 + 3 files changed, 341 insertions(+), 13 deletions(-) diff --git a/integration-tests/nghttpx_http2_test.go b/integration-tests/nghttpx_http2_test.go index 41c527f7..fe1fd37a 100644 --- a/integration-tests/nghttpx_http2_test.go +++ b/integration-tests/nghttpx_http2_test.go @@ -787,6 +787,327 @@ func TestH2H1Upgrade(t *testing.T) { } } +// TestH2H1ProxyProtocolV1TCP4 tests PROXY protocol version 1 +// containing TCP4 entry is accepted and X-Forwarded-For contains +// advertised src address. +func TestH2H1ProxyProtocolV1TCP4(t *testing.T) { + st := newServerTester([]string{"--accept-proxy-protocol", "--add-x-forwarded-for"}, t, func(w http.ResponseWriter, r *http.Request) { + if got, want := r.Header.Get("X-Forwarded-For"), "192.168.0.2"; got != want { + t.Errorf("X-Forwarded-For: %v; want %v", got, want) + } + }) + defer st.Close() + + st.conn.Write([]byte("PROXY TCP4 192.168.0.2 192.168.0.100 12345 8080\r\n")) + + res, err := st.http2(requestParam{ + name: "TestH2H1ProxyProtocolV1TCP4", + }) + + if err != nil { + t.Fatalf("Error st.http2() = %v", err) + } + + if got, want := res.status, 200; got != want { + t.Errorf("res.status: %v; want %v", got, want) + } +} + +// TestH2H1ProxyProtocolV1TCP6 tests PROXY protocol version 1 +// containing TCP6 entry is accepted and X-Forwarded-For contains +// advertised src address. +func TestH2H1ProxyProtocolV1TCP6(t *testing.T) { + st := newServerTester([]string{"--accept-proxy-protocol", "--add-x-forwarded-for"}, t, func(w http.ResponseWriter, r *http.Request) { + if got, want := r.Header.Get("X-Forwarded-For"), "2001:0db8:85a3:0000:0000:8a2e:0370:7334"; got != want { + t.Errorf("X-Forwarded-For: %v; want %v", got, want) + } + }) + defer st.Close() + + st.conn.Write([]byte("PROXY TCP6 2001:0db8:85a3:0000:0000:8a2e:0370:7334 ::1 12345 8080\r\n")) + + res, err := st.http2(requestParam{ + name: "TestH2H1ProxyProtocolV1TCP6", + }) + + if err != nil { + t.Fatalf("Error st.http2() = %v", err) + } + + if got, want := res.status, 200; got != want { + t.Errorf("res.status: %v; want %v", got, want) + } +} + +// TestH2H1ProxyProtocolV1Unknown tests PROXY protocol version 1 +// containing UNKNOWN entry is accepted. +func TestH2H1ProxyProtocolV1Unknown(t *testing.T) { + st := newServerTester([]string{"--accept-proxy-protocol", "--add-x-forwarded-for"}, t, func(w http.ResponseWriter, r *http.Request) { + if got, notWant := r.Header.Get("X-Forwarded-For"), "192.168.0.2"; got == notWant { + t.Errorf("X-Forwarded-For: %v") + } + }) + defer st.Close() + + st.conn.Write([]byte("PROXY UNKNOWN 192.168.0.2 192.168.0.100 12345 8080\r\n")) + + res, err := st.http2(requestParam{ + name: "TestH2H1ProxyProtocolV1Unknown", + }) + + if err != nil { + t.Fatalf("Error st.http2() = %v", err) + } + + if got, want := res.status, 200; got != want { + t.Errorf("res.status: %v; want %v", got, want) + } +} + +// TestH2H1ProxyProtocolV1JustUnknown tests PROXY protocol version 1 +// containing only "PROXY UNKNOWN" is accepted. +func TestH2H1ProxyProtocolV1JustUnknown(t *testing.T) { + st := newServerTester([]string{"--accept-proxy-protocol", "--add-x-forwarded-for"}, t, noopHandler) + defer st.Close() + + st.conn.Write([]byte("PROXY UNKNOWN\r\n")) + + res, err := st.http2(requestParam{ + name: "TestH2H1ProxyProtocolV1JustUnknown", + }) + + if err != nil { + t.Fatalf("Error st.http2() = %v", err) + } + + if got, want := res.status, 200; got != want { + t.Errorf("res.status: %v; want %v", got, want) + } +} + +// TestH2H1ProxyProtocolV1BadLineEnd tests that PROXY protocol version +// 1 line ending without \r\n should be rejected. +func TestH2H1ProxyProtocolV1BadLineEnd(t *testing.T) { + st := newServerTester([]string{"--accept-proxy-protocol"}, t, noopHandler) + defer st.Close() + + st.conn.Write([]byte("PROXY TCP6 ::1 ::1 12345 8080\r \n")) + + _, err := st.http2(requestParam{ + name: "TestH2H1ProxyProtocolV1BadLineEnd", + }) + + if err == nil { + t.Fatalf("connection was not terminated") + } +} + +// TestH2H1ProxyProtocolV1NoEnd tests that PROXY protocol version 1 +// line containing no \r\n should be rejected. +func TestH2H1ProxyProtocolV1NoEnd(t *testing.T) { + st := newServerTester([]string{"--accept-proxy-protocol"}, t, noopHandler) + defer st.Close() + + st.conn.Write([]byte("PROXY TCP6 ::1 ::1 12345 8080")) + + _, err := st.http2(requestParam{ + name: "TestH2H1ProxyProtocolV1NoEnd", + }) + + if err == nil { + t.Fatalf("connection was not terminated") + } +} + +// TestH2H1ProxyProtocolV1EmbeddedNULL tests that PROXY protocol +// version 1 line containing NULL character should be rejected. +func TestH2H1ProxyProtocolV1EmbeddedNULL(t *testing.T) { + st := newServerTester([]string{"--accept-proxy-protocol"}, t, noopHandler) + defer st.Close() + + b := []byte("PROXY TCP6 ::1*foo ::1 12345 8080\r\n") + b[14] = 0 + st.conn.Write(b) + + _, err := st.http2(requestParam{ + name: "TestH2H1ProxyProtocolV1EmbeddedNULL", + }) + + if err == nil { + t.Fatalf("connection was not terminated") + } +} + +// TestH2H1ProxyProtocolV1MissingSrcPort tests that PROXY protocol +// version 1 line without src port should be rejected. +func TestH2H1ProxyProtocolV1MissingSrcPort(t *testing.T) { + st := newServerTester([]string{"--accept-proxy-protocol"}, t, noopHandler) + defer st.Close() + + st.conn.Write([]byte("PROXY TCP6 ::1 ::1 8080\r\n")) + + _, err := st.http2(requestParam{ + name: "TestH2H1ProxyProtocolV1MissingSrcPort", + }) + + if err == nil { + t.Fatalf("connection was not terminated") + } +} + +// TestH2H1ProxyProtocolV1MissingDstPort tests that PROXY protocol +// version 1 line without dst port should be rejected. +func TestH2H1ProxyProtocolV1MissingDstPort(t *testing.T) { + st := newServerTester([]string{"--accept-proxy-protocol"}, t, noopHandler) + defer st.Close() + + st.conn.Write([]byte("PROXY TCP6 ::1 ::1 12345 \r\n")) + + _, err := st.http2(requestParam{ + name: "TestH2H1ProxyProtocolV1MissingDstPort", + }) + + if err == nil { + t.Fatalf("connection was not terminated") + } +} + +// TestH2H1ProxyProtocolV1InvalidSrcPort tests that PROXY protocol +// containing invalid src port should be rejected. +func TestH2H1ProxyProtocolV1InvalidSrcPort(t *testing.T) { + st := newServerTester([]string{"--accept-proxy-protocol"}, t, noopHandler) + defer st.Close() + + st.conn.Write([]byte("PROXY TCP6 ::1 ::1 123x 8080\r\n")) + + _, err := st.http2(requestParam{ + name: "TestH2H1ProxyProtocolV1InvalidSrcPort", + }) + + if err == nil { + t.Fatalf("connection was not terminated") + } +} + +// TestH2H1ProxyProtocolV1InvalidDstPort tests that PROXY protocol +// containing invalid dst port should be rejected. +func TestH2H1ProxyProtocolV1InvalidDstPort(t *testing.T) { + st := newServerTester([]string{"--accept-proxy-protocol"}, t, noopHandler) + defer st.Close() + + st.conn.Write([]byte("PROXY TCP6 ::1 ::1 123456 80x\r\n")) + + _, err := st.http2(requestParam{ + name: "TestH2H1ProxyProtocolV1InvalidDstPort", + }) + + if err == nil { + t.Fatalf("connection was not terminated") + } +} + +// TestH2H1ProxyProtocolV1TooLargeSrcPort tests that PROXY protocol +// containing too large src port should be rejected. +func TestH2H1ProxyProtocolV1TooLargeSrcPort(t *testing.T) { + st := newServerTester([]string{"--accept-proxy-protocol"}, t, noopHandler) + defer st.Close() + + st.conn.Write([]byte("PROXY TCP6 ::1 ::1 65536 8080\r\n")) + + _, err := st.http2(requestParam{ + name: "TestH2H1ProxyProtocolV1TooLargeSrcPort", + }) + + if err == nil { + t.Fatalf("connection was not terminated") + } +} + +// TestH2H1ProxyProtocolV1TooLargeDstPort tests that PROXY protocol +// containing too large dst port should be rejected. +func TestH2H1ProxyProtocolV1TooLargeDstPort(t *testing.T) { + st := newServerTester([]string{"--accept-proxy-protocol"}, t, noopHandler) + defer st.Close() + + st.conn.Write([]byte("PROXY TCP6 ::1 ::1 12345 65536\r\n")) + + _, err := st.http2(requestParam{ + name: "TestH2H1ProxyProtocolV1TooLargeDstPort", + }) + + if err == nil { + t.Fatalf("connection was not terminated") + } +} + +// TestH2H1ProxyProtocolV1InvalidSrcAddr tests that PROXY protocol +// containing invalid src addr should be rejected. +func TestH2H1ProxyProtocolV1InvalidSrcAddr(t *testing.T) { + st := newServerTester([]string{"--accept-proxy-protocol"}, t, noopHandler) + defer st.Close() + + st.conn.Write([]byte("PROXY TCP6 192.168.0.1 ::1 12345 8080\r\n")) + + _, err := st.http2(requestParam{ + name: "TestH2H1ProxyProtocolV1InvalidSrcAddr", + }) + + if err == nil { + t.Fatalf("connection was not terminated") + } +} + +// TestH2H1ProxyProtocolV1InvalidDstAddr tests that PROXY protocol +// containing invalid dst addr should be rejected. +func TestH2H1ProxyProtocolV1InvalidDstAddr(t *testing.T) { + st := newServerTester([]string{"--accept-proxy-protocol"}, t, noopHandler) + defer st.Close() + + st.conn.Write([]byte("PROXY TCP6 ::1 192.168.0.1 12345 8080\r\n")) + + _, err := st.http2(requestParam{ + name: "TestH2H1ProxyProtocolV1InvalidDstAddr", + }) + + if err == nil { + t.Fatalf("connection was not terminated") + } +} + +// TestH2H1ProxyProtocolV1InvalidProtoFamily tests that PROXY protocol +// containing invalid protocol family should be rejected. +func TestH2H1ProxyProtocolV1InvalidProtoFamily(t *testing.T) { + st := newServerTester([]string{"--accept-proxy-protocol"}, t, noopHandler) + defer st.Close() + + st.conn.Write([]byte("PROXY UNIX ::1 ::1 12345 8080\r\n")) + + _, err := st.http2(requestParam{ + name: "TestH2H1ProxyProtocolV1InvalidProtoFamily", + }) + + if err == nil { + t.Fatalf("connection was not terminated") + } +} + +// TestH2H1ProxyProtocolV1InvalidID tests that PROXY protocol +// containing invalid PROXY protocol version 1 ID should be rejected. +func TestH2H1ProxyProtocolV1InvalidID(t *testing.T) { + st := newServerTester([]string{"--accept-proxy-protocol"}, t, noopHandler) + defer st.Close() + + st.conn.Write([]byte("PR0XY TCP6 ::1 ::1 12345 8080\r\n")) + + _, err := st.http2(requestParam{ + name: "TestH2H1ProxyProtocolV1InvalidID", + }) + + if err == nil { + t.Fatalf("connection was not terminated") + } +} + // TestH2H1GracefulShutdown tests graceful shutdown. func TestH2H1GracefulShutdown(t *testing.T) { st := newServerTester(nil, t, noopHandler) diff --git a/src/shrpx_client_handler.cc b/src/shrpx_client_handler.cc index 3bdb6123..e8a211d4 100644 --- a/src/shrpx_client_handler.cc +++ b/src/shrpx_client_handler.cc @@ -857,6 +857,18 @@ ssize_t parse_proxy_line_port(const uint8_t *first, const uint8_t *last) { } } // namespace +int ClientHandler::on_proxy_protocol_finish() { + setup_upstream_io_callback(); + + // Run on_read to process data left in buffer since they are not + // notified further + if (on_read() != 0) { + return -1; + } + + return 0; +} + int ClientHandler::proxy_protocol_read() { if (LOG_ENABLED(INFO)) { CLOG(INFO, this) << "PROXY-protocol: Started"; @@ -878,7 +890,7 @@ int ClientHandler::proxy_protocol_read() { constexpr const char HEADER[] = "PROXY "; - if (rb_.rleft() < str_size(HEADER)) { + if (end - rb_.pos < str_size(HEADER)) { if (LOG_ENABLED(INFO)) { CLOG(INFO, this) << "PROXY-protocol-v1: PROXY version 1 ID not found"; } @@ -897,7 +909,7 @@ int ClientHandler::proxy_protocol_read() { int family; if (rb_.pos[0] == 'T') { - if (rb_.rleft() < 5) { + if (end - rb_.pos < 5) { if (LOG_ENABLED(INFO)) { CLOG(INFO, this) << "PROXY-protocol-v1: INET protocol family not found"; } @@ -917,7 +929,7 @@ int ClientHandler::proxy_protocol_read() { rb_.drain(5); } else { - if (rb_.rleft() < 7) { + if (end - rb_.pos < 7) { if (LOG_ENABLED(INFO)) { CLOG(INFO, this) << "PROXY-protocol-v1: INET protocol family not found"; } @@ -930,7 +942,9 @@ int ClientHandler::proxy_protocol_read() { return -1; } - return 0; + rb_.drain(end + 2 - rb_.pos); + + return on_proxy_protocol_finish(); } // source address @@ -1012,15 +1026,7 @@ int ClientHandler::proxy_protocol_read() { << " bytes read"; } - setup_upstream_io_callback(); - - // Run on_read to process data left in buffer since they are not - // notified further - if (on_read() != 0) { - return -1; - } - - return 0; + return on_proxy_protocol_finish(); } } // namespace shrpx diff --git a/src/shrpx_client_handler.h b/src/shrpx_client_handler.h index 45ef7a20..55389146 100644 --- a/src/shrpx_client_handler.h +++ b/src/shrpx_client_handler.h @@ -72,6 +72,7 @@ public: int upstream_write(); int proxy_protocol_read(); + int on_proxy_protocol_finish(); // Performs I/O operation. Internally calls on_read()/on_write(). int do_read();