diff --git a/internal/netxlite/errorsx/classify.go b/internal/netxlite/errorsx/classify.go index a6b463f..ea833f9 100644 --- a/internal/netxlite/errorsx/classify.go +++ b/internal/netxlite/errorsx/classify.go @@ -124,19 +124,6 @@ const ( quicTLSUnrecognizedName = 112 ) -// quicIsCertificateError tells us whether a specific TLS alert error -// we received is actually an error depending on the certificate. -// -// The set of checks we implement here is a set of heuristics based -// on our understanding of the TLS spec and may need tweaks. -func quicIsCertificateError(alert uint8) bool { - return (alert == quicTLSAlertBadCertificate || - alert == quicTLSAlertUnsupportedCertificate || - alert == quicTLSAlertCertificateExpired || - alert == quicTLSAlertCertificateRevoked || - alert == quicTLSAlertCertificateUnknown) -} - // ClassifyQUICHandshakeError maps an error occurred during the QUIC // handshake to an OONI failure string. // @@ -196,6 +183,29 @@ func ClassifyQUICHandshakeError(err error) string { return ClassifyGenericError(err) } +// quicIsCertificateError tells us whether a specific TLS alert error +// we received is actually an error depending on the certificate. +// +// The set of checks we implement here is a set of heuristics based +// on our understanding of the TLS spec and may need tweaks. +func quicIsCertificateError(alert uint8) bool { + // List out each case separately so we know we test them + switch alert { + case quicTLSAlertBadCertificate: + return true + case quicTLSAlertUnsupportedCertificate: + return true + case quicTLSAlertCertificateExpired: + return true + case quicTLSAlertCertificateRevoked: + return true + case quicTLSAlertCertificateUnknown: + return true + default: + return false + } +} + // ErrDNSBogon indicates that we found a bogon address. Code that // filters for DNS bogons MUST use this error. var ErrDNSBogon = errors.New("dns: detected bogon address") diff --git a/internal/netxlite/errorsx/classify_test.go b/internal/netxlite/errorsx/classify_test.go index b4b7c23..bcee1a5 100644 --- a/internal/netxlite/errorsx/classify_test.go +++ b/internal/netxlite/errorsx/classify_test.go @@ -2,12 +2,9 @@ package errorsx import ( "context" - "crypto/tls" "crypto/x509" "errors" "io" - "net" - "syscall" "testing" "github.com/google/go-cmp/cmp" @@ -16,6 +13,9 @@ import ( ) func TestClassifyGenericError(t *testing.T) { + // Please, keep this list sorted in the same order + // in which checks appear on the code + t.Run("for input being already an ErrWrapper", func(t *testing.T) { err := &ErrWrapper{Failure: FailureEOFError} if ClassifyGenericError(err) != FailureEOFError { @@ -23,20 +23,18 @@ func TestClassifyGenericError(t *testing.T) { } }) - t.Run("for already wrapped error", func(t *testing.T) { - err := io.EOF - if ClassifyGenericError(err) != FailureEOFError { - t.Fatal("unexpected result") + t.Run("for a system call error", func(t *testing.T) { + if ClassifyGenericError(EWOULDBLOCK) != FailureOperationWouldBlock { + t.Fatal("unexpected results") } }) - t.Run("for context.Canceled", func(t *testing.T) { - if ClassifyGenericError(context.Canceled) != FailureInterrupted { - t.Fatal("unexpected result") - } - }) + // Now we enter into classifyWithStringSuffix. We test it here + // since we want to test the ClassifyGenericError in is + // entirety here and the classifyWithStringSuffix function + // is just an implementation detail. - t.Run("for operation was canceled error", func(t *testing.T) { + t.Run("for operation was canceled", func(t *testing.T) { if ClassifyGenericError(errors.New("operation was canceled")) != FailureInterrupted { t.Fatal("unexpected result") } @@ -44,45 +42,12 @@ func TestClassifyGenericError(t *testing.T) { t.Run("for EOF", func(t *testing.T) { if ClassifyGenericError(io.EOF) != FailureEOFError { - t.Fatal("unexpected results") - } - }) - - t.Run("for canceled", func(t *testing.T) { - if ClassifyGenericError(syscall.ECANCELED) != FailureOperationCanceled { - t.Fatal("unexpected results") - } - }) - - t.Run("for connection_refused", func(t *testing.T) { - if ClassifyGenericError(syscall.ECONNREFUSED) != FailureConnectionRefused { - t.Fatal("unexpected results") - } - }) - - t.Run("for connection_reset", func(t *testing.T) { - if ClassifyGenericError(syscall.ECONNRESET) != FailureConnectionReset { - t.Fatal("unexpected results") - } - }) - - t.Run("for host_unreachable", func(t *testing.T) { - if ClassifyGenericError(syscall.EHOSTUNREACH) != FailureHostUnreachable { - t.Fatal("unexpected results") - } - }) - - t.Run("for system timeout", func(t *testing.T) { - if ClassifyGenericError(syscall.ETIMEDOUT) != FailureTimedOut { - t.Fatal("unexpected results") + t.Fatal("unexpected result") } }) t.Run("for context deadline exceeded", func(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 1) - defer cancel() - <-ctx.Done() - if ClassifyGenericError(ctx.Err()) != FailureGenericTimeoutError { + if ClassifyGenericError(context.DeadlineExceeded) != FailureGenericTimeoutError { t.Fatal("unexpected results") } }) @@ -93,22 +58,13 @@ func TestClassifyGenericError(t *testing.T) { } }) - t.Run("for i/o error", func(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 1) - defer cancel() // fail immediately - conn, err := (&net.Dialer{}).DialContext(ctx, "tcp", "www.google.com:80") - if err == nil { - t.Fatal("expected an error here") - } - if conn != nil { - t.Fatal("expected nil connection here") - } - if ClassifyGenericError(err) != FailureGenericTimeoutError { + t.Run("for i/o timeout", func(t *testing.T) { + if ClassifyGenericError(errors.New("i/o timeout")) != FailureGenericTimeoutError { t.Fatal("unexpected results") } }) - t.Run("for TLS handshake timeout error", func(t *testing.T) { + t.Run("for TLS handshake timeout", func(t *testing.T) { err := errors.New("net/http: TLS handshake timeout") if ClassifyGenericError(err) != FailureGenericTimeoutError { t.Fatal("unexpected results") @@ -116,53 +72,44 @@ func TestClassifyGenericError(t *testing.T) { }) t.Run("for no such host", func(t *testing.T) { - if ClassifyGenericError(&net.DNSError{ - Err: "no such host", - }) != FailureDNSNXDOMAINError { + if ClassifyGenericError(errors.New("no such host")) != FailureDNSNXDOMAINError { t.Fatal("unexpected results") } }) - t.Run("for errors including IPv4 address", func(t *testing.T) { - input := errors.New("read tcp 10.0.2.15:56948->93.184.216.34:443: use of closed network connection") - expected := "unknown_failure: read tcp [scrubbed]->[scrubbed]: use of closed network connection" - out := ClassifyGenericError(input) - if out != expected { - t.Fatal(cmp.Diff(expected, out)) + // Now we're back in ClassifyGenericError + + t.Run("for context.Canceled", func(t *testing.T) { + if ClassifyGenericError(context.Canceled) != FailureInterrupted { + t.Fatal("unexpected result") } }) - t.Run("for errors including IPv6 address", func(t *testing.T) { - input := errors.New("read tcp [::1]:56948->[::1]:443: use of closed network connection") - expected := "unknown_failure: read tcp [scrubbed]->[scrubbed]: use of closed network connection" - out := ClassifyGenericError(input) - if out != expected { - t.Fatal(cmp.Diff(expected, out)) - } - }) + t.Run("for unknown errors", func(t *testing.T) { + t.Run("with an IPv4 address", func(t *testing.T) { + input := errors.New("read tcp 10.0.2.15:56948->93.184.216.34:443: use of closed network connection") + expected := "unknown_failure: read tcp [scrubbed]->[scrubbed]: use of closed network connection" + out := ClassifyGenericError(input) + if out != expected { + t.Fatal(cmp.Diff(expected, out)) + } + }) - t.Run("for i/o error", func(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 1) - defer cancel() // fail immediately - udpAddr := &net.UDPAddr{IP: net.ParseIP("216.58.212.164"), Port: 80, Zone: ""} - udpConn, err := net.ListenUDP("udp", &net.UDPAddr{IP: net.IPv4zero, Port: 0}) - if err != nil { - t.Fatal(err) - } - sess, err := quic.DialEarlyContext(ctx, udpConn, udpAddr, "google.com:80", &tls.Config{}, &quic.Config{}) - if err == nil { - t.Fatal("expected an error here") - } - if sess != nil { - t.Fatal("expected nil session here") - } - if ClassifyGenericError(err) != FailureGenericTimeoutError { - t.Fatal("unexpected results") - } + t.Run("with an IPv6 address", func(t *testing.T) { + input := errors.New("read tcp [::1]:56948->[::1]:443: use of closed network connection") + expected := "unknown_failure: read tcp [scrubbed]->[scrubbed]: use of closed network connection" + out := ClassifyGenericError(input) + if out != expected { + t.Fatal(cmp.Diff(expected, out)) + } + }) }) } func TestClassifyQUICHandshakeError(t *testing.T) { + // Please, keep this list sorted in the same order + // in which checks appear on the code + t.Run("for input being already an ErrWrapper", func(t *testing.T) { err := &ErrWrapper{Failure: FailureEOFError} if ClassifyQUICHandshakeError(err) != FailureEOFError { @@ -170,58 +117,93 @@ func TestClassifyQUICHandshakeError(t *testing.T) { } }) - t.Run("for connection_reset", func(t *testing.T) { - if ClassifyQUICHandshakeError(&quic.StatelessResetError{}) != FailureConnectionReset { - t.Fatal("unexpected results") - } - }) - t.Run("for incompatible quic version", func(t *testing.T) { if ClassifyQUICHandshakeError(&quic.VersionNegotiationError{}) != FailureQUICIncompatibleVersion { t.Fatal("unexpected results") } }) - t.Run("for quic connection refused", func(t *testing.T) { - if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: quic.ConnectionRefused}) != FailureConnectionRefused { + t.Run("for stateless reset", func(t *testing.T) { + if ClassifyQUICHandshakeError(&quic.StatelessResetError{}) != FailureConnectionReset { t.Fatal("unexpected results") } }) - t.Run("for quic handshake timeout", func(t *testing.T) { + t.Run("for handshake timeout", func(t *testing.T) { if ClassifyQUICHandshakeError(&quic.HandshakeTimeoutError{}) != FailureGenericTimeoutError { t.Fatal("unexpected results") } }) - t.Run("for QUIC idle connection timeout", func(t *testing.T) { + t.Run("for idle timeout", func(t *testing.T) { if ClassifyQUICHandshakeError(&quic.IdleTimeoutError{}) != FailureGenericTimeoutError { t.Fatal("unexpected results") } }) - t.Run("for QUIC CRYPTO Handshake", func(t *testing.T) { - var err quic.TransportErrorCode = quicTLSAlertHandshakeFailure - if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLFailedHandshake { + t.Run("for connection refused", func(t *testing.T) { + if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: quic.ConnectionRefused}) != FailureConnectionRefused { t.Fatal("unexpected results") } }) - t.Run("for QUIC CRYPTO Invalid Certificate", func(t *testing.T) { + t.Run("for bad certificate", func(t *testing.T) { var err quic.TransportErrorCode = quicTLSAlertBadCertificate if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLInvalidCertificate { t.Fatal("unexpected results") } }) - t.Run("for QUIC CRYPTO Unknown CA", func(t *testing.T) { + t.Run("for unsupported certificate", func(t *testing.T) { + var err quic.TransportErrorCode = quicTLSAlertUnsupportedCertificate + if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLInvalidCertificate { + t.Fatal("unexpected results") + } + }) + + t.Run("for certificate expired", func(t *testing.T) { + var err quic.TransportErrorCode = quicTLSAlertCertificateExpired + if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLInvalidCertificate { + t.Fatal("unexpected results") + } + }) + + t.Run("for certificate revoked", func(t *testing.T) { + var err quic.TransportErrorCode = quicTLSAlertCertificateRevoked + if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLInvalidCertificate { + t.Fatal("unexpected results") + } + }) + + t.Run("for certificate unknown", func(t *testing.T) { + var err quic.TransportErrorCode = quicTLSAlertCertificateUnknown + if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLInvalidCertificate { + t.Fatal("unexpected results") + } + }) + + t.Run("for decrypt error", func(t *testing.T) { + var err quic.TransportErrorCode = quicTLSAlertDecryptError + if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLFailedHandshake { + t.Fatal("unexpected results") + } + }) + + t.Run("for handshake failure", func(t *testing.T) { + var err quic.TransportErrorCode = quicTLSAlertHandshakeFailure + if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLFailedHandshake { + t.Fatal("unexpected results") + } + }) + + t.Run("for unknown CA", func(t *testing.T) { var err quic.TransportErrorCode = quicTLSAlertUnknownCA if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLUnknownAuthority { t.Fatal("unexpected results") } }) - t.Run("for QUIC CRYPTO Bad Hostname", func(t *testing.T) { + t.Run("for unrecognized hostname", func(t *testing.T) { var err quic.TransportErrorCode = quicTLSUnrecognizedName if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLInvalidHostname { t.Fatal("unexpected results") @@ -236,6 +218,9 @@ func TestClassifyQUICHandshakeError(t *testing.T) { } func TestClassifyResolverError(t *testing.T) { + // Please, keep this list sorted in the same order + // in which checks appear on the code + t.Run("for input being already an ErrWrapper", func(t *testing.T) { err := &ErrWrapper{Failure: FailureEOFError} if ClassifyResolverError(err) != FailureEOFError { @@ -257,6 +242,9 @@ func TestClassifyResolverError(t *testing.T) { } func TestClassifyTLSHandshakeError(t *testing.T) { + // Please, keep this list sorted in the same order + // in which checks appear on the code + t.Run("for input being already an ErrWrapper", func(t *testing.T) { err := &ErrWrapper{Failure: FailureEOFError} if ClassifyTLSHandshakeError(err) != FailureEOFError {