Replace byteBuilder and byteReader with cryptobyte

While I'm here, update x/crypto and x/net to their latest versions.
byteReader is a straightforward port, except there doesn't seem to be a
convenient way to read length-prefixed bytes without manually casting
from cryptobyte.String to []byte, so I've done that.

byteBuilder is a bit more involved because it's based on closures, but
still a mechanical change.

As part of this, I switched runner's ticket format to use u24 length
prefixes instead of u32, because cryptobyte.String doesn't have u32
length prefixes. (Although, oddly, cryptobyte.Builder does.)

Fixed: 374
Change-Id: If9bea0b41fe2b8bc48f040a667753b160da469bb
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/61186
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>

go.mod

@@ -3,11 +3,11 @@ module boringssl.googlesource.com/boringssl
 go 1.19
 
 require (
-	golang.org/x/crypto v0.6.0
+	golang.org/x/crypto v0.10.0
-	golang.org/x/net v0.7.0
+	golang.org/x/net v0.11.0
 )
 
 require (
-	golang.org/x/sys v0.5.0 // indirect
+	golang.org/x/sys v0.9.0 // indirect
-	golang.org/x/term v0.5.0 // indirect
+	golang.org/x/term v0.9.0 // indirect
 )

go.sum

@@ -1,8 +1,8 @@
-golang.org/x/crypto v0.6.0 h1:qfktjS5LUO+fFKeJXZ+ikTRijMmljikvG68fpMMruSc=
+golang.org/x/crypto v0.10.0 h1:LKqV2xt9+kDzSTfOhx4FrkEBcMrAgHSYgzywV9zcGmM=
-golang.org/x/crypto v0.6.0/go.mod h1:OFC/31mSvZgRz0V1QTNCzfAI1aIRzbiufJtkMIlEp58=
+golang.org/x/crypto v0.10.0/go.mod h1:o4eNf7Ede1fv+hwOwZsTHl9EsPFO6q6ZvYR8vYfY45I=
-golang.org/x/net v0.7.0 h1:rJrUqqhjsgNp7KqAIc25s9pZnjU7TUcSY7HcVZjdn1g=
+golang.org/x/net v0.11.0 h1:Gi2tvZIJyBtO9SDr1q9h5hEQCp/4L2RQ+ar0qjx2oNU=
-golang.org/x/net v0.7.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs=
+golang.org/x/net v0.11.0/go.mod h1:2L/ixqYpgIVXmeoSA/4Lu7BzTG4KIyPIryS4IsOd1oQ=
-golang.org/x/sys v0.5.0 h1:MUK/U/4lj1t1oPg0HfuXDN/Z1wv31ZJ/YcPiGccS4DU=
+golang.org/x/sys v0.9.0 h1:KS/R3tvhPqvJvwcKfnBHJwwthS11LRhmM5D59eEXa0s=
-golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.9.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/term v0.5.0 h1:n2a8QNdAb0sZNpU9R1ALUXBbY+w51fCQDN+7EdxNBsY=
+golang.org/x/term v0.9.0 h1:GRRCnKYhdQrD8kfRAdQ6Zcw1P0OcELxGLKJvtjVMZ28=
-golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k=
+golang.org/x/term v0.9.0/go.mod h1:M6DEAAIenWoTxdKrOltXcmDY3rSplQUkrvaDU5FcQyo=

ssl/test/runner/handshake_client.go

@@ -21,6 +21,7 @@ import (
 	"time"
 
 	"boringssl.googlesource.com/boringssl/ssl/test/runner/hpke"
+	"golang.org/x/crypto/cryptobyte"
 )
 
 const echBadPayloadByte = 0xff
@@ -71,9 +72,12 @@ func replaceClientHello(hello *clientHelloMsg, in []byte) (*clientHelloMsg, erro
 	// Replace |newHellos|'s key shares with those of |hello|. For simplicity,
 	// we require their lengths match, which is satisfied by matching the
 	// DefaultCurves setting to the selection in the replacement ClientHello.
-	bb := newByteBuilder()
+	bb := cryptobyte.NewBuilder(nil)
 	hello.marshalKeyShares(bb)
-	keyShares := bb.finish()
+	keyShares, err := bb.Bytes()
+	if err != nil {
+		return nil, err
+	}
 	if len(keyShares) != len(newHello.keySharesRaw) {
 		return nil, errors.New("tls: ClientHello key share length is inconsistent with DefaultCurves setting")
 	}

ssl/test/runner/handshake_server.go

@@ -20,6 +20,7 @@ import (
 	"time"
 
 	"boringssl.googlesource.com/boringssl/ssl/test/runner/hpke"
+	"golang.org/x/crypto/cryptobyte"
 )
 
 // serverHandshakeState contains details of a server handshake in progress.
@@ -2443,18 +2444,18 @@ func checkClientHellosEqual(a, b []byte, isDTLS bool, ignoreExtensions []uint16)
 	}
 
 	// Skip the handshake message header.
-	aReader := byteReader(a[4:])
+	aReader := cryptobyte.String(a[4:])
-	bReader := byteReader(b[4:])
+	bReader := cryptobyte.String(b[4:])
 
 	var aVers, bVers uint16
 	var aRandom, bRandom []byte
 	var aSessionID, bSessionID []byte
-	if !aReader.readU16(&aVers) ||
+	if !aReader.ReadUint16(&aVers) ||
-		!bReader.readU16(&bVers) ||
+		!bReader.ReadUint16(&bVers) ||
-		!aReader.readBytes(&aRandom, 32) ||
+		!aReader.ReadBytes(&aRandom, 32) ||
-		!bReader.readBytes(&bRandom, 32) ||
+		!bReader.ReadBytes(&bRandom, 32) ||
-		!aReader.readU8LengthPrefixedBytes(&aSessionID) ||
+		!readUint8LengthPrefixedBytes(&aReader, &aSessionID) ||
-		!bReader.readU8LengthPrefixedBytes(&bSessionID) {
+		!readUint8LengthPrefixedBytes(&bReader, &bSessionID) {
 		return errors.New("tls: could not parse ClientHello")
 	}
 
@@ -2474,17 +2475,17 @@ func checkClientHellosEqual(a, b []byte, isDTLS bool, ignoreExtensions []uint16)
 		// cookie altogether. If we implement DTLS 1.3, we'll need to ensure
 		// that parsing logic above this function rejects this cookie.
 		var aCookie, bCookie []byte
-		if !aReader.readU8LengthPrefixedBytes(&aCookie) ||
+		if !readUint8LengthPrefixedBytes(&aReader, &aCookie) ||
-			!bReader.readU8LengthPrefixedBytes(&bCookie) {
+			!readUint8LengthPrefixedBytes(&bReader, &bCookie) {
 			return errors.New("tls: could not parse ClientHello")
 		}
 	}
 
 	var aCipherSuites, bCipherSuites, aCompressionMethods, bCompressionMethods []byte
-	if !aReader.readU16LengthPrefixedBytes(&aCipherSuites) ||
+	if !readUint16LengthPrefixedBytes(&aReader, &aCipherSuites) ||
-		!bReader.readU16LengthPrefixedBytes(&bCipherSuites) ||
+		!readUint16LengthPrefixedBytes(&bReader, &bCipherSuites) ||
-		!aReader.readU8LengthPrefixedBytes(&aCompressionMethods) ||
+		!readUint8LengthPrefixedBytes(&aReader, &aCompressionMethods) ||
-		!bReader.readU8LengthPrefixedBytes(&bCompressionMethods) {
+		!readUint8LengthPrefixedBytes(&bReader, &bCompressionMethods) {
 		return errors.New("tls: could not parse ClientHello")
 	}
 	if !bytes.Equal(aCipherSuites, bCipherSuites) {
@@ -2499,9 +2500,9 @@ func checkClientHellosEqual(a, b []byte, isDTLS bool, ignoreExtensions []uint16)
 		return nil
 	}
 
-	var aExtensions, bExtensions byteReader
+	var aExtensions, bExtensions cryptobyte.String
-	if !aReader.readU16LengthPrefixed(&aExtensions) ||
+	if !aReader.ReadUint16LengthPrefixed(&aExtensions) ||
-		!bReader.readU16LengthPrefixed(&bExtensions) ||
+		!bReader.ReadUint16LengthPrefixed(&bExtensions) ||
 		len(aReader) != 0 ||
 		len(bReader) != 0 {
 		return errors.New("tls: could not parse ClientHello")
@@ -2510,8 +2511,8 @@ func checkClientHellosEqual(a, b []byte, isDTLS bool, ignoreExtensions []uint16)
 	for len(aExtensions) != 0 {
 		var aID uint16
 		var aBody []byte
-		if !aExtensions.readU16(&aID) ||
+		if !aExtensions.ReadUint16(&aID) ||
-			!aExtensions.readU16LengthPrefixedBytes(&aBody) {
+			!readUint16LengthPrefixedBytes(&aExtensions, &aBody) {
 			return errors.New("tls: could not parse ClientHello")
 		}
 		if _, ok := ignoreExtensionsSet[aID]; ok {
@@ -2524,8 +2525,8 @@ func checkClientHellosEqual(a, b []byte, isDTLS bool, ignoreExtensions []uint16)
 			}
 			var bID uint16
 			var bBody []byte
-			if !bExtensions.readU16(&bID) ||
+			if !bExtensions.ReadUint16(&bID) ||
-				!bExtensions.readU16LengthPrefixedBytes(&bBody) {
+				!readUint16LengthPrefixedBytes(&bExtensions, &bBody) {
 				return errors.New("tls: could not parse ClientHello")
 			}
 			if _, ok := ignoreExtensionsSet[bID]; ok {
@@ -2546,8 +2547,8 @@ func checkClientHellosEqual(a, b []byte, isDTLS bool, ignoreExtensions []uint16)
 	for len(bExtensions) != 0 {
 		var id uint16
 		var body []byte
-		if !bExtensions.readU16(&id) ||
+		if !bExtensions.ReadUint16(&id) ||
-			!bExtensions.readU16LengthPrefixedBytes(&body) {
+			!readUint16LengthPrefixedBytes(&bExtensions, &body) {
 			return errors.New("tls: could not parse ClientHello")
 		}
 		if _, ok := ignoreExtensionsSet[id]; !ok {

ssl/test/runner/prf.go

@@ -13,6 +13,7 @@ import (
 	"encoding"
 	"hash"
 
+	"golang.org/x/crypto/cryptobyte"
 	"golang.org/x/crypto/hkdf"
 )
 
@@ -228,15 +229,15 @@ type finishedHash struct {
 }
 
 func (h *finishedHash) UpdateForHelloRetryRequest() {
-	data := newByteBuilder()
+	data := cryptobyte.NewBuilder(nil)
-	data.addU8(typeMessageHash)
+	data.AddUint8(typeMessageHash)
-	data.addU24(h.hash.Size())
+	data.AddUint24(uint32(h.hash.Size()))
-	data.addBytes(h.Sum())
+	data.AddBytes(h.Sum())
 	h.hash = h.suite.hash().New()
 	if h.buffer != nil {
 		h.buffer = []byte{}
 	}
-	h.Write(data.finish())
+	h.Write(data.BytesOrPanic())
 }
 
 func (h *finishedHash) Write(msg []byte) (n int, err error) {

ssl/test/runner/runner.go

@@ -47,6 +47,7 @@ import (
 
 	"boringssl.googlesource.com/boringssl/ssl/test/runner/hpke"
 	"boringssl.googlesource.com/boringssl/util/testresult"
+	"golang.org/x/crypto/cryptobyte"
 )
 
 var (
@@ -819,10 +820,10 @@ func doExchange(test *testCase, config *Config, conn net.Conn, isResume bool, tr
 				if err := os.MkdirAll(dir, 0755); err != nil {
 					return err
 				}
-				bb := newByteBuilder()
+				bb := cryptobyte.NewBuilder(nil)
-				bb.addU24LengthPrefixed().addBytes(encodedInner)
+				addUint24LengthPrefixedBytes(bb, encodedInner)
-				bb.addBytes(outer)
+				bb.AddBytes(outer)
-				return os.WriteFile(filepath.Join(dir, name), bb.finish(), 0644)
+				return os.WriteFile(filepath.Join(dir, name), bb.BytesOrPanic(), 0644)
 			}
 		}
 

ssl/test/runner/ticket.go

@@ -13,6 +13,8 @@ import (
 	"errors"
 	"io"
 	"time"
+
+	"golang.org/x/crypto/cryptobyte"
 )
 
 // sessionState contains the information that is serialized into a session
@@ -35,49 +37,45 @@ type sessionState struct {
 }
 
 func (s *sessionState) marshal() []byte {
-	msg := newByteBuilder()
+	msg := cryptobyte.NewBuilder(nil)
-	msg.addU16(s.vers)
+	msg.AddUint16(s.vers)
-	msg.addU16(s.cipherSuite)
+	msg.AddUint16(s.cipherSuite)
-	secret := msg.addU16LengthPrefixed()
+	addUint16LengthPrefixedBytes(msg, s.secret)
-	secret.addBytes(s.secret)
+	addUint16LengthPrefixedBytes(msg, s.handshakeHash)
-	handshakeHash := msg.addU16LengthPrefixed()
+	msg.AddUint16(uint16(len(s.certificates)))
-	handshakeHash.addBytes(s.handshakeHash)
-	msg.addU16(uint16(len(s.certificates)))
 	for _, cert := range s.certificates {
-		certMsg := msg.addU32LengthPrefixed()
+		addUint24LengthPrefixedBytes(msg, cert)
-		certMsg.addBytes(cert)
 	}
 
 	if s.extendedMasterSecret {
-		msg.addU8(1)
+		msg.AddUint8(1)
 	} else {
-		msg.addU8(0)
+		msg.AddUint8(0)
 	}
 
 	if s.vers >= VersionTLS13 {
-		msg.addU64(uint64(s.ticketCreationTime.UnixNano()))
+		msg.AddUint64(uint64(s.ticketCreationTime.UnixNano()))
-		msg.addU64(uint64(s.ticketExpiration.UnixNano()))
+		msg.AddUint64(uint64(s.ticketExpiration.UnixNano()))
-		msg.addU32(s.ticketFlags)
+		msg.AddUint32(s.ticketFlags)
-		msg.addU32(s.ticketAgeAdd)
+		msg.AddUint32(s.ticketAgeAdd)
 	}
 
-	earlyALPN := msg.addU16LengthPrefixed()
+	addUint16LengthPrefixedBytes(msg, s.earlyALPN)
-	earlyALPN.addBytes(s.earlyALPN)
 
 	if s.hasApplicationSettings {
-		msg.addU8(1)
+		msg.AddUint8(1)
-		msg.addU16LengthPrefixed().addBytes(s.localApplicationSettings)
+		addUint16LengthPrefixedBytes(msg, s.localApplicationSettings)
-		msg.addU16LengthPrefixed().addBytes(s.peerApplicationSettings)
+		addUint16LengthPrefixedBytes(msg, s.peerApplicationSettings)
 	} else {
-		msg.addU8(0)
+		msg.AddUint8(0)
 	}
 
-	return msg.finish()
+	return msg.BytesOrPanic()
 }
 
-func readBool(reader *byteReader, out *bool) bool {
+func readBool(reader *cryptobyte.String, out *bool) bool {
 	var value uint8
-	if !reader.readU8(&value) {
+	if !reader.ReadUint8(&value) {
 		return false
 	}
 	if value == 0 {
@@ -92,19 +90,19 @@ func readBool(reader *byteReader, out *bool) bool {
 }
 
 func (s *sessionState) unmarshal(data []byte) bool {
-	reader := byteReader(data)
+	reader := cryptobyte.String(data)
 	var numCerts uint16
-	if !reader.readU16(&s.vers) ||
+	if !reader.ReadUint16(&s.vers) ||
-		!reader.readU16(&s.cipherSuite) ||
+		!reader.ReadUint16(&s.cipherSuite) ||
-		!reader.readU16LengthPrefixedBytes(&s.secret) ||
+		!readUint16LengthPrefixedBytes(&reader, &s.secret) ||
-		!reader.readU16LengthPrefixedBytes(&s.handshakeHash) ||
+		!readUint16LengthPrefixedBytes(&reader, &s.handshakeHash) ||
-		!reader.readU16(&numCerts) {
+		!reader.ReadUint16(&numCerts) {
 		return false
 	}
 
 	s.certificates = make([][]byte, int(numCerts))
 	for i := range s.certificates {
-		if !reader.readU32LengthPrefixedBytes(&s.certificates[i]) {
+		if !readUint24LengthPrefixedBytes(&reader, &s.certificates[i]) {
 			return false
 		}
 	}
@@ -115,24 +113,24 @@ func (s *sessionState) unmarshal(data []byte) bool {
 
 	if s.vers >= VersionTLS13 {
 		var ticketCreationTime, ticketExpiration uint64
-		if !reader.readU64(&ticketCreationTime) ||
+		if !reader.ReadUint64(&ticketCreationTime) ||
-			!reader.readU64(&ticketExpiration) ||
+			!reader.ReadUint64(&ticketExpiration) ||
-			!reader.readU32(&s.ticketFlags) ||
+			!reader.ReadUint32(&s.ticketFlags) ||
-			!reader.readU32(&s.ticketAgeAdd) {
+			!reader.ReadUint32(&s.ticketAgeAdd) {
 			return false
 		}
 		s.ticketCreationTime = time.Unix(0, int64(ticketCreationTime))
 		s.ticketExpiration = time.Unix(0, int64(ticketExpiration))
 	}
 
-	if !reader.readU16LengthPrefixedBytes(&s.earlyALPN) ||
+	if !readUint16LengthPrefixedBytes(&reader, &s.earlyALPN) ||
 		!readBool(&reader, &s.hasApplicationSettings) {
 		return false
 	}
 
 	if s.hasApplicationSettings {
-		if !reader.readU16LengthPrefixedBytes(&s.localApplicationSettings) ||
+		if !readUint16LengthPrefixedBytes(&reader, &s.localApplicationSettings) ||
-			!reader.readU16LengthPrefixedBytes(&s.peerApplicationSettings) {
+			!readUint16LengthPrefixedBytes(&reader, &s.peerApplicationSettings) {
 			return false
 		}
 	}