From 31193c50af2aefd0ffd0b6f56db55b5e7a8dd6c1 Mon Sep 17 00:00:00 2001 From: Gunjan Vyas Date: Sat, 10 May 2025 14:27:35 +0530 Subject: [PATCH] lint fix: fix linter errors after migrating to v2 Signed-off-by: Gunjan Vyas --- cmd/ssh-over-vsock/client.go | 2 +- cmd/vm/main_linux.go | 5 +++++ pkg/services/dhcp/dhcp.go | 13 ++++++++++--- pkg/services/forwarder/ports.go | 2 +- pkg/sshclient/ssh_forwarder.go | 2 +- pkg/tap/ip_pool.go | 9 ++++++--- pkg/tap/link.go | 8 ++++---- pkg/tap/protocols.go | 15 +++++++++++++-- pkg/tap/switch.go | 9 +++++++-- pkg/transport/dial_linux.go | 4 ++-- pkg/transport/listen_linux.go | 4 ++-- pkg/virtualnetwork/conn.go | 9 ++++----- pkg/virtualnetwork/mux.go | 5 +++-- pkg/virtualnetwork/virtualnetwork.go | 8 ++++++-- pkg/virtualnetwork/vpnkit.go | 6 ++++++ test-vfkit/vfkit_suite_test.go | 6 +++--- 16 files changed, 74 insertions(+), 33 deletions(-) diff --git a/cmd/ssh-over-vsock/client.go b/cmd/ssh-over-vsock/client.go index cca9a864..fc982276 100644 --- a/cmd/ssh-over-vsock/client.go +++ b/cmd/ssh-over-vsock/client.go @@ -17,7 +17,7 @@ type client struct { func newClient(conn net.Conn, user string, key string) (*client, error) { config, err := newConfig(user, key) if err != nil { - return nil, fmt.Errorf("Error getting config for native Go SSH: %s", err) + return nil, fmt.Errorf("error getting config for native Go SSH: %s", err) } return &client{ diff --git a/cmd/vm/main_linux.go b/cmd/vm/main_linux.go index 71e2106b..366493b3 100644 --- a/cmd/vm/main_linux.go +++ b/cmd/vm/main_linux.go @@ -5,6 +5,7 @@ import ( "flag" "fmt" "io" + "math" "net" "net/http" "os" @@ -175,6 +176,10 @@ func rx(conn net.Conn, tap *water.Interface, errCh chan error, mtu int) { log.Info(packet.String()) } + if n < 0 || n > math.MaxUint16 { + log.Errorf("invalid frame length") + return + } binary.LittleEndian.PutUint16(size, uint16(n)) if _, err := conn.Write(append(size, frame...)); err != nil { errCh <- errors.Wrap(err, "cannot write size and packet to socket") diff --git a/pkg/services/dhcp/dhcp.go b/pkg/services/dhcp/dhcp.go index f0218fbc..aa8aa100 100644 --- a/pkg/services/dhcp/dhcp.go +++ b/pkg/services/dhcp/dhcp.go @@ -3,6 +3,7 @@ package dhcp import ( "encoding/json" "errors" + "math" "net" "net/http" "time" @@ -50,7 +51,13 @@ func handler(configuration *types.Configuration, ipPool *tap.IPPool) server4.Han reply.UpdateOption(dhcpv4.Option{Code: dhcpv4.OptionSubnetMask, Value: dhcpv4.IP(parsedSubnet.Mask)}) reply.UpdateOption(dhcpv4.Option{Code: dhcpv4.OptionRouter, Value: dhcpv4.IP(net.ParseIP(configuration.GatewayIP))}) reply.UpdateOption(dhcpv4.Option{Code: dhcpv4.OptionDomainNameServer, Value: dhcpv4.IPs([]net.IP{net.ParseIP(configuration.GatewayIP)})}) - reply.UpdateOption(dhcpv4.Option{Code: dhcpv4.OptionInterfaceMTU, Value: dhcpv4.Uint16(configuration.MTU)}) + + mtu := configuration.MTU + if mtu < 0 || mtu > math.MaxUint16 { + log.Errorf("dhcp: invalid MTU %d", mtu) + } else { + reply.UpdateOption(dhcpv4.Option{Code: dhcpv4.OptionInterfaceMTU, Value: dhcpv4.Uint16(mtu)}) + } reply.UpdateOption(dhcpv4.Option{Code: dhcpv4.OptionDNSDomainSearchList, Value: &rfc1035label.Labels{ Labels: configuration.DNSSearchDomains, }}) @@ -71,7 +78,7 @@ func handler(configuration *types.Configuration, ipPool *tap.IPPool) server4.Han } } -func dial(s *stack.Stack, nic int) (*gonet.UDPConn, error) { +func dial(s *stack.Stack, nic tcpip.NICID) (*gonet.UDPConn, error) { var wq waiter.Queue ep, err := s.NewEndpoint(udp.ProtocolNumber, ipv4.ProtocolNumber, &wq) if err != nil { @@ -98,7 +105,7 @@ type Server struct { } func New(configuration *types.Configuration, stack *stack.Stack, ipPool *tap.IPPool) (*Server, error) { - ln, err := dial(stack, 1) + ln, err := dial(stack, tcpip.NICID(1)) if err != nil { return nil, err } diff --git a/pkg/services/forwarder/ports.go b/pkg/services/forwarder/ports.go index a1de5401..200471e2 100644 --- a/pkg/services/forwarder/ports.go +++ b/pkg/services/forwarder/ports.go @@ -378,7 +378,7 @@ func tcpipAddress(nicID tcpip.NICID, remote string) (address tcpip.FullAddress, return address, errors.New("invalid remote addr") } - port, err := strconv.Atoi(split[1]) + port, err := strconv.ParseUint(split[1], 10, 16) if err != nil { return address, err diff --git a/pkg/sshclient/ssh_forwarder.go b/pkg/sshclient/ssh_forwarder.go index 9133a2e7..8f137fab 100644 --- a/pkg/sshclient/ssh_forwarder.go +++ b/pkg/sshclient/ssh_forwarder.go @@ -92,7 +92,7 @@ func connectForward(ctx context.Context, bastion *Bastion) (CloseWriteConn, erro return nil, errors.Wrapf(err, "Couldn't reestablish ssh tunnel on path: %s", bastion.Path) } // Check if ssh connection is still alive - _, _, err = bastion.Client.Conn.SendRequest("alive@gvproxy", true, nil) + _, _, err = bastion.Client.SendRequest("alive@gvproxy", true, nil) if err != nil { for bastionRetries := 1; ; bastionRetries++ { err = bastion.Reconnect(ctx) diff --git a/pkg/tap/ip_pool.go b/pkg/tap/ip_pool.go index 87e470cc..fc6a609d 100644 --- a/pkg/tap/ip_pool.go +++ b/pkg/tap/ip_pool.go @@ -2,6 +2,7 @@ package tap import ( "errors" + "math" "net" "sync" @@ -48,9 +49,11 @@ func (p *IPPool) GetOrAssign(mac string) (net.IP, error) { } } - var i uint64 - for i = 1; i < p.count; i++ { - candidate, err := cidr.Host(p.base, int(i)) + if p.count > math.MaxInt { + return nil, errors.New("IP pool exceeds maximum number of IP addresses") + } + for i := 1; i < int(p.count); i++ { + candidate, err := cidr.Host(p.base, i) if err != nil { continue } diff --git a/pkg/tap/link.go b/pkg/tap/link.go index eb3b2960..37359454 100644 --- a/pkg/tap/link.go +++ b/pkg/tap/link.go @@ -13,7 +13,7 @@ import ( type LinkEndpoint struct { debug bool - mtu int + mtu uint32 mac tcpip.LinkAddress ip string virtualIPs map[string]struct{} @@ -22,7 +22,7 @@ type LinkEndpoint struct { networkSwitch NetworkSwitch } -func NewLinkEndpoint(debug bool, mtu int, macAddress string, ip string, virtualIPs []string) (*LinkEndpoint, error) { +func NewLinkEndpoint(debug bool, mtu uint32, macAddress string, ip string, virtualIPs []string) (*LinkEndpoint, error) { linkAddr, err := net.ParseMAC(macAddress) if err != nil { return nil, err @@ -82,11 +82,11 @@ func (e *LinkEndpoint) MaxHeaderLength() uint16 { } func (e *LinkEndpoint) MTU() uint32 { - return uint32(e.mtu) + return e.mtu } func (e *LinkEndpoint) SetMTU(mtu uint32) { - e.mtu = int(mtu) + e.mtu = mtu } func (e *LinkEndpoint) Wait() {} diff --git a/pkg/tap/protocols.go b/pkg/tap/protocols.go index 6402abf2..26a4a6dd 100644 --- a/pkg/tap/protocols.go +++ b/pkg/tap/protocols.go @@ -2,6 +2,9 @@ package tap import ( "encoding/binary" + "math" + + log "github.com/sirupsen/logrus" ) type protocol interface { @@ -27,7 +30,11 @@ func (s *hyperkitProtocol) Buf() []byte { } func (s *hyperkitProtocol) Write(buf []byte, size int) { - binary.LittleEndian.PutUint16(buf, uint16(size)) + if size < 0 || size > math.MaxUint16 { + log.Warnf("size out of range. Resetting to %d", math.MaxUint16) + size = math.MaxUint16 + } + binary.LittleEndian.PutUint16(buf, uint16(size)) //#nosec: G115 } func (s *hyperkitProtocol) Read(buf []byte) int { @@ -46,7 +53,11 @@ func (s *qemuProtocol) Buf() []byte { } func (s *qemuProtocol) Write(buf []byte, size int) { - binary.BigEndian.PutUint32(buf, uint32(size)) + if size > math.MaxUint32 { + log.Warnf("size exceeds max limit. Resetting to: %d", math.MaxInt32) + size = math.MaxUint32 + } + binary.BigEndian.PutUint32(buf, uint32(size)) //#nosec: G115. Safely checked } func (s *qemuProtocol) Read(buf []byte) int { diff --git a/pkg/tap/switch.go b/pkg/tap/switch.go index bc6b36b1..3b02fd7c 100644 --- a/pkg/tap/switch.go +++ b/pkg/tap/switch.go @@ -3,6 +3,7 @@ package tap import ( "bufio" "context" + "fmt" "io" "net" "sync" @@ -127,6 +128,10 @@ func (e *Switch) txPkt(pkt *stack.PacketBuffer) error { dst := eth.DestinationAddress() src := eth.SourceAddress() + size := pkt.Size() + if size < 0 { + return fmt.Errorf("packet size out of range") + } if dst == header.EthernetBroadcastAddress { e.camLock.RLock() srcID, ok := e.cam[src] @@ -144,7 +149,7 @@ func (e *Switch) txPkt(pkt *stack.PacketBuffer) error { return err } - atomic.AddUint64(&e.Sent, uint64(pkt.Size())) + atomic.AddUint64(&e.Sent, uint64(size)) } } else { e.camLock.RLock() @@ -159,7 +164,7 @@ func (e *Switch) txPkt(pkt *stack.PacketBuffer) error { if err != nil { return err } - atomic.AddUint64(&e.Sent, uint64(pkt.Size())) + atomic.AddUint64(&e.Sent, uint64(size)) } return nil } diff --git a/pkg/transport/dial_linux.go b/pkg/transport/dial_linux.go index 6030bf04..2fd3e432 100644 --- a/pkg/transport/dial_linux.go +++ b/pkg/transport/dial_linux.go @@ -18,11 +18,11 @@ func Dial(endpoint string) (net.Conn, string, error) { } switch parsed.Scheme { case "vsock": - contextID, err := strconv.Atoi(parsed.Hostname()) + contextID, err := strconv.ParseUint(parsed.Hostname(), 10, 32) if err != nil { return nil, "", err } - port, err := strconv.Atoi(parsed.Port()) + port, err := strconv.ParseUint(parsed.Port(), 10, 32) if err != nil { return nil, "", err } diff --git a/pkg/transport/listen_linux.go b/pkg/transport/listen_linux.go index dfefecd3..53489502 100644 --- a/pkg/transport/listen_linux.go +++ b/pkg/transport/listen_linux.go @@ -13,13 +13,13 @@ const DefaultURL = "vsock://:1024" func listenURL(parsed *url.URL) (net.Listener, error) { switch parsed.Scheme { case "vsock": - port, err := strconv.Atoi(parsed.Port()) + port, err := strconv.ParseUint(parsed.Port(), 10, 32) if err != nil { return nil, err } if parsed.Hostname() != "" { - cid, err := strconv.Atoi(parsed.Hostname()) + cid, err := strconv.ParseUint(parsed.Hostname(), 10, 32) if err != nil { return nil, err } diff --git a/pkg/virtualnetwork/conn.go b/pkg/virtualnetwork/conn.go index 038d8c5e..0cce39da 100644 --- a/pkg/virtualnetwork/conn.go +++ b/pkg/virtualnetwork/conn.go @@ -28,12 +28,11 @@ func (n *VirtualNetwork) DialContextTCP(ctx context.Context, addr string) (net.C if err != nil { return nil, err } - return gonet.DialContextTCP(ctx, n.stack, tcpip.FullAddress{ NIC: 1, Addr: tcpip.AddrFrom4Slice(ip.To4()), - Port: uint16(port), + Port: port, }, ipv4.ProtocolNumber) } @@ -45,11 +44,11 @@ func (n *VirtualNetwork) Listen(network, addr string) (net.Listener, error) { return gonet.ListenTCP(n.stack, tcpip.FullAddress{ NIC: 1, Addr: tcpip.AddrFrom4Slice(ip.To4()), - Port: uint16(port), + Port: port, }, ipv4.ProtocolNumber) } -func splitIPPort(network string, addr string) (net.IP, uint64, error) { +func splitIPPort(network string, addr string) (net.IP, uint16, error) { if network != "tcp" { return nil, 0, errors.New("only tcp is supported") } @@ -65,5 +64,5 @@ func splitIPPort(network string, addr string) (net.IP, uint64, error) { if ip == nil { return nil, 0, errors.New("invalid address, must be an IP") } - return ip, port, nil + return ip, uint16(port), nil } diff --git a/pkg/virtualnetwork/mux.go b/pkg/virtualnetwork/mux.go index 59629f34..9bba4aae 100644 --- a/pkg/virtualnetwork/mux.go +++ b/pkg/virtualnetwork/mux.go @@ -33,11 +33,12 @@ func (n *VirtualNetwork) ServicesMux() *http.ServeMux { http.Error(w, "ip is mandatory", http.StatusInternalServerError) return } - port, err := strconv.Atoi(r.URL.Query().Get("port")) + port, err := strconv.ParseUint(r.URL.Query().Get("port"), 10, 16) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return } + port16 := uint16(port) hj, ok := w.(http.Hijacker) if !ok { @@ -67,7 +68,7 @@ func (n *VirtualNetwork) ServicesMux() *http.ServeMux { return gonet.DialContextTCP(ctx, n.stack, tcpip.FullAddress{ NIC: 1, Addr: tcpip.AddrFrom4Slice(net.ParseIP(ip).To4()), - Port: uint16(port), + Port: port16, }, ipv4.ProtocolNumber) }, OnDialError: func(_ net.Conn, dstDialErr error) { diff --git a/pkg/virtualnetwork/virtualnetwork.go b/pkg/virtualnetwork/virtualnetwork.go index 4133717d..5d35445e 100644 --- a/pkg/virtualnetwork/virtualnetwork.go +++ b/pkg/virtualnetwork/virtualnetwork.go @@ -41,11 +41,15 @@ func New(configuration *types.Configuration) (*VirtualNetwork, error) { ipPool.Reserve(net.ParseIP(ip), mac) } - tapEndpoint, err := tap.NewLinkEndpoint(configuration.Debug, configuration.MTU, configuration.GatewayMacAddress, configuration.GatewayIP, configuration.GatewayVirtualIPs) + mtu := configuration.MTU + if mtu < 0 || mtu > math.MaxUint32 { + return nil, errors.New("mtu is out of range") + } + tapEndpoint, err := tap.NewLinkEndpoint(configuration.Debug, uint32(mtu), configuration.GatewayMacAddress, configuration.GatewayIP, configuration.GatewayVirtualIPs) if err != nil { return nil, errors.Wrap(err, "cannot create tap endpoint") } - networkSwitch := tap.NewSwitch(configuration.Debug, configuration.MTU) + networkSwitch := tap.NewSwitch(configuration.Debug, mtu) tapEndpoint.Connect(networkSwitch) networkSwitch.Connect(tapEndpoint) diff --git a/pkg/virtualnetwork/vpnkit.go b/pkg/virtualnetwork/vpnkit.go index 577670c6..a37ba49a 100644 --- a/pkg/virtualnetwork/vpnkit.go +++ b/pkg/virtualnetwork/vpnkit.go @@ -4,7 +4,9 @@ import ( "context" "crypto/rand" "encoding/binary" + "fmt" "io" + "math" "net" "github.com/containers/gvisor-tap-vsock/pkg/types" @@ -41,6 +43,10 @@ func vpnkitHandshake(conn net.Conn, configuration *types.Configuration) error { // https://github.com/moby/hyperkit/blob/2f061e447e1435cdf1b9eda364cea6414f2c606b/src/lib/pci_virtio_net_vpnkit.c#L131 resp := make([]byte, 258) resp[0] = 0x01 + + if configuration.MTU < 0 || configuration.MTU > math.MaxUint16 { + return fmt.Errorf("invalid MTU: %d", configuration.MTU) + } mtu := uint16(configuration.MTU) binary.LittleEndian.PutUint16(resp[1:3], mtu) binary.LittleEndian.PutUint16(resp[3:5], mtu+header.EthernetMinimumSize) diff --git a/test-vfkit/vfkit_suite_test.go b/test-vfkit/vfkit_suite_test.go index acc4390a..855c6322 100644 --- a/test-vfkit/vfkit_suite_test.go +++ b/test-vfkit/vfkit_suite_test.go @@ -62,7 +62,7 @@ func init() { var _ = ginkgo.BeforeSuite(func() { // clear the environment before running the tests. It may happen the tests were abruptly stopped earlier leaving a dirty env - clear() + cleanup() // check if vfkit version is greater than v0.5 (ignition support is available starting from v0.6) version, err := vfkitVersion() @@ -207,7 +207,7 @@ func sshCommand(cmd ...string) *exec.Cmd { return sshCmd } -func clear() { +func cleanup() { _ = os.Remove(efiStore) _ = os.Remove(sock) _ = os.Remove(vfkitSock) @@ -249,5 +249,5 @@ var _ = ginkgo.AfterSuite(func() { log.Error(err) } } - clear() + cleanup() })