Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

check transport parameters after 0-RTT resumption #3985

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -1665,6 +1665,14 @@ func (s *connection) handleTransportParameters(params *wire.TransportParameters)
ErrorMessage: err.Error(),
}
}

if s.perspective == protocol.PerspectiveClient && s.peerParams != nil && s.ConnectionState().Used0RTT && !params.ValidForUpdate(s.peerParams) {
return &qerr.TransportError{
ErrorCode: qerr.ProtocolViolation,
ErrorMessage: "server sent reduced limits after accepting 0-RTT data",
}
}

s.peerParams = params
// On the client side we have to wait for handshake completion.
// During a 0-RTT connection, we are only allowed to use the new transport parameters for 1-RTT packets.
Expand Down
34 changes: 34 additions & 0 deletions connection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3035,6 +3035,40 @@ var _ = Describe("Client Connection", func() {
ErrorMessage: "expected original_destination_connection_id to equal deadbeef, is decafbad",
})))
})

It("errors if the transport parameters contain reduced limits after knowing 0-RTT data is accepted by the server", func() {
conn.perspective = protocol.PerspectiveClient
conn.peerParams = &wire.TransportParameters{
ActiveConnectionIDLimit: 3,
InitialMaxData: 0x5000,
InitialMaxStreamDataBidiLocal: 0x5000,
InitialMaxStreamDataBidiRemote: 1000,
InitialMaxStreamDataUni: 1000,
MaxBidiStreamNum: 500,
MaxUniStreamNum: 500,
}
params := &wire.TransportParameters{
OriginalDestinationConnectionID: destConnID,
InitialSourceConnectionID: destConnID,
ActiveConnectionIDLimit: 3,
InitialMaxData: 0x5000,
InitialMaxStreamDataBidiLocal: 0x5000,
InitialMaxStreamDataBidiRemote: 1000,
InitialMaxStreamDataUni: 1000,
MaxBidiStreamNum: 300,
MaxUniStreamNum: 300,
}
expectClose(false, true)
processed := make(chan struct{})
tracer.EXPECT().ReceivedTransportParameters(params).Do(func(*wire.TransportParameters) { close(processed) })
cryptoSetup.EXPECT().ConnectionState().Return(handshake.ConnectionState{Used0RTT: true})
paramsChan <- params
Eventually(processed).Should(BeClosed())
Eventually(errChan).Should(Receive(MatchError(&qerr.TransportError{
ErrorCode: qerr.ProtocolViolation,
ErrorMessage: "server sent reduced limits after accepting 0-RTT data",
})))
})
})

Context("handling potentially injected packets", func() {
Expand Down
88 changes: 88 additions & 0 deletions internal/wire/transport_parameter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -612,5 +612,93 @@ var _ = Describe("Transport Parameters", func() {
Expect(p.ValidFor0RTT(saved)).To(BeFalse())
})
})

Context("client checks the parameters after successfully sending 0-RTT data", func() {
var p TransportParameters
saved := &TransportParameters{
InitialMaxStreamDataBidiLocal: 1,
InitialMaxStreamDataBidiRemote: 2,
InitialMaxStreamDataUni: 3,
InitialMaxData: 4,
MaxBidiStreamNum: 5,
MaxUniStreamNum: 6,
ActiveConnectionIDLimit: 7,
}

BeforeEach(func() {
p = *saved
Expect(p.ValidForUpdate(saved)).To(BeTrue())
})

It("rejects the parameters if the InitialMaxStreamDataBidiLocal was reduced", func() {
p.InitialMaxStreamDataBidiLocal = saved.InitialMaxStreamDataBidiLocal - 1
Expect(p.ValidForUpdate(saved)).To(BeFalse())
})

It("doesn't reject the parameters if the InitialMaxStreamDataBidiLocal was increased", func() {
p.InitialMaxStreamDataBidiLocal = saved.InitialMaxStreamDataBidiLocal + 1
Expect(p.ValidForUpdate(saved)).To(BeTrue())
})

It("rejects the parameters if the InitialMaxStreamDataBidiRemote was reduced", func() {
p.InitialMaxStreamDataBidiRemote = saved.InitialMaxStreamDataBidiRemote - 1
Expect(p.ValidForUpdate(saved)).To(BeFalse())
})

It("doesn't reject the parameters if the InitialMaxStreamDataBidiRemote was increased", func() {
p.InitialMaxStreamDataBidiRemote = saved.InitialMaxStreamDataBidiRemote + 1
Expect(p.ValidForUpdate(saved)).To(BeTrue())
})

It("rejects the parameters if the InitialMaxStreamDataUni was reduced", func() {
p.InitialMaxStreamDataUni = saved.InitialMaxStreamDataUni - 1
Expect(p.ValidForUpdate(saved)).To(BeFalse())
})

It("doesn't reject the parameters if the InitialMaxStreamDataUni was increased", func() {
p.InitialMaxStreamDataUni = saved.InitialMaxStreamDataUni + 1
Expect(p.ValidForUpdate(saved)).To(BeTrue())
})

It("rejects the parameters if the InitialMaxData was reduced", func() {
p.InitialMaxData = saved.InitialMaxData - 1
Expect(p.ValidForUpdate(saved)).To(BeFalse())
})

It("doesn't reject the parameters if the InitialMaxData was increased", func() {
p.InitialMaxData = saved.InitialMaxData + 1
Expect(p.ValidForUpdate(saved)).To(BeTrue())
})

It("rejects the parameters if the MaxBidiStreamNum was reduced", func() {
p.MaxBidiStreamNum = saved.MaxBidiStreamNum - 1
Expect(p.ValidForUpdate(saved)).To(BeFalse())
})

It("doesn't reject the parameters if the MaxBidiStreamNum was increased", func() {
p.MaxBidiStreamNum = saved.MaxBidiStreamNum + 1
Expect(p.ValidForUpdate(saved)).To(BeTrue())
})

It("rejects the parameters if the MaxUniStreamNum reduced", func() {
p.MaxUniStreamNum = saved.MaxUniStreamNum - 1
Expect(p.ValidForUpdate(saved)).To(BeFalse())
})

It("doesn't reject the parameters if the MaxUniStreamNum was increased", func() {
p.MaxUniStreamNum = saved.MaxUniStreamNum + 1
Expect(p.ValidForUpdate(saved)).To(BeTrue())
})

It("rejects the parameters if the ActiveConnectionIDLimit reduced", func() {
p.ActiveConnectionIDLimit = saved.ActiveConnectionIDLimit - 1
Expect(p.ValidForUpdate(saved)).To(BeFalse())
})

It("doesn't reject the parameters if the ActiveConnectionIDLimit increased", func() {
p.ActiveConnectionIDLimit = saved.ActiveConnectionIDLimit + 1
Expect(p.ValidForUpdate(saved)).To(BeTrue())
})
})
})
})
12 changes: 12 additions & 0 deletions internal/wire/transport_parameters.go
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,18 @@ func (p *TransportParameters) ValidFor0RTT(saved *TransportParameters) bool {
p.ActiveConnectionIDLimit == saved.ActiveConnectionIDLimit
}

// ValidForUpdate checks that the new transport parameters don't reduce limits after resuming a 0-RTT connection.
// It is only used on the client side.
func (p *TransportParameters) ValidForUpdate(saved *TransportParameters) bool {
return p.ActiveConnectionIDLimit >= saved.ActiveConnectionIDLimit &&
p.InitialMaxData >= saved.InitialMaxData &&
p.InitialMaxStreamDataBidiLocal >= saved.InitialMaxStreamDataBidiLocal &&
p.InitialMaxStreamDataBidiRemote >= saved.InitialMaxStreamDataBidiRemote &&
p.InitialMaxStreamDataUni >= saved.InitialMaxStreamDataUni &&
p.MaxBidiStreamNum >= saved.MaxBidiStreamNum &&
p.MaxUniStreamNum >= saved.MaxUniStreamNum
}

// String returns a string representation, intended for logging.
func (p *TransportParameters) String() string {
logString := "&wire.TransportParameters{OriginalDestinationConnectionID: %s, InitialSourceConnectionID: %s, "
Expand Down