introduce a logging.CloseReason for version negotiation errors

This commit is contained in:
Marten Seemann 2021-03-15 13:20:23 +08:00
parent 3bce408c8d
commit 5208845191
5 changed files with 55 additions and 6 deletions

View file

@ -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

View file

@ -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
}

View file

@ -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)
})
})

View file

@ -123,6 +123,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.
@ -1061,8 +1070,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
}
@ -1425,8 +1436,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 {

View file

@ -2630,9 +2630,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()
@ -2641,7 +2644,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() {