fix(netx): make sure we save quic udp conn events (#423)

https://github.com/ooni/probe-cli/pull/421 was wrong because we need
a more rich interface for quic-go to call ReadMsgUDP.

With this commit, we use such an interface: OOBCapablePacketConn.

Still part of https://github.com/ooni/probe/issues/1505.
This commit is contained in:
Simone Basso 2021-07-02 11:00:12 +02:00 committed by GitHub
parent 30c7e2cdb3
commit ceb2aa8a8d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 82 additions and 49 deletions

View File

@ -4,15 +4,15 @@ import (
"net" "net"
"time" "time"
"github.com/lucas-clemente/quic-go"
"github.com/ooni/probe-cli/v3/internal/engine/netx/trace" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace"
"github.com/ooni/probe-cli/v3/internal/errorsx" "github.com/ooni/probe-cli/v3/internal/errorsx"
"github.com/ooni/probe-cli/v3/internal/quicx"
) )
// QUICListener listens for QUIC connections. // QUICListener listens for QUIC connections.
type QUICListener interface { type QUICListener interface {
// Listen creates a new listening UDPConn. // Listen creates a new listening UDPConn.
Listen(addr *net.UDPAddr) (quicx.UDPConn, error) Listen(addr *net.UDPAddr) (quic.OOBCapablePacketConn, error)
} }
// QUICListenerSaver is a QUICListener that also implements saving events. // QUICListenerSaver is a QUICListener that also implements saving events.
@ -25,22 +25,22 @@ type QUICListenerSaver struct {
} }
// Listen implements QUICListener.Listen. // Listen implements QUICListener.Listen.
func (qls *QUICListenerSaver) Listen(addr *net.UDPAddr) (quicx.UDPConn, error) { func (qls *QUICListenerSaver) Listen(addr *net.UDPAddr) (quic.OOBCapablePacketConn, error) {
pconn, err := qls.QUICListener.Listen(addr) pconn, err := qls.QUICListener.Listen(addr)
if err != nil { if err != nil {
return nil, err return nil, err
} }
return saverUDPConn{UDPConn: pconn, saver: qls.Saver}, nil return saverUDPConn{OOBCapablePacketConn: pconn, saver: qls.Saver}, nil
} }
type saverUDPConn struct { type saverUDPConn struct {
quicx.UDPConn quic.OOBCapablePacketConn
saver *trace.Saver saver *trace.Saver
} }
func (c saverUDPConn) WriteTo(p []byte, addr net.Addr) (int, error) { func (c saverUDPConn) WriteTo(p []byte, addr net.Addr) (int, error) {
start := time.Now() start := time.Now()
count, err := c.UDPConn.WriteTo(p, addr) count, err := c.OOBCapablePacketConn.WriteTo(p, addr)
stop := time.Now() stop := time.Now()
c.saver.Write(trace.Event{ c.saver.Write(trace.Event{
Address: addr.String(), Address: addr.String(),
@ -56,7 +56,7 @@ func (c saverUDPConn) WriteTo(p []byte, addr net.Addr) (int, error) {
func (c saverUDPConn) ReadMsgUDP(b, oob []byte) (int, int, int, *net.UDPAddr, error) { func (c saverUDPConn) ReadMsgUDP(b, oob []byte) (int, int, int, *net.UDPAddr, error) {
start := time.Now() start := time.Now()
n, oobn, flags, addr, err := c.UDPConn.ReadMsgUDP(b, oob) n, oobn, flags, addr, err := c.OOBCapablePacketConn.ReadMsgUDP(b, oob)
stop := time.Now() stop := time.Now()
var data []byte var data []byte
if n > 0 { if n > 0 {

View File

@ -6,7 +6,6 @@ import (
"net" "net"
"github.com/lucas-clemente/quic-go" "github.com/lucas-clemente/quic-go"
"github.com/ooni/probe-cli/v3/internal/quicx"
) )
// QUICContextDialer is a dialer for QUIC using Context. // QUICContextDialer is a dialer for QUIC using Context.
@ -21,7 +20,7 @@ type QUICContextDialer interface {
// QUICListener listens for QUIC connections. // QUICListener listens for QUIC connections.
type QUICListener interface { type QUICListener interface {
// Listen creates a new listening UDPConn. // Listen creates a new listening UDPConn.
Listen(addr *net.UDPAddr) (quicx.UDPConn, error) Listen(addr *net.UDPAddr) (quic.OOBCapablePacketConn, error)
} }
// ErrorWrapperQUICListener is a QUICListener that wraps errors. // ErrorWrapperQUICListener is a QUICListener that wraps errors.
@ -33,7 +32,7 @@ type ErrorWrapperQUICListener struct {
var _ QUICListener = &ErrorWrapperQUICListener{} var _ QUICListener = &ErrorWrapperQUICListener{}
// Listen implements QUICListener.Listen. // Listen implements QUICListener.Listen.
func (qls *ErrorWrapperQUICListener) Listen(addr *net.UDPAddr) (quicx.UDPConn, error) { func (qls *ErrorWrapperQUICListener) Listen(addr *net.UDPAddr) (quic.OOBCapablePacketConn, error) {
pconn, err := qls.QUICListener.Listen(addr) pconn, err := qls.QUICListener.Listen(addr)
if err != nil { if err != nil {
return nil, SafeErrWrapperBuilder{ return nil, SafeErrWrapperBuilder{
@ -44,17 +43,17 @@ func (qls *ErrorWrapperQUICListener) Listen(addr *net.UDPAddr) (quicx.UDPConn, e
return &errorWrapperUDPConn{pconn}, nil return &errorWrapperUDPConn{pconn}, nil
} }
// errorWrapperUDPConn is a quicx.UDPConn that wraps errors. // errorWrapperUDPConn is a quic.OOBCapablePacketConn that wraps errors.
type errorWrapperUDPConn struct { type errorWrapperUDPConn struct {
// UDPConn is the underlying conn. // OOBCapablePacketConn is the underlying conn.
quicx.UDPConn quic.OOBCapablePacketConn
} }
var _ quicx.UDPConn = &errorWrapperUDPConn{} var _ quic.OOBCapablePacketConn = &errorWrapperUDPConn{}
// WriteTo implements quicx.UDPConn.WriteTo. // WriteTo implements quic.OOBCapablePacketConn.WriteTo.
func (c *errorWrapperUDPConn) WriteTo(p []byte, addr net.Addr) (int, error) { func (c *errorWrapperUDPConn) WriteTo(p []byte, addr net.Addr) (int, error) {
count, err := c.UDPConn.WriteTo(p, addr) count, err := c.OOBCapablePacketConn.WriteTo(p, addr)
if err != nil { if err != nil {
return 0, SafeErrWrapperBuilder{ return 0, SafeErrWrapperBuilder{
Error: err, Error: err,
@ -64,9 +63,9 @@ func (c *errorWrapperUDPConn) WriteTo(p []byte, addr net.Addr) (int, error) {
return count, nil return count, nil
} }
// ReadMsgUDP implements quicx.UDPConn.ReadMsgUDP. // ReadMsgUDP implements quic.OOBCapablePacketConn.ReadMsgUDP.
func (c *errorWrapperUDPConn) ReadMsgUDP(b, oob []byte) (int, int, int, *net.UDPAddr, error) { func (c *errorWrapperUDPConn) ReadMsgUDP(b, oob []byte) (int, int, int, *net.UDPAddr, error) {
n, oobn, flags, addr, err := c.UDPConn.ReadMsgUDP(b, oob) n, oobn, flags, addr, err := c.OOBCapablePacketConn.ReadMsgUDP(b, oob)
if err != nil { if err != nil {
return 0, 0, 0, nil, SafeErrWrapperBuilder{ return 0, 0, 0, nil, SafeErrWrapperBuilder{
Error: err, Error: err,

View File

@ -10,13 +10,12 @@ import (
"github.com/lucas-clemente/quic-go" "github.com/lucas-clemente/quic-go"
"github.com/ooni/probe-cli/v3/internal/netxmocks" "github.com/ooni/probe-cli/v3/internal/netxmocks"
"github.com/ooni/probe-cli/v3/internal/quicx"
) )
func TestErrorWrapperQUICListenerSuccess(t *testing.T) { func TestErrorWrapperQUICListenerSuccess(t *testing.T) {
ql := &ErrorWrapperQUICListener{ ql := &ErrorWrapperQUICListener{
QUICListener: &netxmocks.QUICListener{ QUICListener: &netxmocks.QUICListener{
MockListen: func(addr *net.UDPAddr) (quicx.UDPConn, error) { MockListen: func(addr *net.UDPAddr) (quic.OOBCapablePacketConn, error) {
return &net.UDPConn{}, nil return &net.UDPConn{}, nil
}, },
}, },
@ -31,7 +30,7 @@ func TestErrorWrapperQUICListenerSuccess(t *testing.T) {
func TestErrorWrapperQUICListenerFailure(t *testing.T) { func TestErrorWrapperQUICListenerFailure(t *testing.T) {
ql := &ErrorWrapperQUICListener{ ql := &ErrorWrapperQUICListener{
QUICListener: &netxmocks.QUICListener{ QUICListener: &netxmocks.QUICListener{
MockListen: func(addr *net.UDPAddr) (quicx.UDPConn, error) { MockListen: func(addr *net.UDPAddr) (quic.OOBCapablePacketConn, error) {
return nil, io.EOF return nil, io.EOF
}, },
}, },
@ -47,7 +46,7 @@ func TestErrorWrapperQUICListenerFailure(t *testing.T) {
func TestErrorWrapperUDPConnWriteToSuccess(t *testing.T) { func TestErrorWrapperUDPConnWriteToSuccess(t *testing.T) {
quc := &errorWrapperUDPConn{ quc := &errorWrapperUDPConn{
UDPConn: &netxmocks.QUICUDPConn{ OOBCapablePacketConn: &netxmocks.QUICUDPConn{
MockWriteTo: func(p []byte, addr net.Addr) (int, error) { MockWriteTo: func(p []byte, addr net.Addr) (int, error) {
return 10, nil return 10, nil
}, },
@ -67,7 +66,7 @@ func TestErrorWrapperUDPConnWriteToSuccess(t *testing.T) {
func TestErrorWrapperUDPConnWriteToFailure(t *testing.T) { func TestErrorWrapperUDPConnWriteToFailure(t *testing.T) {
expected := errors.New("mocked error") expected := errors.New("mocked error")
quc := &errorWrapperUDPConn{ quc := &errorWrapperUDPConn{
UDPConn: &netxmocks.QUICUDPConn{ OOBCapablePacketConn: &netxmocks.QUICUDPConn{
MockWriteTo: func(p []byte, addr net.Addr) (int, error) { MockWriteTo: func(p []byte, addr net.Addr) (int, error) {
return 0, expected return 0, expected
}, },
@ -87,7 +86,7 @@ func TestErrorWrapperUDPConnWriteToFailure(t *testing.T) {
func TestErrorWrapperUDPConnReadMsgUDPSuccess(t *testing.T) { func TestErrorWrapperUDPConnReadMsgUDPSuccess(t *testing.T) {
expected := errors.New("mocked error") expected := errors.New("mocked error")
quc := &errorWrapperUDPConn{ quc := &errorWrapperUDPConn{
UDPConn: &netxmocks.QUICUDPConn{ OOBCapablePacketConn: &netxmocks.QUICUDPConn{
MockReadMsgUDP: func(b, oob []byte) (int, int, int, *net.UDPAddr, error) { MockReadMsgUDP: func(b, oob []byte) (int, int, int, *net.UDPAddr, error) {
return 0, 0, 0, nil, expected return 0, 0, 0, nil, expected
}, },
@ -115,7 +114,7 @@ func TestErrorWrapperUDPConnReadMsgUDPSuccess(t *testing.T) {
func TestErrorWrapperUDPConnReadMsgUDPFailure(t *testing.T) { func TestErrorWrapperUDPConnReadMsgUDPFailure(t *testing.T) {
quc := &errorWrapperUDPConn{ quc := &errorWrapperUDPConn{
UDPConn: &netxmocks.QUICUDPConn{ OOBCapablePacketConn: &netxmocks.QUICUDPConn{
MockReadMsgUDP: func(b, oob []byte) (int, int, int, *net.UDPAddr, error) { MockReadMsgUDP: func(b, oob []byte) (int, int, int, *net.UDPAddr, error) {
return 10, 1, 0, nil, nil return 10, 1, 0, nil, nil
}, },

View File

@ -8,7 +8,6 @@ import (
"strconv" "strconv"
"github.com/lucas-clemente/quic-go" "github.com/lucas-clemente/quic-go"
"github.com/ooni/probe-cli/v3/internal/quicx"
) )
// QUICContextDialer is a dialer for QUIC using Context. // QUICContextDialer is a dialer for QUIC using Context.
@ -23,7 +22,7 @@ type QUICContextDialer interface {
// QUICListener listens for QUIC connections. // QUICListener listens for QUIC connections.
type QUICListener interface { type QUICListener interface {
// Listen creates a new listening UDPConn. // Listen creates a new listening UDPConn.
Listen(addr *net.UDPAddr) (quicx.UDPConn, error) Listen(addr *net.UDPAddr) (quic.OOBCapablePacketConn, error)
} }
// QUICListenerStdlib is a QUICListener using the standard library. // QUICListenerStdlib is a QUICListener using the standard library.
@ -32,7 +31,7 @@ type QUICListenerStdlib struct{}
var _ QUICListener = &QUICListenerStdlib{} var _ QUICListener = &QUICListenerStdlib{}
// Listen implements QUICListener.Listen. // Listen implements QUICListener.Listen.
func (qls *QUICListenerStdlib) Listen(addr *net.UDPAddr) (quicx.UDPConn, error) { func (qls *QUICListenerStdlib) Listen(addr *net.UDPAddr) (quic.OOBCapablePacketConn, error) {
return net.ListenUDP("udp", addr) return net.ListenUDP("udp", addr)
} }

View File

@ -12,7 +12,6 @@ import (
"github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp"
"github.com/lucas-clemente/quic-go" "github.com/lucas-clemente/quic-go"
"github.com/ooni/probe-cli/v3/internal/netxmocks" "github.com/ooni/probe-cli/v3/internal/netxmocks"
"github.com/ooni/probe-cli/v3/internal/quicx"
) )
func TestQUICDialerQUICGoCannotSplitHostPort(t *testing.T) { func TestQUICDialerQUICGoCannotSplitHostPort(t *testing.T) {
@ -76,7 +75,7 @@ func TestQUICDialerQUICGoCannotListen(t *testing.T) {
} }
systemdialer := QUICDialerQUICGo{ systemdialer := QUICDialerQUICGo{
QUICListener: &netxmocks.QUICListener{ QUICListener: &netxmocks.QUICListener{
MockListen: func(addr *net.UDPAddr) (quicx.UDPConn, error) { MockListen: func(addr *net.UDPAddr) (quic.OOBCapablePacketConn, error) {
return nil, expected return nil, expected
}, },
}, },

View File

@ -4,19 +4,19 @@ import (
"context" "context"
"crypto/tls" "crypto/tls"
"net" "net"
"syscall"
"time" "time"
"github.com/lucas-clemente/quic-go" "github.com/lucas-clemente/quic-go"
"github.com/ooni/probe-cli/v3/internal/quicx"
) )
// QUICListener is a mockable netxlite.QUICListener. // QUICListener is a mockable netxlite.QUICListener.
type QUICListener struct { type QUICListener struct {
MockListen func(addr *net.UDPAddr) (quicx.UDPConn, error) MockListen func(addr *net.UDPAddr) (quic.OOBCapablePacketConn, error)
} }
// Listen calls MockListen. // Listen calls MockListen.
func (ql *QUICListener) Listen(addr *net.UDPAddr) (quicx.UDPConn, error) { func (ql *QUICListener) Listen(addr *net.UDPAddr) (quic.OOBCapablePacketConn, error) {
return ql.MockListen(addr) return ql.MockListen(addr)
} }
@ -140,9 +140,11 @@ type QUICUDPConn struct {
MockSetReadDeadline func(t time.Time) error MockSetReadDeadline func(t time.Time) error
MockSetWriteDeadline func(t time.Time) error MockSetWriteDeadline func(t time.Time) error
MockReadFrom func(p []byte) (n int, addr net.Addr, err error) MockReadFrom func(p []byte) (n int, addr net.Addr, err error)
MockSyscallConn func() (syscall.RawConn, error)
MockWriteMsgUDP func(b, oob []byte, addr *net.UDPAddr) (n, oobn int, err error)
} }
var _ net.PacketConn = &QUICUDPConn{} var _ quic.OOBCapablePacketConn = &QUICUDPConn{}
// WriteTo calls MockWriteTo. // WriteTo calls MockWriteTo.
func (c *QUICUDPConn) WriteTo(p []byte, addr net.Addr) (int, error) { func (c *QUICUDPConn) WriteTo(p []byte, addr net.Addr) (int, error) {
@ -188,3 +190,13 @@ func (c *QUICUDPConn) SetWriteDeadline(t time.Time) error {
func (c *QUICUDPConn) ReadFrom(b []byte) (int, net.Addr, error) { func (c *QUICUDPConn) ReadFrom(b []byte) (int, net.Addr, error) {
return c.MockReadFrom(b) return c.MockReadFrom(b)
} }
// SyscallConn calls MockSyscallConn.
func (c *QUICUDPConn) SyscallConn() (syscall.RawConn, error) {
return c.MockSyscallConn()
}
// WriteMsgUDP calls MockReadMsgUDP.
func (c *QUICUDPConn) WriteMsgUDP(b, oob []byte, addr *net.UDPAddr) (n, oobn int, err error) {
return c.MockWriteMsgUDP(b, oob, addr)
}

View File

@ -6,18 +6,18 @@ import (
"errors" "errors"
"net" "net"
"reflect" "reflect"
"syscall"
"testing" "testing"
"time" "time"
"github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp"
"github.com/lucas-clemente/quic-go" "github.com/lucas-clemente/quic-go"
"github.com/ooni/probe-cli/v3/internal/quicx"
) )
func TestQUICListenerListen(t *testing.T) { func TestQUICListenerListen(t *testing.T) {
expected := errors.New("mocked error") expected := errors.New("mocked error")
ql := &QUICListener{ ql := &QUICListener{
MockListen: func(addr *net.UDPAddr) (quicx.UDPConn, error) { MockListen: func(addr *net.UDPAddr) (quic.OOBCapablePacketConn, error) {
return nil, expected return nil, expected
}, },
} }
@ -417,3 +417,41 @@ func TestQUICUDPConnReadFrom(t *testing.T) {
t.Fatal("expected nil here") t.Fatal("expected nil here")
} }
} }
func TestQUICUDPConnSyscallConn(t *testing.T) {
expected := errors.New("mocked error")
quc := &QUICUDPConn{
MockSyscallConn: func() (syscall.RawConn, error) {
return nil, expected
},
}
conn, err := quc.SyscallConn()
if !errors.Is(err, expected) {
t.Fatal("not the error we expected", err)
}
if conn != nil {
t.Fatal("expected nil here")
}
}
func TestQUICUDPConnWriteMsgUDP(t *testing.T) {
expected := errors.New("mocked error")
quc := &QUICUDPConn{
MockWriteMsgUDP: func(b, oob []byte, addr *net.UDPAddr) (n int, oobn int, err error) {
return 0, 0, expected
},
}
b := make([]byte, 128)
oob := make([]byte, 128)
addr := &net.UDPAddr{}
n, oobn, err := quc.WriteMsgUDP(b, oob, addr)
if !errors.Is(err, expected) {
t.Fatal("not the error we expected", err)
}
if n != 0 {
t.Fatal("expected 0 here")
}
if oobn != 0 {
t.Fatal("expected 0 here")
}
}

View File

@ -1,13 +0,0 @@
// Package quicx contains definitions useful to implement QUIC.
package quicx
import "net"
// UDPConn is an UDP connection used by quic.
type UDPConn interface {
// PacketConn is the underlying base interface.
net.PacketConn
// ReadMsgUDP behaves like net.UDPConn.ReadMsgUDP.
ReadMsgUDP(b, oob []byte) (n, oobn, flags int, addr *net.UDPAddr, err error)
}