From eae9aa78ab5816ef8d3a3390ea35b5d8c234da13 Mon Sep 17 00:00:00 2001 From: karampok Date: Mon, 25 Nov 2019 09:47:14 +0100 Subject: [PATCH] infraenv: use net.UDPAddr * remove bind field as it is deprecated * replace add.AppAddr to net.UDPAddr in topo interface --- go/beacon_srv/main.go | 16 ++++------ go/cert_srv/setup.go | 2 +- go/lib/infra/infraenv/infraenv.go | 34 ++++++++++----------- go/lib/infra/messenger/addr.go | 26 ---------------- go/lib/infra/messenger/addr_test.go | 42 -------------------------- go/lib/infra/modules/itopo/topology.go | 28 +++-------------- go/lib/sciond/mock_sciond/BUILD.bazel | 1 - go/lib/sciond/mock_sciond/sciond.go | 16 ---------- go/lib/snet/addr.go | 9 ++++++ go/lib/snet/dispatcher.go | 2 +- go/path_srv/main.go | 2 +- go/sciond/main.go | 10 ++++-- 12 files changed, 48 insertions(+), 140 deletions(-) diff --git a/go/beacon_srv/main.go b/go/beacon_srv/main.go index 945fee44e4..c626793ba1 100644 --- a/go/beacon_srv/main.go +++ b/go/beacon_srv/main.go @@ -132,7 +132,7 @@ func realMain() int { opentracing.SetGlobalTracer(tracer) nc := infraenv.NetworkConfig{ IA: topo.IA(), - Public: topo.SPublicAddress(addr.SvcBS, cfg.General.ID), + Public: topo.PublicAddress(addr.SvcBS, cfg.General.ID), SVC: addr.SvcBS, ReconnectToDispatcher: cfg.General.ReconnectToDispatcher, QUIC: infraenv.QUIC{ @@ -191,8 +191,9 @@ func realMain() int { // We do not need to drain the connection, since the src address is spoofed // to contain the topo address. ovAddr := topo.PublicAddress(addr.SvcBS, cfg.General.ID) - ovAddr.L4 = 0 - conn, _, err := pktDisp.RegisterTimeout(topo.IA(), ovAddr, nil, addr.SvcNone, time.Second) + t := addr.AppAddrFromUDP(ovAddr) + t.L4 = 0 + conn, _, err := pktDisp.RegisterTimeout(topo.IA(), t, nil, addr.SvcNone, time.Second) if err != nil { log.Crit("Unable to create SCION packet conn", "err", err) return 1 @@ -290,15 +291,10 @@ func (t *periodicTasks) Start() error { } t.running = true topo := t.topoProvider.Get() - topoAddress := topo.PublicAddress(addr.SvcBS, cfg.General.ID) - if topoAddress == nil { + bs := topo.PublicAddress(addr.SvcBS, cfg.General.ID) + if bs == nil { return serrors.New("Unable to find topo address") } - bs := &net.UDPAddr{ - IP: topoAddress.L3.IP(), - Port: int(topoAddress.L4), - } - var err error if t.registrars, err = t.startSegRegRunners(); err != nil { return err diff --git a/go/cert_srv/setup.go b/go/cert_srv/setup.go index aea8cc50eb..945c20f274 100644 --- a/go/cert_srv/setup.go +++ b/go/cert_srv/setup.go @@ -164,7 +164,7 @@ func setMessenger(cfg *config.Config, router snet.Router) error { } nc := infraenv.NetworkConfig{ IA: topo.IA(), - Public: topo.SPublicAddress(addr.SvcCS, cfg.General.ID), + Public: topo.PublicAddress(addr.SvcCS, cfg.General.ID), SVC: addr.SvcCS, ReconnectToDispatcher: cfg.General.ReconnectToDispatcher, QUIC: infraenv.QUIC{ diff --git a/go/lib/infra/infraenv/infraenv.go b/go/lib/infra/infraenv/infraenv.go index b40383c76f..a7b74316f4 100644 --- a/go/lib/infra/infraenv/infraenv.go +++ b/go/lib/infra/infraenv/infraenv.go @@ -61,9 +61,7 @@ type NetworkConfig struct { IA addr.IA // Public is the Internet-reachable address in the case where the service // is behind NAT. - Public *snet.Addr - // Bind is the local address the server should listen on. - Bind *snet.Addr + Public *net.UDPAddr // SVC registers this server to receive packets with the specified SVC // destination address. SVC addr.HostSVC @@ -152,7 +150,10 @@ func (nc *NetworkConfig) AddressRewriter( Resolver: &svc.Resolver{ LocalIA: nc.IA, ConnFactory: connFactory, - Machine: buildLocalMachine(nc.Bind, nc.Public), + Machine: snet.LocalMachine{ + PublicIP: nc.Public.IP, + InterfaceIP: nc.Public.IP, + }, // Legacy control payloads have a 4-byte length prefix. A // 0-value for the prefix is invalid, so SVC resolution-aware // servers can use this to detect that the client is attempting @@ -170,7 +171,12 @@ func (nc *NetworkConfig) AddressRewriter( // resolution requests. If argument address is not the empty string, it will be // included as the QUIC address in SVC resolution replies. func (nc *NetworkConfig) initUDPSocket(quicAddress string) (net.PacketConn, error) { - reply := messenger.BuildReply(nc.Public.Host) + reply := &svc.Reply{ + Transports: map[svc.Transport]string{ + svc.UDP: nc.Public.String(), + }, + } + if quicAddress != "" { reply.Transports[svc.QUIC] = quicAddress } @@ -196,7 +202,12 @@ func (nc *NetworkConfig) initUDPSocket(quicAddress string) (net.PacketConn, erro }, ) network := snet.NewCustomNetworkWithPR(nc.IA, packetDispatcher) - conn, err := network.ListenSCIONWithBindSVC("udp4", nc.Public, nc.Bind, nc.SVC, 0) + var listenAddr *snet.Addr + if nc.Public != nil { + listenAddr = &snet.Addr{IA: nc.IA, Host: addr.AppAddrFromUDP(nc.Public)} + } + + conn, err := network.ListenSCIONWithBindSVC("udp4", listenAddr, nil, nc.SVC, 0) if err != nil { return nil, common.NewBasicError("Unable to listen on SCION", err) } @@ -248,17 +259,6 @@ func (nc *NetworkConfig) buildQUICConfig(conn net.PacketConn) (*messenger.QUICCo }, nil } -func buildLocalMachine(bind, public *snet.Addr) snet.LocalMachine { - var mi snet.LocalMachine - mi.PublicIP = public.Host.L3.IP() - if bind != nil { - mi.InterfaceIP = bind.Host.L3.IP() - } else { - mi.InterfaceIP = mi.PublicIP - } - return mi -} - // LegacyForwardingHandler is an SVC resolution handler that only responds to // packets that have an SVC destination address and contain exactly 4 0x00 // bytes in their payload. All other packets are considered to originate from diff --git a/go/lib/infra/messenger/addr.go b/go/lib/infra/messenger/addr.go index 59d15b7532..da45f29669 100644 --- a/go/lib/infra/messenger/addr.go +++ b/go/lib/infra/messenger/addr.go @@ -16,7 +16,6 @@ package messenger import ( "context" - "fmt" "net" "time" @@ -223,31 +222,6 @@ func parseReply(reply *svc.Reply) (*addr.AppAddr, error) { }, nil } -// BuildReply constructs a reply from an application address. If the -// application address is not well formed (has L3, has L4, UDP/IP protocols), -// the returned reply is non-nil and empty. -func BuildReply(address *addr.AppAddr) *svc.Reply { - if address == nil || address.L3 == nil { - return &svc.Reply{} - } - port := fmt.Sprintf("%v", address.L4) - - var ip string - switch t := address.L3.(type) { - case addr.HostIPv4: - ip = t.String() - case addr.HostIPv6: - ip = t.String() - default: - return &svc.Reply{} - } - return &svc.Reply{ - Transports: map[svc.Transport]string{ - svc.UDP: net.JoinHostPort(ip, port), - }, - } -} - // LocalSVCRouter is used to construct overlay information for SVC servers // running in the local AS. type LocalSVCRouter interface { diff --git a/go/lib/infra/messenger/addr_test.go b/go/lib/infra/messenger/addr_test.go index b8f1068e10..b587b603a4 100644 --- a/go/lib/infra/messenger/addr_test.go +++ b/go/lib/infra/messenger/addr_test.go @@ -415,48 +415,6 @@ func TestParseReply(t *testing.T) { } } -func TestBuildReply(t *testing.T) { - testCases := map[string]struct { - input *addr.AppAddr - want *svc.Reply - }{ - "nil app address": { - want: &svc.Reply{}, - }, - "nil L3": { - input: &addr.AppAddr{L4: 1}, - want: &svc.Reply{}, - }, - "nil L4": { - input: newSVCAppAddr(addr.SvcBS), - want: &svc.Reply{}, - }, - "IPv4 L3, UDP L4": { - input: newUDPAppAddr(&net.UDPAddr{IP: net.IP{192, 168, 0, 1}, Port: 1}), - want: &svc.Reply{ - Transports: map[svc.Transport]string{ - svc.UDP: "192.168.0.1:1", - }, - }, - }, - "IPv6 L3, UDP L4": { - input: newUDPAppAddr(&net.UDPAddr{IP: net.ParseIP("2001:db8::1"), Port: 1}), - want: &svc.Reply{ - Transports: map[svc.Transport]string{ - svc.UDP: "[2001:db8::1]:1", - }, - }, - }, - } - - for tn, tc := range testCases { - t.Run(tn, func(t *testing.T) { - got := messenger.BuildReply(tc.input) - assert.Equal(t, got, tc.want) - }) - } -} - func initResolver(resolver *mock_messenger.MockResolver, f func(*mock_messenger.MockResolver)) { if f != nil { f(resolver) diff --git a/go/lib/infra/modules/itopo/topology.go b/go/lib/infra/modules/itopo/topology.go index 111c40756b..abdc0c4900 100644 --- a/go/lib/infra/modules/itopo/topology.go +++ b/go/lib/infra/modules/itopo/topology.go @@ -39,14 +39,7 @@ type Topology interface { // PublicAddress gets the public address of a server with the requested type and name, and nil // if no such server exists. - // - // FIXME(scrye): See whether this or its snet variant below can be removed. - PublicAddress(svc addr.HostSVC, name string) *addr.AppAddr - // SPublicAddress gets the public address of a server with the requested type and name, and nil - // if no such server exists. - // - // FIXME(scrye): See whether this or its app variant above can be removed. - SPublicAddress(svc addr.HostSVC, name string) *snet.Addr + PublicAddress(svc addr.HostSVC, name string) *net.UDPAddr // Exists returns true if the service and name are present in the topology file. Exists(svc addr.HostSVC, name string) bool @@ -225,27 +218,16 @@ func (t *topologyS) BR(name string) (topology.BRInfo, bool) { return br, ok } -func (t *topologyS) SPublicAddress(svc addr.HostSVC, name string) *snet.Addr { - address := t.PublicAddress(svc, name) - if address == nil { - return nil - } - return &snet.Addr{ - IA: t.IA(), - Host: address.Copy(), - } -} - -func (t *topologyS) PublicAddress(svc addr.HostSVC, name string) *addr.AppAddr { +func (t *topologyS) PublicAddress(svc addr.HostSVC, name string) *net.UDPAddr { topoAddr := t.topoAddress(svc, name) if topoAddr == nil { return nil } - publicAddr := topoAddr.SCIONAddress - if publicAddr == nil { + pa := topoAddr.SCIONAddress + if pa == nil { return nil } - return publicAddr.Copy() + return &net.UDPAddr{IP: pa.L3.IP(), Port: int(pa.L4)} } func (t *topologyS) Exists(svc addr.HostSVC, name string) bool { diff --git a/go/lib/sciond/mock_sciond/BUILD.bazel b/go/lib/sciond/mock_sciond/BUILD.bazel index 35ac0eb938..59ab63cd0f 100644 --- a/go/lib/sciond/mock_sciond/BUILD.bazel +++ b/go/lib/sciond/mock_sciond/BUILD.bazel @@ -7,7 +7,6 @@ go_library( visibility = ["//visibility:public"], deps = [ "//go/lib/addr:go_default_library", - "//go/lib/common:go_default_library", "//go/lib/ctrl/path_mgmt:go_default_library", "//go/lib/sciond:go_default_library", "//go/lib/snet:go_default_library", diff --git a/go/lib/sciond/mock_sciond/sciond.go b/go/lib/sciond/mock_sciond/sciond.go index 7c9704e14e..67545f3d25 100644 --- a/go/lib/sciond/mock_sciond/sciond.go +++ b/go/lib/sciond/mock_sciond/sciond.go @@ -8,7 +8,6 @@ import ( context "context" gomock "github.com/golang/mock/gomock" addr "github.com/scionproto/scion/go/lib/addr" - common "github.com/scionproto/scion/go/lib/common" path_mgmt "github.com/scionproto/scion/go/lib/ctrl/path_mgmt" sciond "github.com/scionproto/scion/go/lib/sciond" snet "github.com/scionproto/scion/go/lib/snet" @@ -106,21 +105,6 @@ func (mr *MockConnectorMockRecorder) Close(arg0 interface{}) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Close", reflect.TypeOf((*MockConnector)(nil).Close), arg0) } -// IFInfo mocks base method -func (m *MockConnector) IFInfo(arg0 context.Context, arg1 []common.IFIDType) (*sciond.IFInfoReply, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "IFInfo", arg0, arg1) - ret0, _ := ret[0].(*sciond.IFInfoReply) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// IFInfo indicates an expected call of IFInfo -func (mr *MockConnectorMockRecorder) IFInfo(arg0, arg1 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IFInfo", reflect.TypeOf((*MockConnector)(nil).IFInfo), arg0, arg1) -} - // Paths mocks base method func (m *MockConnector) Paths(arg0 context.Context, arg1, arg2 addr.IA, arg3 uint16, arg4 sciond.PathReqFlags) ([]snet.Path, error) { m.ctrl.T.Helper() diff --git a/go/lib/snet/addr.go b/go/lib/snet/addr.go index 05f7e62773..2b0a461874 100644 --- a/go/lib/snet/addr.go +++ b/go/lib/snet/addr.go @@ -39,6 +39,15 @@ type Addr struct { NextHop *net.UDPAddr } +// ToNetUDPAddr returns a net.UDPAddr or nil. +func (a *Addr) ToNetUDPAddr() *net.UDPAddr { + switch a.Host.L3.Type() { + case addr.HostTypeIPv4, addr.HostTypeIPv6: + return &net.UDPAddr{IP: a.Host.L3.IP(), Port: int(a.Host.L4)} + } + return nil +} + func (a *Addr) Network() string { return "scion" } diff --git a/go/lib/snet/dispatcher.go b/go/lib/snet/dispatcher.go index 557d59b58e..3a4c45bc51 100644 --- a/go/lib/snet/dispatcher.go +++ b/go/lib/snet/dispatcher.go @@ -56,7 +56,7 @@ func (s *DefaultPacketDispatcherService) RegisterTimeout(ia addr.IA, public *add if err != nil { return nil, 0, err } - return &SCIONPacketConn{conn: rconn, scmpHandler: s.SCMPHandler}, port, err + return &SCIONPacketConn{conn: rconn, scmpHandler: s.SCMPHandler}, port, nil } // RevocationHandler is called by the default SCMP Handler whenever revocations are encountered. diff --git a/go/path_srv/main.go b/go/path_srv/main.go index 6c92d7e2b7..6c0d299f93 100644 --- a/go/path_srv/main.go +++ b/go/path_srv/main.go @@ -124,7 +124,7 @@ func realMain() int { opentracing.SetGlobalTracer(tracer) nc := infraenv.NetworkConfig{ IA: topo.IA(), - Public: topo.SPublicAddress(addr.SvcPS, cfg.General.ID), + Public: topo.PublicAddress(addr.SvcPS, cfg.General.ID), SVC: addr.SvcPS, ReconnectToDispatcher: cfg.General.ReconnectToDispatcher, QUIC: infraenv.QUIC{ diff --git a/go/sciond/main.go b/go/sciond/main.go index 75329cbcbe..6ea95c92d9 100644 --- a/go/sciond/main.go +++ b/go/sciond/main.go @@ -18,6 +18,7 @@ import ( "context" "flag" "fmt" + "net" _ "net/http/pprof" "os" "path/filepath" @@ -115,10 +116,15 @@ func realMain() int { } defer trCloser.Close() opentracing.SetGlobalTracer(tracer) + + var publicIP *net.UDPAddr + if p := cfg.SD.Public; p != nil { + publicIP = p.ToNetUDPAddr() + } + nc := infraenv.NetworkConfig{ IA: itopo.Get().IA(), - Public: cfg.SD.Public, - Bind: cfg.SD.Bind, + Public: publicIP, SVC: addr.SvcNone, ReconnectToDispatcher: cfg.General.ReconnectToDispatcher, QUIC: infraenv.QUIC{