forked from quic-go/quic-go
send initial version in the CHLO, not the current version
This commit fixes two bugs: 1. In the CHLO, we need to send the initial QUIC version. It will differ from the current version if version negotiation was performed. 2. The session setup was using the wrong version (current version, and not the initial version), such that we would have sent the wrong version in the TLS handshake as well.
This commit is contained in:
@@ -353,6 +353,7 @@ func (c *client) handleVersionNegotiationPacket(hdr *wire.Header) error {
|
||||
|
||||
func (c *client) createNewSession(initialVersion protocol.VersionNumber, negotiatedVersions []protocol.VersionNumber) error {
|
||||
var err error
|
||||
utils.Debugf("createNewSession with initial version %s", initialVersion)
|
||||
c.session, c.handshakeChan, err = newClientSession(
|
||||
c.conn,
|
||||
c.hostname,
|
||||
|
||||
@@ -23,6 +23,7 @@ type cryptoSetupClient struct {
|
||||
hostname string
|
||||
connID protocol.ConnectionID
|
||||
version protocol.VersionNumber
|
||||
initialVersion protocol.VersionNumber
|
||||
negotiatedVersions []protocol.VersionNumber
|
||||
|
||||
cryptoStream io.ReadWriter
|
||||
@@ -74,6 +75,7 @@ func NewCryptoSetupClient(
|
||||
params *TransportParameters,
|
||||
paramsChan chan<- TransportParameters,
|
||||
aeadChanged chan<- protocol.EncryptionLevel,
|
||||
initialVersion protocol.VersionNumber,
|
||||
negotiatedVersions []protocol.VersionNumber,
|
||||
) (CryptoSetup, error) {
|
||||
nullAEAD, err := crypto.NewNullAEAD(protocol.PerspectiveClient, connID, version)
|
||||
@@ -92,6 +94,7 @@ func NewCryptoSetupClient(
|
||||
nullAEAD: nullAEAD,
|
||||
paramsChan: paramsChan,
|
||||
aeadChanged: aeadChanged,
|
||||
initialVersion: initialVersion,
|
||||
negotiatedVersions: negotiatedVersions,
|
||||
divNonceChan: make(chan []byte),
|
||||
}, nil
|
||||
@@ -423,7 +426,7 @@ func (h *cryptoSetupClient) getTags() (map[Tag][]byte, error) {
|
||||
}
|
||||
|
||||
versionTag := make([]byte, 4)
|
||||
binary.BigEndian.PutUint32(versionTag, uint32(h.version))
|
||||
binary.BigEndian.PutUint32(versionTag, uint32(h.initialVersion))
|
||||
tags[TagVER] = versionTag
|
||||
|
||||
if len(h.stk) > 0 {
|
||||
|
||||
@@ -105,7 +105,7 @@ var _ = Describe("Client Crypto Setup", func() {
|
||||
|
||||
stream = newMockStream()
|
||||
certManager = &mockCertManager{}
|
||||
version := protocol.Version37
|
||||
version := protocol.Version39
|
||||
// use a buffered channel here, so that we can parse a SHLO without having to receive the TransportParameters to avoid blocking
|
||||
paramsChan = make(chan TransportParameters, 1)
|
||||
aeadChanged = make(chan protocol.EncryptionLevel, 2)
|
||||
@@ -118,6 +118,7 @@ var _ = Describe("Client Crypto Setup", func() {
|
||||
&TransportParameters{IdleTimeout: protocol.DefaultIdleTimeout},
|
||||
paramsChan,
|
||||
aeadChanged,
|
||||
protocol.Version37,
|
||||
nil,
|
||||
)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
@@ -490,6 +491,7 @@ var _ = Describe("Client Crypto Setup", func() {
|
||||
})
|
||||
|
||||
It("has the right values for an inchoate CHLO", func() {
|
||||
Expect(cs.version).ToNot(Equal(cs.initialVersion)) // make sure we can test that TagVER actually has the initial version, and not the current version
|
||||
cs.hostname = "sni-hostname"
|
||||
certManager.commonCertificateHashes = []byte("common certs")
|
||||
tags, err := cs.getTags()
|
||||
|
||||
@@ -148,7 +148,7 @@ var newClientSession = func(
|
||||
connectionID protocol.ConnectionID,
|
||||
tlsConf *tls.Config,
|
||||
config *Config,
|
||||
initialVersion protocol.VersionNumber, // needed for validation of the version negotaion over TLS
|
||||
initialVersion protocol.VersionNumber,
|
||||
negotiatedVersions []protocol.VersionNumber, // needed for validation of the GQUIC version negotiaton
|
||||
) (packetHandler, <-chan handshakeEvent, error) {
|
||||
s := &session{
|
||||
@@ -158,7 +158,7 @@ var newClientSession = func(
|
||||
version: v,
|
||||
config: config,
|
||||
}
|
||||
return s.setup(nil, hostname, tlsConf, v, negotiatedVersions)
|
||||
return s.setup(nil, hostname, tlsConf, initialVersion, negotiatedVersions)
|
||||
}
|
||||
|
||||
func (s *session) setup(
|
||||
@@ -261,6 +261,7 @@ func (s *session) setup(
|
||||
transportParams,
|
||||
paramsChan,
|
||||
aeadChanged,
|
||||
initialVersion,
|
||||
negotiatedVersions,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -1522,6 +1522,7 @@ var _ = Describe("Client Session", func() {
|
||||
_ *handshake.TransportParameters,
|
||||
_ chan<- handshake.TransportParameters,
|
||||
aeadChangedP chan<- protocol.EncryptionLevel,
|
||||
_ protocol.VersionNumber,
|
||||
_ []protocol.VersionNumber,
|
||||
) (handshake.CryptoSetup, error) {
|
||||
aeadChanged = aeadChangedP
|
||||
|
||||
Reference in New Issue
Block a user