Skip to content

Commit 7f48261

Browse files
pirespires
authored
Merge pull request #71 from isedev/feature/issue-69-tests
Disallow header parsing beyond 107 bytes
2 parents c4bcea2 + a368adc commit 7f48261

File tree

4 files changed

+240
-16
lines changed

4 files changed

+240
-16
lines changed

‎header.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@ var (
1616
SIGV1 = []byte{'\x50', '\x52', '\x4F', '\x58', '\x59'}
1717
SIGV2 = []byte{'\x0D', '\x0A', '\x0D', '\x0A', '\x00', '\x0D', '\x0A', '\x51', '\x55', '\x49', '\x54', '\x0A'}
1818

19-
ErrLineMustEndWithCrlf = errors.New("proxyproto: header is invalid, must end with \\r\\n")
19+
ErrCantReadVersion1Header = errors.New("proxyproto: can't read version 1 header")
20+
ErrVersion1HeaderTooLong = errors.New("proxyproto: version 1 header must be 107 bytes or less")
21+
ErrLineMustEndWithCrlf = errors.New("proxyproto: version 1 header is invalid, must end with \\r\\n")
2022
ErrCantReadProtocolVersionAndCommand = errors.New("proxyproto: can't read proxy protocol version and command")
2123
ErrCantReadAddressFamilyAndProtocol = errors.New("proxyproto: can't read address family or protocol")
2224
ErrCantReadLength = errors.New("proxyproto: can't read length")

‎header_test.go

+6-5
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,12 @@ import (
1313
// Stuff to be used in both versions tests.
1414

1515
const (
16-
NO_PROTOCOL = "There is no spoon"
17-
IP4_ADDR = "127.0.0.1"
18-
IP6_ADDR = "::1"
19-
PORT = 65533
20-
INVALID_PORT = 99999
16+
NO_PROTOCOL = "There is no spoon"
17+
IP4_ADDR = "127.0.0.1"
18+
IP6_ADDR = "::1"
19+
IP6_LONG_ADDR = "1234:5678:9abc:def0:cafe:babe:dead:2bad"
20+
PORT = 65533
21+
INVALID_PORT = 99999
2122
)
2223

2324
var (

‎v1.go

+71-8
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package proxyproto
33
import (
44
"bufio"
55
"bytes"
6+
"fmt"
67
"net"
78
"strconv"
89
"strings"
@@ -22,17 +23,79 @@ func initVersion1() *Header {
2223
}
2324

2425
func parseVersion1(reader *bufio.Reader) (*Header, error) {
25-
// Read until LF shows up, otherwise fail.
26-
// At this point, can't be sure CR precedes LF which will be validated next.
27-
line, err := reader.ReadString('\n')
28-
if err != nil {
29-
return nil, ErrLineMustEndWithCrlf
30-
}
31-
if !strings.HasSuffix(line, crlf) {
26+
//The header cannot be more than 107 bytes long. Per spec:
27+
//
28+
// (...)
29+
// - worst case (optional fields set to 0xff) :
30+
// "PROXY UNKNOWN ffff:f...f:ffff ffff:f...f:ffff 65535 65535\r\n"
31+
// => 5 + 1 + 7 + 1 + 39 + 1 + 39 + 1 + 5 + 1 + 5 + 2 = 107 chars
32+
//
33+
// So a 108-byte buffer is always enough to store all the line and a
34+
// trailing zero for string processing.
35+
//
36+
// It must also be CRLF terminated, as above. The header does not otherwise
37+
// contain a CR or LF byte.
38+
//
39+
// ISSUE #69
40+
// We can't use Peek here as it will block trying to fill the buffer, which
41+
// will never happen if the header is TCP4 or TCP6 (max. 56 and 104 bytes
42+
// respectively) and the server is expected to speak first.
43+
//
44+
// Similarly, we can't use ReadString or ReadBytes as these will keep reading
45+
// until the delimiter is found; an abusive client could easily disrupt a
46+
// server by sending a large amount of data that do not contain a LF byte.
47+
// Another means of attack would be to start connections and simply not send
48+
// data after the initial PROXY signature bytes, accumulating a large
49+
// number of blocked goroutines on the server. ReadSlice will also block for
50+
// a delimiter when the internal buffer does not fill up.
51+
//
52+
// A plain Read is also problematic since we risk reading past the end of the
53+
// header without being able to easily put the excess bytes back into the reader's
54+
// buffer (with the current implementation's design).
55+
//
56+
// So we use a ReadByte loop, which solves the overflow problem and avoids
57+
// reading beyond the end of the header. However, we need one more trick to harden
58+
// against partial header attacks (slow loris) - per spec:
59+
//
60+
// (..) The sender must always ensure that the header is sent at once, so that
61+
// the transport layer maintains atomicity along the path to the receiver. The
62+
// receiver may be tolerant to partial headers or may simply drop the connection
63+
// when receiving a partial header. Recommendation is to be tolerant, but
64+
// implementation constraints may not always easily permit this.
65+
//
66+
// We are subject to such implementation constraints. So we return an error if
67+
// the header cannot be fully extracted with a single read of the underlying
68+
// reader.
69+
buf := make([]byte, 0, 107)
70+
for {
71+
b, err := reader.ReadByte()
72+
if err != nil {
73+
return nil, fmt.Errorf(ErrCantReadVersion1Header.Error()+": %v", err)
74+
}
75+
buf = append(buf, b)
76+
if b == '\n' {
77+
// End of header found
78+
break
79+
}
80+
if len(buf) == 107 {
81+
// No delimiter in first 107 bytes
82+
return nil, ErrVersion1HeaderTooLong
83+
}
84+
if reader.Buffered() == 0 {
85+
// Header was not buffered in a single read. Since we can't
86+
// differentiate between genuine slow writers and DoS agents,
87+
// we abort. On healthy networks, this should never happen.
88+
return nil, ErrCantReadVersion1Header
89+
}
90+
}
91+
92+
// Check for CR before LF.
93+
if len(buf) < 2 || buf[len(buf)-2] != '\r' {
3294
return nil, ErrLineMustEndWithCrlf
3395
}
96+
3497
// Check full signature.
35-
tokens := strings.Split(line[:len(line)-2], separator)
98+
tokens := strings.Split(string(buf[:len(buf)-2]), separator)
3699

37100
// Expect at least 2 tokens: "PROXY" and the transport protocol.
38101
if len(tokens) < 2 {

‎v1_test.go

+160-2
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,25 @@ package proxyproto
33
import (
44
"bufio"
55
"bytes"
6+
"io"
7+
"net"
68
"strconv"
79
"strings"
810
"testing"
11+
"time"
912
)
1013

1114
var (
1215
IPv4AddressesAndPorts = strings.Join([]string{IP4_ADDR, IP4_ADDR, strconv.Itoa(PORT), strconv.Itoa(PORT)}, separator)
1316
IPv4AddressesAndInvalidPorts = strings.Join([]string{IP4_ADDR, IP4_ADDR, strconv.Itoa(INVALID_PORT), strconv.Itoa(INVALID_PORT)}, separator)
1417
IPv6AddressesAndPorts = strings.Join([]string{IP6_ADDR, IP6_ADDR, strconv.Itoa(PORT), strconv.Itoa(PORT)}, separator)
18+
IPv6LongAddressesAndPorts = strings.Join([]string{IP6_LONG_ADDR, IP6_LONG_ADDR, strconv.Itoa(PORT), strconv.Itoa(PORT)}, separator)
1519

1620
fixtureTCP4V1 = "PROXY TCP4 " + IPv4AddressesAndPorts + crlf + "GET /"
1721
fixtureTCP6V1 = "PROXY TCP6 " + IPv6AddressesAndPorts + crlf + "GET /"
1822

23+
fixtureTCP6V1Overflow = "PROXY TCP6 " + IPv6LongAddressesAndPorts
24+
1925
fixtureUnknown = "PROXY UNKNOWN" + crlf
2026
fixtureUnknownWithAddresses = "PROXY UNKNOWN " + IPv4AddressesAndInvalidPorts + crlf
2127
)
@@ -58,7 +64,7 @@ var invalidParseV1Tests = []struct {
5864
{
5965
desc: "incomplete signature TCP4",
6066
reader: newBufioReader([]byte("PROXY TCP4 " + IPv4AddressesAndPorts)),
61-
expectedError: ErrLineMustEndWithCrlf,
67+
expectedError: ErrCantReadVersion1Header,
6268
},
6369
{
6470
desc: "TCP6 with IPv4 addresses",
@@ -75,13 +81,18 @@ var invalidParseV1Tests = []struct {
7581
reader: newBufioReader([]byte("PROXY TCP4 " + IPv4AddressesAndInvalidPorts + crlf)),
7682
expectedError: ErrInvalidPortNumber,
7783
},
84+
{
85+
desc: "header too long",
86+
reader: newBufioReader([]byte("PROXY UNKNOWN " + IPv6LongAddressesAndPorts + " " + crlf)),
87+
expectedError: ErrVersion1HeaderTooLong,
88+
},
7889
}
7990

8091
func TestReadV1Invalid(t *testing.T) {
8192
for _, tt := range invalidParseV1Tests {
8293
t.Run(tt.desc, func(t *testing.T) {
8394
if _, err := Read(tt.reader); err != tt.expectedError {
84-
t.Fatalf("expected %s, actual %s", tt.expectedError, err.Error())
95+
t.Fatalf("expected %s, actual %v", tt.expectedError, err)
8596
}
8697
})
8798
}
@@ -175,3 +186,150 @@ func TestWriteV1Valid(t *testing.T) {
175186
})
176187
}
177188
}
189+
190+
// Tests for parseVersion1 overflow - issue #69.
191+
192+
type dataSource struct {
193+
NBytes int
194+
NRead int
195+
}
196+
197+
func (ds *dataSource) Read(b []byte) (int, error) {
198+
if ds.NRead >= ds.NBytes {
199+
return 0, io.EOF
200+
}
201+
avail := ds.NBytes - ds.NRead
202+
if len(b) < avail {
203+
avail = len(b)
204+
}
205+
for i := 0; i < avail; i++ {
206+
b[i] = 0x20
207+
}
208+
ds.NRead += avail
209+
return avail, nil
210+
}
211+
212+
func TestParseVersion1Overflow(t *testing.T) {
213+
ds := &dataSource{}
214+
reader := bufio.NewReader(ds)
215+
bufSize := reader.Size()
216+
ds.NBytes = bufSize * 16
217+
parseVersion1(reader)
218+
if ds.NRead > bufSize {
219+
t.Fatalf("read: expected max %d bytes, actual %d\n", bufSize, ds.NRead)
220+
}
221+
}
222+
223+
func listen(t *testing.T) *Listener {
224+
l, err := net.Listen("tcp", "127.0.0.1:0")
225+
if err != nil {
226+
t.Fatalf("listen: %v", err)
227+
}
228+
return &Listener{Listener: l}
229+
}
230+
231+
func client(t *testing.T, addr, header string, length int, terminate bool, wait time.Duration, done chan struct{}) {
232+
c, err := net.Dial("tcp", addr)
233+
if err != nil {
234+
t.Fatalf("dial: %v", err)
235+
}
236+
defer c.Close()
237+
238+
if terminate && length < 2 {
239+
length = 2
240+
}
241+
242+
buf := make([]byte, len(header)+length)
243+
copy(buf, []byte(header))
244+
for i := 0; i < length-2; i++ {
245+
buf[i+len(header)] = 0x20
246+
}
247+
if terminate {
248+
copy(buf[len(header)+length-2:], []byte(crlf))
249+
}
250+
251+
n, err := c.Write(buf)
252+
if err != nil {
253+
t.Fatalf("write: %v", err)
254+
}
255+
if n != len(buf) {
256+
t.Fatalf("write; short write")
257+
}
258+
259+
time.Sleep(wait)
260+
close(done)
261+
}
262+
263+
func TestVersion1Overflow(t *testing.T) {
264+
done := make(chan struct{})
265+
266+
l := listen(t)
267+
go client(t, l.Addr().String(), fixtureTCP6V1Overflow, 10240, true, 10*time.Second, done)
268+
269+
c, err := l.Accept()
270+
if err != nil {
271+
t.Fatalf("accept: %v", err)
272+
}
273+
274+
b := []byte{}
275+
_, err = c.Read(b)
276+
if err == nil {
277+
t.Fatalf("net.Conn: no error reported for oversized header")
278+
}
279+
}
280+
281+
func TestVersion1SlowLoris(t *testing.T) {
282+
done := make(chan struct{})
283+
timeout := make(chan error)
284+
285+
l := listen(t)
286+
go client(t, l.Addr().String(), fixtureTCP6V1Overflow, 0, false, 10*time.Second, done)
287+
288+
c, err := l.Accept()
289+
if err != nil {
290+
t.Fatalf("accept: %v", err)
291+
}
292+
293+
go func() {
294+
b := []byte{}
295+
_, err = c.Read(b)
296+
timeout <- err
297+
}()
298+
299+
select {
300+
case <-done:
301+
t.Fatalf("net.Conn: reader still blocked after 10 seconds")
302+
case err := <-timeout:
303+
if err == nil {
304+
t.Fatalf("net.Conn: no error reported for incomplete header")
305+
}
306+
}
307+
}
308+
309+
func TestVersion1SlowLorisOverflow(t *testing.T) {
310+
done := make(chan struct{})
311+
timeout := make(chan error)
312+
313+
l := listen(t)
314+
go client(t, l.Addr().String(), fixtureTCP6V1Overflow, 10240, false, 10*time.Second, done)
315+
316+
c, err := l.Accept()
317+
if err != nil {
318+
t.Fatalf("accept: %v", err)
319+
}
320+
321+
go func() {
322+
b := []byte{}
323+
_, err = c.Read(b)
324+
timeout <- err
325+
}()
326+
327+
select {
328+
case <-done:
329+
t.Fatalf("net.Conn: reader still blocked after 10 seconds")
330+
case err := <-timeout:
331+
if err == nil {
332+
t.Fatalf("net.Conn: no error reported for incomplete and overflowed header")
333+
}
334+
}
335+
}

0 commit comments

Comments
 (0)