From c6a9ec6d094ea84f6a5205380da9f4c3673cae12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Subir=C3=A0=20Nieto?= Date: Fri, 10 Nov 2023 10:28:42 +0100 Subject: [PATCH] fix validateNextHopAddr --- dispatcher/dispatcher.go | 13 ++++----- dispatcher/dispatcher_test.go | 51 +++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 6 deletions(-) diff --git a/dispatcher/dispatcher.go b/dispatcher/dispatcher.go index 9ae3ee265a..a239e568bd 100644 --- a/dispatcher/dispatcher.go +++ b/dispatcher/dispatcher.go @@ -401,19 +401,20 @@ func (m ipv6ControlMessage) Destination() net.IP { // it returns false. This implements a safeguard for traffic reflection as discussed in: // https://github.com/scionproto/scion/pull/4280#issuecomment-1775177351 func (s *Server) validateNextHopAddr(addr netip.AddrPort, oobuffer []byte) bool { - udpAddr, ok := s.conn.LocalAddr().(*net.UDPAddr) + // take localAddr to be consistent with the setIPPktInfo(ยท) configuration + localAddr, ok := s.conn.LocalAddr().(*net.UDPAddr) if !ok { panic(fmt.Sprintln("Connection address is not UDPAddr", "conn", s.conn.LocalAddr().Network())) } var cm controlMessage - if udpAddr.AddrPort().Addr().Unmap().Is4() { + if localAddr.AddrPort().Addr().Unmap().Is4() { cm = ipv4ControlMessage{ ControlMessage: new(ipv4.ControlMessage), } } - if udpAddr.AddrPort().Addr().Unmap().Is6() { + if localAddr.AddrPort().Addr().Unmap().Is6() { cm = ipv6ControlMessage{ ControlMessage: new(ipv6.ControlMessage), } @@ -421,16 +422,15 @@ func (s *Server) validateNextHopAddr(addr netip.AddrPort, oobuffer []byte) bool if err := cm.Parse(oobuffer); err != nil { log.Error("Parsing message", "err", err) - fmt.Println("Parsing message", "err", err) return false } - fmt.Println(cm.String()) if !cm.Destination().IsUnspecified() { pktAddr, ok := netip.AddrFromSlice(cm.Destination()) if !ok { + log.Error("Getting DST from IP_PKT info", "DST", cm.Destination()) return false } - if addr.Addr().Unmap().Compare(pktAddr) != 0 { + if addr.Addr().Unmap().Compare(pktAddr.Unmap()) != 0 { log.Error("UDP/IP addr destination different from UDP/SCION addr", "UDP/IP:", pktAddr.String(), "UDP/SCION:", addr.Addr().String()) @@ -438,6 +438,7 @@ func (s *Server) validateNextHopAddr(addr netip.AddrPort, oobuffer []byte) bool } return true } + log.Error("Unable to validate next hop address", "addr", addr) return false } diff --git a/dispatcher/dispatcher_test.go b/dispatcher/dispatcher_test.go index 0dffae27bd..8e6fc2ecce 100644 --- a/dispatcher/dispatcher_test.go +++ b/dispatcher/dispatcher_test.go @@ -66,6 +66,11 @@ func TestValidateAddr(t *testing.T) { dispIPv4Addr := xtest.MustParseUDPAddr(t, "127.0.0.1:40032") clientIPv6Addr := xtest.MustParseUDPAddr(t, "[::1]:0") dispIPv6Addr := xtest.MustParseUDPAddr(t, "[::1]:40032") + mappedDispIPv4Addr := &net.UDPAddr{ + IP: dispIPv4Addr.IP.To16(), + Port: 40032, + } + undefinedIPv6 := xtest.MustParseUDPAddr(t, "[::]:40032") testCases := []testCase{ { Name: "valid UDP/IPv4", @@ -299,6 +304,52 @@ func TestValidateAddr(t *testing.T) { }, ExpectedValue: false, }, + { + Name: "IPv4-mapped-IPv6 to IPv4", + ClientAddr: clientAddr, + DispAddr: dispIPv4Addr, + Pkt: &snet.Packet{ + PacketInfo: snet.PacketInfo{ + Source: snet.SCIONAddress{ + IA: xtest.MustParseIA("1-ff00:0:2"), + Host: addr.HostIP(clientAddr.AddrPort().Addr()), + }, + Destination: snet.SCIONAddress{ + IA: xtest.MustParseIA("1-ff00:0:1"), + Host: addr.HostIP(mappedDispIPv4Addr.AddrPort().Addr()), + }, + Payload: snet.UDPPayload{ + SrcPort: 20001, + DstPort: 40001, + }, + Path: path.Empty{}, + }, + }, + ExpectedValue: true, + }, + { + Name: "IPv4 to undefined IPv6", + ClientAddr: clientAddr, + DispAddr: undefinedIPv6, + Pkt: &snet.Packet{ + PacketInfo: snet.PacketInfo{ + Source: snet.SCIONAddress{ + IA: xtest.MustParseIA("1-ff00:0:2"), + Host: addr.HostIP(clientAddr.AddrPort().Addr()), + }, + Destination: snet.SCIONAddress{ + IA: xtest.MustParseIA("1-ff00:0:1"), + Host: addr.HostIP(dispIPv4Addr.AddrPort().Addr()), + }, + Payload: snet.UDPPayload{ + SrcPort: 20001, + DstPort: 40001, + }, + Path: path.Empty{}, + }, + }, + ExpectedValue: true, + }, } for _, test := range testCases { t.Run(test.Name, func(t *testing.T) {