Make hiddenpaths registration work both locally and remotely

acceptance/common/scion.py

@@ -14,7 +14,7 @@
 
 import logging
 import json
-from typing import Any, Dict, List, MutableMapping
+from typing import Any, Dict, List, MutableMapping, Mapping
 
 import toml
 import yaml
@@ -69,6 +69,29 @@ def update_json(change_dict: Dict[str, Any], files: LocalPath):
             json.dump(t, f, indent=2)
 
 
+def load_from_json(key: str, files: LocalPath) -> Any:
+    """ Reads the value associated with the given key from the given json files.
+
+    The first value found is returned. If not found, None is returned.
+
+    Args:
+        key: dot separated path of the JSON key.
+        files: names of file or files to read.
+
+    Returns:
+        The value. None if the path doesn't exist in the dictionary tree.
+
+    Raises:
+        IOError / FileNotFoundError: File path is not valid
+    """
+    for file in files:
+        with open(file, "r") as f:
+            t = json.load(f)
+            v = val_at_path(t, key)
+            if v is not None:
+                return v
+
+
 class ASList:
     """
     ASList is a list of AS separated by core and non-core ASes. It can be loaded
@@ -117,6 +140,18 @@ def path_to_dict(path: str, val: Any) -> Dict:
     return d
 
 
+def val_at_path(d: Mapping[str, Any], path: str) -> Any:
+    """
+    Walks nested dictionaries by following the given path and returns the value
+    associated with the leaf key.
+    """
+    v = d
+    for k in path.split('.'):
+        v = v.get(k, None)
+        if not isinstance(v, Mapping):
+            return v
+
+
 def merge_dict(change_dict: Dict[str, Any], orig_dict: MutableMapping[str, Any]):
     """
     Merge changes into the original dictionary. Leaf values in the change dict

acceptance/hidden_paths/test.py

@@ -61,16 +61,6 @@ def setup_prepare(self):
             "4": "172.20.0.65",
             "5": "172.20.0.73",
         }
-        # XXX(lukedirtwalker): The ports below are the dynamic QUIC server
-        # ports. Thanks to the docker setup they are setup consistently so we
-        # can use them. Optimally we would define a static server port inside
-        # the CS and use that one instead.
-        control_addresses = {
-            "2": "172.20.0.51:32768",
-            "3": "172.20.0.59:32768",
-            "4": "172.20.0.67:32768",
-            "5": "172.20.0.75:32768",
-        }
         # Each AS participating in hidden paths has their own hidden paths configuration file.
         hp_configs = {
             "2": "hp_groups_as2_as5.yml",
@@ -98,13 +88,16 @@ def setup_prepare(self):
             # even though some don't need the registration service.
             as_dir_path = self.artifacts / "gen" / ("ASff00_0_%s" % as_number)
 
+            # The hidden_segment services are behind the same server as the control_service.
+            topology_file = as_dir_path / "topology.json"
+            control_service_addr = scion.load_from_json(
+                'control_service.%s.addr' % control_id, [topology_file])
             topology_update = {
                 "hidden_segment_lookup_service.%s.addr" % control_id:
-                    control_addresses[as_number],
+                    control_service_addr,
                 "hidden_segment_registration_service.%s.addr" % control_id:
-                    control_addresses[as_number],
+                    control_service_addr,
             }
-            topology_file = as_dir_path / "topology.json"
             scion.update_json(topology_update, [topology_file])
 
     def setup_start(self):

control/cmd/control/main.go

@@ -200,11 +200,17 @@ func realMain(ctx context.Context) error {
 		return err
 	}
 
+	// FIXME: readability would be improved if we could be consistent with address
+	// representations in NetworkConfig (string or cooked, chose one).
 	nc := infraenv.NetworkConfig{
-		IA:                    topo.IA(),
+		IA: topo.IA(),
+		// Public: (Historical name) The TCP/IP:port address for the control service.
 		Public:                topo.ControlServiceAddress(globalCfg.General.ID),
 		ReconnectToDispatcher: globalCfg.General.ReconnectToDispatcher,
 		QUIC: infraenv.QUIC{
+			// Address: the QUIC/SCION address of this service. If not
+			// configured, QUICStack() uses the same IP and port as
+			// for the public address.
 			Address:     globalCfg.QUIC.Address,
 			TLSVerifier: trust.NewTLSCryptoVerifier(trustDB),
 			GetCertificate: cs.NewTLSCertificateLoader(
@@ -221,6 +227,7 @@ func realMain(ctx context.Context) error {
 		},
 		SCIONNetworkMetrics:    metrics.SCIONNetworkMetrics,
 		SCIONPacketConnMetrics: metrics.SCIONPacketConnMetrics,
+		MTU:                    topo.MTU(),
 	}
 	quicStack, err := nc.QUICStack()
 	if err != nil {

control/hiddenpaths.go

@@ -84,7 +84,8 @@ func (c HiddenPathConfigurator) Setup(location string) (*HiddenPathRegistrationC
 			&hpgrpc.AuthoritativeSegmentServer{
 				Lookup:   c.localAuthServer(groups),
 				Verifier: c.Verifier,
-			})
+			},
+		)
 		hspb.RegisterHiddenSegmentRegistrationServiceServer(c.InterASQUICServer,
 			&hpgrpc.RegistrationServer{
 				Registry: hiddenpath.RegistryServer{

control/tasks.go

@@ -196,6 +196,11 @@ func (t *TasksConfig) segmentWriter(segType seg.Type,
 		Writer:   writer,
 		Tick:     beaconing.NewTick(t.RegistrationInterval),
 	}
+	// The period of the task is short because we want to retry quickly
+	// if we fail fast. So during one interval we'll make as many attempts
+	// as we can until we succeed. After succeeding, the task does nothing
+	// until the end of the interval. The interval itself is used as a
+	// timeout. If we fail slow we give up at the end of the cycle.
 	return periodic.Start(r, 500*time.Millisecond, t.RegistrationInterval)
 }
 

pkg/experimental/hiddenpath/BUILD.bazel

@@ -25,6 +25,7 @@ go_library(
         "//pkg/private/serrors:go_default_library",
         "//pkg/segment:go_default_library",
         "//pkg/snet:go_default_library",
+        "//pkg/snet/path:go_default_library",
         "//private/config:go_default_library",
         "//private/pathdb:go_default_library",
         "//private/pathdb/query:go_default_library",

pkg/experimental/hiddenpath/discovery.go

@@ -21,6 +21,7 @@ import (
 	"github.com/scionproto/scion/pkg/addr"
 	"github.com/scionproto/scion/pkg/private/serrors"
 	"github.com/scionproto/scion/pkg/snet"
+	"github.com/scionproto/scion/pkg/snet/path"
 )
 
 // Servers is a list of discovered remote hidden segment server.
@@ -80,19 +81,22 @@ func (r LookupResolver) Resolve(ctx context.Context, ia addr.IA) (net.Addr, erro
 func resolve(ctx context.Context, ia addr.IA, discoverer Discoverer, router snet.Router,
 	extractAddr func(Servers) (*net.UDPAddr, error)) (net.Addr, error) {
 
-	path, err := router.Route(ctx, ia)
+	p, err := router.Route(ctx, ia)
 	if err != nil {
 		return nil, serrors.WrapStr("looking up path", err)
 	}
-	if path == nil {
+	if p == nil {
 		return nil, serrors.WrapStr("no path found to remote", err)
 	}
 	dsAddr := &snet.SVCAddr{
 		IA:      ia,
-		NextHop: path.UnderlayNextHop(),
-		Path:    path.Dataplane(),
+		NextHop: p.UnderlayNextHop(),
+		Path:    p.Dataplane(),
 		SVC:     addr.SvcDS,
 	}
+	if dsAddr.Path == nil {
+		dsAddr.Path = path.Empty{}
+	}
 	hps, err := discoverer.Discover(ctx, dsAddr)
 	if err != nil {
 		return nil, serrors.WrapStr("discovering hidden path server", err)
@@ -104,7 +108,7 @@ func resolve(ctx context.Context, ia addr.IA, discoverer Discoverer, router snet
 	return &snet.UDPAddr{
 		IA:      ia,
 		Host:    a,
-		NextHop: path.UnderlayNextHop(),
-		Path:    path.Dataplane(),
+		NextHop: dsAddr.NextHop,
+		Path:    dsAddr.Path,
 	}, nil
 }

pkg/snet/path.go

@@ -249,10 +249,6 @@ func (p *partialPath) Dataplane() DataplanePath {
 	return p.dataplane
 }
 
-func (p *partialPath) Interfaces() []PathInterface {
-	return nil
-}
-
 func (p *partialPath) Source() addr.IA {
 	return p.source
 }

pkg/snet/router.go

@@ -55,16 +55,3 @@ func (r *BaseRouter) Route(ctx context.Context, dst addr.IA) (Path, error) {
 func (r *BaseRouter) AllRoutes(ctx context.Context, dst addr.IA) ([]Path, error) {
 	return r.Querier.Query(ctx, dst)
 }
-
-// IntraASPathQuerier implements the PathQuerier interface. It will only provide
-// AS internal paths, i.e., empty paths with only the IA as destination. This
-// should only be used in places where you know that you only need to
-// communicate inside the AS.
-type IntraASPathQuerier struct {
-	IA addr.IA
-}
-
-// Query implements PathQuerier.
-func (q IntraASPathQuerier) Query(_ context.Context, _ addr.IA) ([]Path, error) {
-	return []Path{&partialPath{source: q.IA, destination: q.IA}}, nil
-}

private/app/appnet/BUILD.bazel

@@ -5,6 +5,7 @@ go_library(
     srcs = [
         "addr.go",
         "infraenv.go",
+        "intraAS.go",
     ],
     importpath = "github.com/scionproto/scion/private/app/appnet",
     visibility = ["//visibility:public"],
@@ -13,6 +14,7 @@ go_library(
         "//pkg/daemon:go_default_library",
         "//pkg/log:go_default_library",
         "//pkg/private/serrors:go_default_library",
+        "//pkg/slayers/path:go_default_library",
         "//pkg/snet:go_default_library",
         "//pkg/snet/path:go_default_library",
         "//pkg/snet/squic:go_default_library",

private/app/appnet/addr.go

@@ -161,6 +161,7 @@ func (r AddressRewriter) buildFullAddress(ctx context.Context,
 			return nil, serrors.WrapStr("Unable to resolve underlay", err)
 		}
 		ret.NextHop = ov
+		ret.Path = path.Empty{}
 	}
 
 	return ret, nil

private/app/appnet/addr_test.go

@@ -80,8 +80,8 @@ func TestRedirectQUIC(t *testing.T) {
 
 			a, r, err := aw.RedirectToQUIC(context.Background(), tc.input)
 			tc.assertErr(t, err)
-			assert.Equal(t, a, tc.wantAddr)
-			assert.Equal(t, r, tc.wantRedirect)
+			assert.Equal(t, tc.wantAddr, a)
+			assert.Equal(t, tc.wantRedirect, r)
 		})
 	}
 
@@ -100,7 +100,6 @@ func TestRedirectQUIC(t *testing.T) {
 		path.EXPECT().Metadata().Return(&snet.PathMetadata{
 			Interfaces: make([]snet.PathInterface, 1), // just non-empty
 		})
-
 		aw := infraenv.AddressRewriter{
 			Router:                router,
 			Resolver:              resolver,
@@ -137,7 +136,6 @@ func TestRedirectQUIC(t *testing.T) {
 		path.EXPECT().Metadata().Return(&snet.PathMetadata{
 			Interfaces: make([]snet.PathInterface, 1), // just non-empty
 		})
-
 		aw := infraenv.AddressRewriter{
 			Router:                router,
 			Resolver:              resolver,
@@ -182,7 +180,7 @@ func TestRedirectQUIC(t *testing.T) {
 		want := &snet.SVCAddr{
 			SVC:     addr.SvcCS,
 			NextHop: &net.UDPAddr{IP: net.ParseIP("10.1.1.1")},
-			Path:    snetpath.SCION{},
+			Path:    snetpath.Empty{},
 		}
 		a, r, err := aw.RedirectToQUIC(context.Background(), input)
 		assert.NoError(t, err)
@@ -225,7 +223,7 @@ func TestBuildFullAddress(t *testing.T) {
 			SVC:  addr.SvcCS,
 		}
 		a, err := aw.BuildFullAddress(context.Background(), input)
-		assert.Equal(t, a, input)
+		assert.Equal(t, input, a)
 		assert.NoError(t, err)
 	})
 
@@ -255,7 +253,7 @@ func TestBuildFullAddress(t *testing.T) {
 			NextHop: &net.UDPAddr{},
 			SVC:     addr.SvcCS,
 		}
-		assert.Equal(t, a, want)
+		assert.Equal(t, want, a)
 		assert.NoError(t, err)
 	})
 
@@ -285,8 +283,13 @@ func TestBuildFullAddress(t *testing.T) {
 		input := &snet.SVCAddr{IA: localIA, SVC: addr.SvcCS, Path: snetpath.Empty{}}
 		a, err := aw.BuildFullAddress(context.Background(), input)
 
-		want := &snet.SVCAddr{IA: localIA, NextHop: underlayAddr, SVC: addr.SvcCS}
-		assert.Equal(t, a, want)
+		want := &snet.SVCAddr{
+			IA:      localIA,
+			NextHop: underlayAddr,
+			SVC:     addr.SvcCS,
+			Path:    snetpath.Empty{},
+		}
+		assert.Equal(t, want, a)
 		assert.NoError(t, err)
 	})
 
@@ -388,9 +391,9 @@ func TestResolve(t *testing.T) {
 			}
 			initResolver(resolver, tc.ResolverSetup)
 			p, a, redirect, err := aw.ResolveSVC(context.Background(), path, tc.input)
-			assert.Equal(t, p, tc.wantPath)
-			assert.Equal(t, a.String(), tc.want.String())
-			assert.Equal(t, redirect, tc.wantQUICRedirect)
+			assert.Equal(t, tc.wantPath, p)
+			assert.Equal(t, tc.want.String(), a.String())
+			assert.Equal(t, tc.wantQUICRedirect, redirect)
 			tc.assertErr(t, err)
 		})
 	}
@@ -448,7 +451,7 @@ func TestParseReply(t *testing.T) {
 			if err != nil {
 				return
 			}
-			assert.Equal(t, a.String(), tc.want.String())
+			assert.Equal(t, tc.want.String(), a.String())
 		})
 	}
 }

private/app/appnet/infraenv.go

@@ -83,6 +83,8 @@ type NetworkConfig struct {
 	SCIONNetworkMetrics snet.SCIONNetworkMetrics
 	// Metrics injected into DefaultPacketDispatcherService.
 	SCIONPacketConnMetrics snet.SCIONPacketConnMetrics
+	// MTU of the local AS
+	MTU uint16
 }
 
 // QUICStack contains everything to run a QUIC based RPC stack.
@@ -103,8 +105,9 @@ func (nc *NetworkConfig) TCPStack() (net.Listener, error) {
 
 func (nc *NetworkConfig) QUICStack() (*QUICStack, error) {
 	if nc.QUIC.Address == "" {
-		nc.QUIC.Address = net.JoinHostPort(nc.Public.IP.String(), "0")
+		nc.QUIC.Address = nc.Public.String()
 	}
+
 	client, server, err := nc.initQUICSockets()
 	if err != nil {
 		return nil, err
@@ -221,7 +224,7 @@ func (nc *NetworkConfig) AddressRewriter(
 		}
 	}
 	return &AddressRewriter{
-		Router:    &snet.BaseRouter{Querier: snet.IntraASPathQuerier{IA: nc.IA}},
+		Router:    &snet.BaseRouter{Querier: IntraASPathQuerier{IA: nc.IA, MTU: nc.MTU}},
 		SVCRouter: nc.SVCResolver,
 		Resolver: &svc.Resolver{
 			LocalIA:     nc.IA,
@@ -266,9 +269,15 @@ func (nc *NetworkConfig) initSvcRedirect(quicAddress string) (func(), error) {
 		Dispatcher: packetDispatcher,
 		Metrics:    nc.SCIONNetworkMetrics,
 	}
-	conn, err := network.Listen(context.Background(), "udp", nc.Public, addr.SvcWildcard)
+
+	// The service resolution address gets a dynamic port. In reality, neither the
+	// address nor the port are needed to address the resolver, but the dispatcher still
+	// requires them and checks unicity. At least a dynamic port is allowed.
+	srAddr := &net.UDPAddr{IP: nc.Public.IP, Port: 0}
+	conn, err := network.Listen(context.Background(), "udp", srAddr, addr.SvcWildcard)
 	if err != nil {
-		return nil, serrors.WrapStr("listening on SCION", err, "addr", nc.Public)
+		log.Info("Listen failed", "err", err)
+		return nil, serrors.WrapStr("listening on SCION", err, "addr", srAddr)
 	}
 
 	ctx, cancel := context.WithCancel(context.Background())