diff --git a/integrationtests/self/mitm_test.go b/integrationtests/self/mitm_test.go index dd7ea54b8..860da5afd 100644 --- a/integrationtests/self/mitm_test.go +++ b/integrationtests/self/mitm_test.go @@ -371,7 +371,7 @@ var _ = Describe("MITM test", func() { } err := runTest(delayCb) Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("No compatible QUIC version found.")) + Expect(err.Error()).To(ContainSubstring("no compatible QUIC version found")) }) // times out, because client doesn't accept subsequent real retry packets from server diff --git a/logging/close_reason.go b/logging/close_reason.go index e35e47425..135a3be45 100644 --- a/logging/close_reason.go +++ b/logging/close_reason.go @@ -13,6 +13,7 @@ type CloseReason struct { timeout *TimeoutReason statelessResetToken *StatelessResetToken + versions []VersionNumber } // NewApplicationCloseReason creates a new CloseReason for an application error. @@ -35,6 +36,11 @@ func NewStatelessResetCloseReason(token StatelessResetToken) CloseReason { return CloseReason{statelessResetToken: &token} } +// NewVersionNegotiationError creates a new CloseReason for a version negotiation error. +func NewVersionNegotiationError(versions []VersionNumber) CloseReason { + return CloseReason{versions: versions} +} + // ApplicationError gets the application error. func (r *CloseReason) ApplicationError() (errorCode ApplicationError, remote bool, ok bool) { if r.applicationError == nil { @@ -66,3 +72,7 @@ func (r *CloseReason) StatelessReset() (token StatelessResetToken, ok bool) { } return *r.statelessResetToken, true } + +func (r *CloseReason) VersionNegotiation() (versions []VersionNumber, ok bool) { + return r.versions, len(r.versions) > 0 +} diff --git a/logging/close_reason_test.go b/logging/close_reason_test.go index 80cef958f..6fc352f0f 100644 --- a/logging/close_reason_test.go +++ b/logging/close_reason_test.go @@ -26,6 +26,11 @@ var _ = Describe("Close Reason", func() { ExpectWithOffset(1, ok).To(BeFalse()) } + checkNotVN := func(r CloseReason) { + _, ok := r.VersionNegotiation() + ExpectWithOffset(1, ok).To(BeFalse()) + } + It("application errors", func() { r := NewApplicationCloseReason(1337, true) errorCode, remote, ok := r.ApplicationError() @@ -35,6 +40,7 @@ var _ = Describe("Close Reason", func() { checkNotTransportError(r) checkNotStatelessReset(r) checkNotTimeout(r) + checkNotVN(r) }) It("transport errors", func() { @@ -46,6 +52,7 @@ var _ = Describe("Close Reason", func() { checkNotApplicationError(r) checkNotStatelessReset(r) checkNotTimeout(r) + checkNotVN(r) }) It("transport errors", func() { @@ -55,7 +62,7 @@ var _ = Describe("Close Reason", func() { Expect(timeout).To(Equal(TimeoutReasonIdle)) checkNotApplicationError(r) checkNotTransportError(r) - checkNotStatelessReset(r) + checkNotVN(r) }) It("stateless resets", func() { @@ -66,5 +73,17 @@ var _ = Describe("Close Reason", func() { checkNotApplicationError(r) checkNotTransportError(r) checkNotTimeout(r) + checkNotVN(r) + }) + + It("version negotiation errors", func() { + r := NewVersionNegotiationError([]VersionNumber{1, 2, 3}) + vn, ok := r.VersionNegotiation() + Expect(ok).To(BeTrue()) + Expect(vn).To(Equal([]VersionNumber{1, 2, 3})) + checkNotApplicationError(r) + checkNotTransportError(r) + checkNotTimeout(r) + checkNotStatelessReset(r) }) }) diff --git a/qlog/event.go b/qlog/event.go index 9694bb7ed..c2251c41a 100644 --- a/qlog/event.go +++ b/qlog/event.go @@ -96,7 +96,6 @@ func (e eventConnectionClosed) Name() string { return "connection_closed" func (e eventConnectionClosed) IsNil() bool { return false } func (e eventConnectionClosed) MarshalJSONObject(enc *gojay.Encoder) { - // TODO: add version mismatch if token, ok := e.Reason.StatelessReset(); ok { enc.StringKey("owner", ownerRemote.String()) enc.StringKey("trigger", "stateless_reset") @@ -124,6 +123,10 @@ func (e eventConnectionClosed) MarshalJSONObject(enc *gojay.Encoder) { enc.StringKey("owner", owner.String()) enc.StringKey("connection_code", transportError(code).String()) } + if _, ok := e.Reason.VersionNegotiation(); ok { + enc.StringKey("owner", ownerRemote.String()) + enc.StringKey("trigger", "version_negotiation") + } } type eventPacketSent struct { diff --git a/qlog/qlog_test.go b/qlog/qlog_test.go index fa7ff601a..cf9329bf6 100644 --- a/qlog/qlog_test.go +++ b/qlog/qlog_test.go @@ -205,6 +205,17 @@ var _ = Describe("Tracing", func() { Expect(ev).To(HaveKeyWithValue("stateless_reset_token", "00112233445566778899aabbccddeeff")) }) + It("records connection closing due to version negotiation failure", func() { + tracer.ClosedConnection(logging.NewVersionNegotiationError([]logging.VersionNumber{1, 2, 3})) + entry := exportAndParseSingle() + Expect(entry.Time).To(BeTemporally("~", time.Now(), scaleDuration(10*time.Millisecond))) + Expect(entry.Name).To(Equal("transport:connection_closed")) + ev := entry.Event + Expect(ev).To(HaveLen(2)) + Expect(ev).To(HaveKeyWithValue("owner", "remote")) + Expect(ev).To(HaveKeyWithValue("trigger", "version_negotiation")) + }) + It("records application errors", func() { tracer.ClosedConnection(logging.NewApplicationCloseReason(1337, true)) entry := exportAndParseSingle() diff --git a/session.go b/session.go index 2a75fd559..e3c703594 100644 --- a/session.go +++ b/session.go @@ -133,6 +133,15 @@ func (errCloseForRecreating) Is(target error) bool { return ok } +type errVersionNegotiation struct { + ourVersions []protocol.VersionNumber + theirVersions []protocol.VersionNumber +} + +func (e errVersionNegotiation) Error() string { + return fmt.Sprintf("no compatible QUIC version found (we support %s, server offered %s)", e.ourVersions, e.theirVersions) +} + // A Session is a QUIC session type session struct { // Destination connection ID used during the handshake. @@ -1074,8 +1083,10 @@ func (s *session) handleVersionNegotiationPacket(p *receivedPacket) { } newVersion, ok := protocol.ChooseSupportedVersion(s.config.Versions, supportedVersions) if !ok { - //nolint:stylecheck - s.destroyImpl(fmt.Errorf("No compatible QUIC version found. We support %s, server offered %s.", s.config.Versions, supportedVersions)) + s.destroyImpl(errVersionNegotiation{ + ourVersions: s.config.Versions, + theirVersions: supportedVersions, + }) s.logger.Infof("No compatible QUIC version found.") return } @@ -1438,8 +1449,11 @@ func (s *session) handleCloseError(closeErr closeError) { // timeout errors are logged as soon as they occur (to distinguish between handshake and idle timeouts) if nerr, ok := closeErr.err.(net.Error); !ok || !nerr.Timeout() { var resetErr statelessResetErr + var vnErr errVersionNegotiation if errors.As(closeErr.err, &resetErr) { s.tracer.ClosedConnection(logging.NewStatelessResetCloseReason(resetErr.token)) + } else if errors.As(closeErr.err, &vnErr) { + s.tracer.ClosedConnection(logging.NewVersionNegotiationError(vnErr.theirVersions)) } else if quicErr.IsApplicationError() { s.tracer.ClosedConnection(logging.NewApplicationCloseReason(quicErr.ErrorCode, closeErr.remote)) } else { diff --git a/session_test.go b/session_test.go index b8772c0d6..31681ca7d 100644 --- a/session_test.go +++ b/session_test.go @@ -2628,9 +2628,12 @@ var _ = Describe("Client Session", func() { errChan <- sess.run() }() sessionRunner.EXPECT().Remove(srcConnID).MaxTimes(1) + var closeReason logging.CloseReason gomock.InOrder( tracer.EXPECT().ReceivedVersionNegotiationPacket(gomock.Any(), gomock.Any()), - tracer.EXPECT().ClosedConnection(gomock.Any()), + tracer.EXPECT().ClosedConnection(gomock.Any()).Do(func(r logging.CloseReason) { + closeReason = r + }), tracer.EXPECT().Close(), ) cryptoSetup.EXPECT().Close() @@ -2639,7 +2642,10 @@ var _ = Describe("Client Session", func() { Eventually(errChan).Should(Receive(&err)) Expect(err).To(HaveOccurred()) Expect(err).ToNot(BeAssignableToTypeOf(&errCloseForRecreating{})) - Expect(err.Error()).To(ContainSubstring("No compatible QUIC version found")) + Expect(err.Error()).To(ContainSubstring("no compatible QUIC version found")) + vns, ok := closeReason.VersionNegotiation() + Expect(ok).To(BeTrue()) + Expect(vns).To(ContainElement(logging.VersionNumber(12345678))) }) It("ignores Version Negotiation packets that offer the current version", func() {