Skip to content

Commit

Permalink
check transport parameters after 0-RTT resumption (#3985)
Browse files Browse the repository at this point in the history
* check new transport parameters do not contain redueced limits

* redefine ValidForUpdate and add tests

* fix test assertion and update comment
  • Loading branch information
tanghaowillow committed Aug 2, 2023
1 parent f3a0ce1 commit 1c47ebe
Show file tree
Hide file tree
Showing 4 changed files with 142 additions and 0 deletions.
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

0 comments on commit 1c47ebe

Please sign in to comment.