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.

Sign up for GitHub

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

Jump to bottom

remove error return value from ComposeVersionNegotiation #3410

Merged
merged 1 commit into from May 12, 2022
Merged

remove error return value from ComposeVersionNegotiation #3410

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 2 additions & 4 deletions connection_test.go
View file
Open in desktop
Expand Up @@ -674,8 +674,7 @@ var _ = Describe("Connection", func() {
})

It("drops Version Negotiation packets", func() {
b, err := wire.ComposeVersionNegotiation(srcConnID, destConnID, conn.config.Versions)
Expect(err).ToNot(HaveOccurred())
b := wire.ComposeVersionNegotiation(srcConnID, destConnID, conn.config.Versions)
tracer.EXPECT().DroppedPacket(logging.PacketTypeVersionNegotiation, protocol.ByteCount(len(b)), logging.PacketDropUnexpectedPacket)
Expect(conn.handlePacketImpl(&receivedPacket{
data: b,
Expand Down Expand Up @@ -2592,8 +2591,7 @@ var _ = Describe("Client Connection", func() {

Context("handling Version Negotiation", func() {
getVNP := func(versions ...protocol.VersionNumber) *receivedPacket {
b, err := wire.ComposeVersionNegotiation(srcConnID, destConnID, versions)
Expect(err).ToNot(HaveOccurred())
b := wire.ComposeVersionNegotiation(srcConnID, destConnID, versions)
return &receivedPacket{
data: b,
buffer: getPacketBuffer(),
Expand Down
6 changes: 1 addition & 5 deletions fuzzing/header/cmd/corpus.go
View file
Open in desktop
Expand Up @@ -24,11 +24,7 @@ func getVNP(src, dest protocol.ConnectionID, numVersions int) []byte {
for i := 0; i < numVersions; i++ {
versions[i] = protocol.VersionNumber(rand.Uint32())
}
data, err := wire.ComposeVersionNegotiation(src, dest, versions)
if err != nil {
log.Fatal(err)
}
return data
return wire.ComposeVersionNegotiation(src, dest, versions)
}

func main() {
Expand Down
4 changes: 1 addition & 3 deletions fuzzing/header/fuzz.go
View file
Open in desktop
Expand Up @@ -91,8 +91,6 @@ func fuzzVNP(data []byte) int {
if len(versions) == 0 {
panic("no versions")
}
if _, err := wire.ComposeVersionNegotiation(hdr.SrcConnectionID, hdr.DestConnectionID, versions); err != nil {
panic(err)
}
wire.ComposeVersionNegotiation(hdr.SrcConnectionID, hdr.DestConnectionID, versions)
return 1
}
2 changes: 1 addition & 1 deletion integrationtests/self/mitm_test.go
View file
Open in desktop
Expand Up @@ -297,7 +297,7 @@ var _ = Describe("MITM test", func() {
sendForgedVersionNegotationPacket := func(conn net.PacketConn, remoteAddr net.Addr, hdr *wire.Header) {
// Create fake version negotiation packet with no supported versions
versions := []protocol.VersionNumber{}
packet, _ := wire.ComposeVersionNegotiation(hdr.SrcConnectionID, hdr.DestConnectionID, versions)
packet := wire.ComposeVersionNegotiation(hdr.SrcConnectionID, hdr.DestConnectionID, versions)

// Send the packet
_, err := conn.WriteTo(packet, remoteAddr)
Expand Down
4 changes: 2 additions & 2 deletions internal/wire/version_negotiation.go
View file
Open in desktop
Expand Up @@ -35,7 +35,7 @@ func ParseVersionNegotiationPacket(b *bytes.Reader) (*Header, []protocol.Version
}

// 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)
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))
Expand All @@ -50,5 +50,5 @@ func ComposeVersionNegotiation(destConnID, srcConnID protocol.ConnectionID, vers
for _, v := range greasedVersions {
utils.BigEndian.WriteUint32(buf, uint32(v))
}
return buf.Bytes(), nil
return buf.Bytes()
}
13 changes: 5 additions & 8 deletions internal/wire/version_negotiation_test.go
View file
Open in desktop
Expand Up @@ -36,29 +36,26 @@ var _ = Describe("Version Negotiation Packets", func() {
It("errors if it contains versions of the wrong length", func() {
connID := protocol.ConnectionID{1, 2, 3, 4, 5, 6, 7, 8}
versions := []protocol.VersionNumber{0x22334455, 0x33445566}
data, err := ComposeVersionNegotiation(connID, connID, versions)
Expect(err).ToNot(HaveOccurred())
_, _, err = ParseVersionNegotiationPacket(bytes.NewReader(data[:len(data)-2]))
data := ComposeVersionNegotiation(connID, connID, versions)
_, _, err := ParseVersionNegotiationPacket(bytes.NewReader(data[:len(data)-2]))
Expect(err).To(MatchError("Version Negotiation packet has a version list with an invalid length"))
})

It("errors if the version list is empty", func() {
connID := protocol.ConnectionID{1, 2, 3, 4, 5, 6, 7, 8}
versions := []protocol.VersionNumber{0x22334455}
data, err := ComposeVersionNegotiation(connID, connID, versions)
Expect(err).ToNot(HaveOccurred())
data := ComposeVersionNegotiation(connID, connID, versions)
// remove 8 bytes (two versions), since ComposeVersionNegotiation also added a reserved version number
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"))
})

It("adds a reserved version", func() {
srcConnID := protocol.ConnectionID{0xde, 0xad, 0xbe, 0xef, 0xca, 0xfe, 0x13, 0x37}
destConnID := protocol.ConnectionID{1, 2, 3, 4, 5, 6, 7, 8}
versions := []protocol.VersionNumber{1001, 1003}
data, err := ComposeVersionNegotiation(destConnID, srcConnID, versions)
Expect(err).ToNot(HaveOccurred())
data := ComposeVersionNegotiation(destConnID, srcConnID, versions)
Expect(data[0] & 0x80).ToNot(BeZero())
hdr, supportedVersions, err := ParseVersionNegotiationPacket(bytes.NewReader(data))
Expect(err).ToNot(HaveOccurred())
Expand Down
6 changes: 1 addition & 5 deletions server.go
View file
Open in desktop
Expand Up @@ -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) {
s.logger.Debugf("Client offered version %s, sending Version Negotiation", hdr.Version)
data, err := wire.ComposeVersionNegotiation(hdr.SrcConnectionID, hdr.DestConnectionID, s.config.Versions)
if err != nil {
s.logger.Debugf("Error composing Version Negotiation: %s", err)
return
}
data := wire.ComposeVersionNegotiation(hdr.SrcConnectionID, hdr.DestConnectionID, s.config.Versions)
if s.config.Tracer != nil {
s.config.Tracer.SentPacket(
p.remoteAddr,
Expand Down
3 changes: 1 addition & 2 deletions server_test.go
View file
Open in desktop
Expand Up @@ -426,12 +426,11 @@ var _ = Describe("Server", func() {
})

It("ignores Version Negotiation packets", func() {
data, err := wire.ComposeVersionNegotiation(
data := wire.ComposeVersionNegotiation(
protocol.ConnectionID{1, 2, 3, 4},
protocol.ConnectionID{4, 3, 2, 1},
[]protocol.VersionNumber{1, 2, 3},
)
Expect(err).ToNot(HaveOccurred())
raddr := &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 1337}
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) {
Expand Down