remove error return value from ComposeVersionNegotiation (#3410)

This commit is contained in:
Chao Fei 2022-05-12 23:03:39 +08:00 committed by GitHub
parent b9a77af8f9
commit 60bbe92194
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 14 additions and 30 deletions

View file

@ -674,8 +674,7 @@ var _ = Describe("Connection", func() {
}) })
It("drops Version Negotiation packets", func() { It("drops Version Negotiation packets", func() {
b, err := wire.ComposeVersionNegotiation(srcConnID, destConnID, conn.config.Versions) b := wire.ComposeVersionNegotiation(srcConnID, destConnID, conn.config.Versions)
Expect(err).ToNot(HaveOccurred())
tracer.EXPECT().DroppedPacket(logging.PacketTypeVersionNegotiation, protocol.ByteCount(len(b)), logging.PacketDropUnexpectedPacket) tracer.EXPECT().DroppedPacket(logging.PacketTypeVersionNegotiation, protocol.ByteCount(len(b)), logging.PacketDropUnexpectedPacket)
Expect(conn.handlePacketImpl(&receivedPacket{ Expect(conn.handlePacketImpl(&receivedPacket{
data: b, data: b,
@ -2592,8 +2591,7 @@ var _ = Describe("Client Connection", func() {
Context("handling Version Negotiation", func() { Context("handling Version Negotiation", func() {
getVNP := func(versions ...protocol.VersionNumber) *receivedPacket { getVNP := func(versions ...protocol.VersionNumber) *receivedPacket {
b, err := wire.ComposeVersionNegotiation(srcConnID, destConnID, versions) b := wire.ComposeVersionNegotiation(srcConnID, destConnID, versions)
Expect(err).ToNot(HaveOccurred())
return &receivedPacket{ return &receivedPacket{
data: b, data: b,
buffer: getPacketBuffer(), buffer: getPacketBuffer(),

View file

@ -24,11 +24,7 @@ func getVNP(src, dest protocol.ConnectionID, numVersions int) []byte {
for i := 0; i < numVersions; i++ { for i := 0; i < numVersions; i++ {
versions[i] = protocol.VersionNumber(rand.Uint32()) versions[i] = protocol.VersionNumber(rand.Uint32())
} }
data, err := wire.ComposeVersionNegotiation(src, dest, versions) return wire.ComposeVersionNegotiation(src, dest, versions)
if err != nil {
log.Fatal(err)
}
return data
} }
func main() { func main() {

View file

@ -91,8 +91,6 @@ func fuzzVNP(data []byte) int {
if len(versions) == 0 { if len(versions) == 0 {
panic("no versions") panic("no versions")
} }
if _, err := wire.ComposeVersionNegotiation(hdr.SrcConnectionID, hdr.DestConnectionID, versions); err != nil { wire.ComposeVersionNegotiation(hdr.SrcConnectionID, hdr.DestConnectionID, versions)
panic(err)
}
return 1 return 1
} }

View file

@ -297,7 +297,7 @@ var _ = Describe("MITM test", func() {
sendForgedVersionNegotationPacket := func(conn net.PacketConn, remoteAddr net.Addr, hdr *wire.Header) { sendForgedVersionNegotationPacket := func(conn net.PacketConn, remoteAddr net.Addr, hdr *wire.Header) {
// Create fake version negotiation packet with no supported versions // Create fake version negotiation packet with no supported versions
versions := []protocol.VersionNumber{} versions := []protocol.VersionNumber{}
packet, _ := wire.ComposeVersionNegotiation(hdr.SrcConnectionID, hdr.DestConnectionID, versions) packet := wire.ComposeVersionNegotiation(hdr.SrcConnectionID, hdr.DestConnectionID, versions)
// Send the packet // Send the packet
_, err := conn.WriteTo(packet, remoteAddr) _, err := conn.WriteTo(packet, remoteAddr)

View file

@ -35,7 +35,7 @@ func ParseVersionNegotiationPacket(b *bytes.Reader) (*Header, []protocol.Version
} }
// ComposeVersionNegotiation composes a Version Negotiation // ComposeVersionNegotiation composes a Version Negotiation
func ComposeVersionNegotiation(destConnID, srcConnID protocol.ConnectionID, versions []protocol.VersionNumber) ([]byte, error) { func ComposeVersionNegotiation(destConnID, srcConnID protocol.ConnectionID, versions []protocol.VersionNumber) []byte {
greasedVersions := protocol.GetGreasedVersions(versions) greasedVersions := protocol.GetGreasedVersions(versions)
expectedLen := 1 /* type byte */ + 4 /* version field */ + 1 /* dest connection ID length field */ + destConnID.Len() + 1 /* src connection ID length field */ + srcConnID.Len() + len(greasedVersions)*4 expectedLen := 1 /* type byte */ + 4 /* version field */ + 1 /* dest connection ID length field */ + destConnID.Len() + 1 /* src connection ID length field */ + srcConnID.Len() + len(greasedVersions)*4
buf := bytes.NewBuffer(make([]byte, 0, expectedLen)) buf := bytes.NewBuffer(make([]byte, 0, expectedLen))
@ -50,5 +50,5 @@ func ComposeVersionNegotiation(destConnID, srcConnID protocol.ConnectionID, vers
for _, v := range greasedVersions { for _, v := range greasedVersions {
utils.BigEndian.WriteUint32(buf, uint32(v)) utils.BigEndian.WriteUint32(buf, uint32(v))
} }
return buf.Bytes(), nil return buf.Bytes()
} }

View file

@ -36,20 +36,18 @@ var _ = Describe("Version Negotiation Packets", func() {
It("errors if it contains versions of the wrong length", func() { It("errors if it contains versions of the wrong length", func() {
connID := protocol.ConnectionID{1, 2, 3, 4, 5, 6, 7, 8} connID := protocol.ConnectionID{1, 2, 3, 4, 5, 6, 7, 8}
versions := []protocol.VersionNumber{0x22334455, 0x33445566} versions := []protocol.VersionNumber{0x22334455, 0x33445566}
data, err := ComposeVersionNegotiation(connID, connID, versions) data := ComposeVersionNegotiation(connID, connID, versions)
Expect(err).ToNot(HaveOccurred()) _, _, err := ParseVersionNegotiationPacket(bytes.NewReader(data[:len(data)-2]))
_, _, err = ParseVersionNegotiationPacket(bytes.NewReader(data[:len(data)-2]))
Expect(err).To(MatchError("Version Negotiation packet has a version list with an invalid length")) Expect(err).To(MatchError("Version Negotiation packet has a version list with an invalid length"))
}) })
It("errors if the version list is empty", func() { It("errors if the version list is empty", func() {
connID := protocol.ConnectionID{1, 2, 3, 4, 5, 6, 7, 8} connID := protocol.ConnectionID{1, 2, 3, 4, 5, 6, 7, 8}
versions := []protocol.VersionNumber{0x22334455} versions := []protocol.VersionNumber{0x22334455}
data, err := ComposeVersionNegotiation(connID, connID, versions) data := ComposeVersionNegotiation(connID, connID, versions)
Expect(err).ToNot(HaveOccurred())
// remove 8 bytes (two versions), since ComposeVersionNegotiation also added a reserved version number // remove 8 bytes (two versions), since ComposeVersionNegotiation also added a reserved version number
data = data[:len(data)-8] data = data[:len(data)-8]
_, _, err = ParseVersionNegotiationPacket(bytes.NewReader(data)) _, _, err := ParseVersionNegotiationPacket(bytes.NewReader(data))
Expect(err).To(MatchError("Version Negotiation packet has empty version list")) Expect(err).To(MatchError("Version Negotiation packet has empty version list"))
}) })
@ -57,8 +55,7 @@ var _ = Describe("Version Negotiation Packets", func() {
srcConnID := protocol.ConnectionID{0xde, 0xad, 0xbe, 0xef, 0xca, 0xfe, 0x13, 0x37} srcConnID := protocol.ConnectionID{0xde, 0xad, 0xbe, 0xef, 0xca, 0xfe, 0x13, 0x37}
destConnID := protocol.ConnectionID{1, 2, 3, 4, 5, 6, 7, 8} destConnID := protocol.ConnectionID{1, 2, 3, 4, 5, 6, 7, 8}
versions := []protocol.VersionNumber{1001, 1003} versions := []protocol.VersionNumber{1001, 1003}
data, err := ComposeVersionNegotiation(destConnID, srcConnID, versions) data := ComposeVersionNegotiation(destConnID, srcConnID, versions)
Expect(err).ToNot(HaveOccurred())
Expect(data[0] & 0x80).ToNot(BeZero()) Expect(data[0] & 0x80).ToNot(BeZero())
hdr, supportedVersions, err := ParseVersionNegotiationPacket(bytes.NewReader(data)) hdr, supportedVersions, err := ParseVersionNegotiationPacket(bytes.NewReader(data))
Expect(err).ToNot(HaveOccurred()) Expect(err).ToNot(HaveOccurred())

View file

@ -651,11 +651,7 @@ func (s *baseServer) sendError(remoteAddr net.Addr, hdr *wire.Header, sealer han
func (s *baseServer) sendVersionNegotiationPacket(p *receivedPacket, hdr *wire.Header) { func (s *baseServer) sendVersionNegotiationPacket(p *receivedPacket, hdr *wire.Header) {
s.logger.Debugf("Client offered version %s, sending Version Negotiation", hdr.Version) s.logger.Debugf("Client offered version %s, sending Version Negotiation", hdr.Version)
data, err := wire.ComposeVersionNegotiation(hdr.SrcConnectionID, hdr.DestConnectionID, s.config.Versions) data := wire.ComposeVersionNegotiation(hdr.SrcConnectionID, hdr.DestConnectionID, s.config.Versions)
if err != nil {
s.logger.Debugf("Error composing Version Negotiation: %s", err)
return
}
if s.config.Tracer != nil { if s.config.Tracer != nil {
s.config.Tracer.SentPacket( s.config.Tracer.SentPacket(
p.remoteAddr, p.remoteAddr,

View file

@ -426,12 +426,11 @@ var _ = Describe("Server", func() {
}) })
It("ignores Version Negotiation packets", func() { It("ignores Version Negotiation packets", func() {
data, err := wire.ComposeVersionNegotiation( data := wire.ComposeVersionNegotiation(
protocol.ConnectionID{1, 2, 3, 4}, protocol.ConnectionID{1, 2, 3, 4},
protocol.ConnectionID{4, 3, 2, 1}, protocol.ConnectionID{4, 3, 2, 1},
[]protocol.VersionNumber{1, 2, 3}, []protocol.VersionNumber{1, 2, 3},
) )
Expect(err).ToNot(HaveOccurred())
raddr := &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 1337} raddr := &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 1337}
done := make(chan struct{}) done := make(chan struct{})
tracer.EXPECT().DroppedPacket(raddr, logging.PacketTypeVersionNegotiation, protocol.ByteCount(len(data)), logging.PacketDropUnexpectedPacket).Do(func(net.Addr, logging.PacketType, protocol.ByteCount, logging.PacketDropReason) { tracer.EXPECT().DroppedPacket(raddr, logging.PacketTypeVersionNegotiation, protocol.ByteCount(len(data)), logging.PacketDropUnexpectedPacket).Do(func(net.Addr, logging.PacketType, protocol.ByteCount, logging.PacketDropReason) {