Merge pull request #2249 from oasislabs/yawning/feature/reject-unrout…

…able

registry: Add `registry.debug.allow_unroutable_addresses`

go/common/address.go

@@ -71,6 +71,9 @@ func FindAllAddresses() ([]net.IP, error) {
 			case *net.IPNet:
 				ip = a.IP
 			}
+			if !ip.IsGlobalUnicast() {
+				continue
+			}
 
 			if ip != nil {
 				addresses = append(addresses, ip)

go/common/node/address.go

@@ -14,6 +14,8 @@ var (
 	// invalid.
 	ErrInvalidAddress = errors.New("node: invalid transport address")
 
+	unroutableNetworks []net.IPNet
+
 	_ encoding.TextMarshaler   = (*Address)(nil)
 	_ encoding.TextUnmarshaler = (*Address)(nil)
 )
@@ -56,6 +58,16 @@ func (a *Address) FromIP(ip net.IP, port uint16) error {
 	return nil
 }
 
+// IsRoutable returns true iff the address is likely to be globally routable.
+func (a *Address) IsRoutable() bool {
+	for _, v := range unroutableNetworks {
+		if v.Contains(a.IP) {
+			return false
+		}
+	}
+	return true
+}
+
 // ToProtoAddresses converts a list of Addresses to protocol buffers.
 func ToProtoAddresses(addrs []Address) []*pbCommon.Address {
 	var pbAddrs []*pbCommon.Address
@@ -110,3 +122,42 @@ func toProtoAddress(addr Address) *pbCommon.Address {
 
 	return pbAddr
 }
+
+func init() {
+	// List taken from RFC 6890.  This is different from what tendermint
+	// does (more restrictive).
+	for _, v := range []string{
+		"0.0.0.0/8",          // RFC 1122
+		"10.0.0.0/8",         // RFC 1918: Private-Use
+		"100.64.0.0/10",      // RFC 6598: Shared Address Space
+		"127.0.0.0/8",        // RFC 1122: Loopback
+		"169.254.0.0/16",     // RFC 3927: Link Local
+		"172.16.0.0/12",      // RFC 1918: Private-Use
+		"192.0.0.0/24",       // RFC 6890
+		"192.0.0.0/29",       // RFC 6333: DS-Lite
+		"192.0.2.0/24",       // RFC 5737: Documentation (TEST-NET-1)
+		"192.168.0.0/16",     // RFC 1918: Private-Use
+		"192.18.0.0/15",      // RFC 2544: Benchmarking
+		"198.51.100.0/24",    // RFC 5737: TEST-NET-2
+		"203.0.113.0/24",     // RFC 5737: TEST-NET-3
+		"240.0.0.0/4",        // RFC 1112: Reserved
+		"255.255.255.255/32", // RFC 919: Limited Broadcast
+		"::1/128",            // RFC 4291: Loopback Address
+		"::/128",             // RFC 4291: Unspecified Address
+		"::ffff:0:0/96",      // RFC 4291: IPv4-mapped Address
+		"100::/64",           // RFC 6666: Discard-Only Address Block
+		"2001::/32",          // RFC 4380: TEREDO
+		"2001:2::/48",        // RFC 5180: Benchmarking
+		"2001:db8::/32",      // RFC 3849: Documentation
+		"2001:10::/28",       // RFC 4843: ORCHID
+		"2002::/16",          // RFC 3056: 6to4
+		"fc00::/7",           // RFC 4193: Unique-Local
+		"fe80::/10",          // RFC 4291: Linked-Scoped Unicast
+	} {
+		_, ipNet, err := net.ParseCIDR(v)
+		if err != nil {
+			panic("node: failed to parse reserved net: " + err.Error())
+		}
+		unroutableNetworks = append(unroutableNetworks, *ipNet)
+	}
+}

go/oasis-node/cmd/debug/byzantine/p2p.go

@@ -85,3 +85,7 @@ func (ph *p2pHandle) stop() error {
 
 	return nil
 }
+
+func init() {
+	p2p.DebugForceAllowUnroutableAddresses()
+}

go/oasis-node/cmd/debug/byzantine/tendermint.go

@@ -118,6 +118,7 @@ func (ht *honestTendermint) start(id *identity.Identity, dataDir string, useMock
 		return errors.Wrap(err, "honest Tendermint service RegisterApplication staking")
 	}
 	if err := ht.service.RegisterApplication(registryapp.New(timeSource, &registry.Config{
+		DebugAllowUnroutableAddresses: true,
 		DebugAllowRuntimeRegistration: false,
 		DebugBypassStake:              false,
 	})); err != nil {

go/oasis-node/node_test.go

@@ -53,6 +53,7 @@ var (
 		{"epochtime.backend", "tendermint_mock"},
 		{"consensus.backend", "tendermint"},
 		{"consensus.validator", true},
+		{"registry.debug.allow_unroutable_addresses", true},
 		{"registry.debug.allow_runtime_registration", true},
 		{"registry.debug.bypass_stake", true},
 		{"roothash.tendermint.index_blocks", true},

go/oasis-test-runner/oasis/args.go

@@ -17,6 +17,7 @@ import (
 	"github.com/oasislabs/oasis-core/go/oasis-node/cmd/common/flags"
 	"github.com/oasislabs/oasis-core/go/oasis-node/cmd/common/grpc"
 	"github.com/oasislabs/oasis-core/go/oasis-node/cmd/debug/byzantine"
+	"github.com/oasislabs/oasis-core/go/registry"
 	roothashTm "github.com/oasislabs/oasis-core/go/roothash/tendermint"
 	"github.com/oasislabs/oasis-core/go/storage"
 	"github.com/oasislabs/oasis-core/go/tendermint"
@@ -112,6 +113,11 @@ func (args *argBuilder) beaconDeterministic(deterministic bool) *argBuilder {
 	return args
 }
 
+func (args *argBuilder) registryDebugAllowUnroutableAddresses() *argBuilder {
+	args.vec = append(args.vec, "--"+registry.CfgDebugAllowUnroutableAddresses)
+	return args
+}
+
 func (args *argBuilder) roothashTendermintIndexBlocks() *argBuilder {
 	args.vec = append(args.vec, "--"+roothashTm.CfgIndexBlocks)
 	return args

go/oasis-test-runner/oasis/oasis.go

@@ -377,6 +377,7 @@ func (net *Network) startOasisNode(
 	if len(subCmd) == 0 {
 		extraArgs = extraArgs.
 			appendIASProxy(net.iasProxy).
+			registryDebugAllowUnroutableAddresses().
 			tendermintDebugAddrBookLenient()
 	}
 	args := append([]string{}, subCmd...)

go/registry/api/api.go

@@ -285,7 +285,7 @@ func VerifyDeregisterEntityArgs(logger *logging.Logger, sigTimestamp *signature.
 }
 
 // VerifyRegisterNodeArgs verifies arguments for RegisterNode.
-func VerifyRegisterNodeArgs(logger *logging.Logger, sigNode *node.SignedNode, entity *entity.Entity, now time.Time, isGenesis bool, kmOperator signature.PublicKey, regRuntimes []*Runtime) (*node.Node, error) {
+func VerifyRegisterNodeArgs(cfg *Config, logger *logging.Logger, sigNode *node.SignedNode, entity *entity.Entity, now time.Time, isGenesis bool, kmOperator signature.PublicKey, regRuntimes []*Runtime) (*node.Node, error) {
 	var n node.Node
 	if sigNode == nil {
 		return nil, ErrInvalidArgument
@@ -397,12 +397,13 @@ func VerifyRegisterNodeArgs(logger *logging.Logger, sigNode *node.SignedNode, en
 
 	// If node is a validator, ensure it has ConensusInfo.
 	if n.HasRoles(node.RoleValidator) {
-		// Verify that addresses are non-empty.
-		if len(n.Consensus.Addresses) == 0 {
-			logger.Error("RegisterNode: missing consensus addresses",
+		if err := verifyAddresses(cfg, n.Consensus.Addresses); err != nil {
+			addrs, _ := json.Marshal(n.Consensus.Addresses)
+			logger.Error("RegisterNode: missing/invalid consensus addresses",
 				"node", n,
+				"consensus_addrs", addrs,
 			)
-			return nil, ErrInvalidArgument
+			return nil, err
 		}
 	}
 
@@ -419,12 +420,21 @@ func VerifyRegisterNodeArgs(logger *logging.Logger, sigNode *node.SignedNode, en
 
 	// If node is a worker, ensure it has CommitteeInfo and P2PInfo.
 	if n.HasRoles(node.RoleComputeWorker | node.RoleStorageWorker | node.RoleTransactionScheduler | node.RoleKeyManager | node.RoleMergeWorker) {
-		// Verify that addresses are non-empty.
-		if len(n.Committee.Addresses) == 0 || len(n.P2P.Addresses) == 0 {
-			logger.Error("RegisterNode: missing committee or p2p addresses",
+		if err := verifyAddresses(cfg, n.Committee.Addresses); err != nil {
+			addrs, _ := json.Marshal(n.Committee.Addresses)
+			logger.Error("RegisterNode: missing/invalid committee addresses",
 				"node", n,
+				"committee_addrs", addrs,
 			)
-			return nil, ErrInvalidArgument
+			return nil, err
+		}
+		if err := verifyAddresses(cfg, n.P2P.Addresses); err != nil {
+			addrs, _ := json.Marshal(n.P2P.Addresses)
+			logger.Error("RegisterNode: missing/invald P2P addresses",
+				"node", n,
+				"p2p_addrs", addrs,
+			)
+			return nil, err
 		}
 
 		// Verify that certificate is well-formed.
@@ -534,6 +544,37 @@ func VerifyNodeRuntimeEnclaveIDs(logger *logging.Logger, rt *node.Runtime, regRu
 	return nil
 }
 
+// VerifyAddress verifies a node address.
+func VerifyAddress(addr node.Address, allowUnroutable bool) error {
+	if !allowUnroutable {
+		// Use the runtime to reject clearly invalid addresses.
+		if !addr.IP.IsGlobalUnicast() {
+			return ErrInvalidArgument
+		}
+
+		if !addr.IsRoutable() {
+			return ErrInvalidArgument
+		}
+	}
+
+	return nil
+}
+
+func verifyAddresses(cfg *Config, addrs []node.Address) error {
+	// Treat having no addresses as invalid, regardless.
+	if len(addrs) == 0 {
+		return ErrInvalidArgument
+	}
+
+	for _, v := range addrs {
+		if err := VerifyAddress(v, cfg.DebugAllowUnroutableAddresses); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 // sortRuntimeList sorts the given runtime list to ensure a canonical order.
 func sortRuntimeList(runtimes []*node.Runtime) {
 	sort.Slice(runtimes, func(i, j int) bool {
@@ -761,6 +802,10 @@ type Genesis struct {
 
 // Config is the per-backend common configuration.
 type Config struct {
+	// DebugAllowUnroutableAddresses is true iff node registration should
+	// allow unroutable addreses.
+	DebugAllowUnroutableAddresses bool
+
 	// DebugAllowRuntimeRegistration is true iff runtime registration should be
 	// allowed outside of the genesis block.
 	DebugAllowRuntimeRegistration bool

go/registry/init.go

@@ -17,6 +17,10 @@ import (
 )
 
 const (
+	// CfgDebugAllowUnroutableAddresses allows unroutable addresses in node
+	// registration.
+	CfgDebugAllowUnroutableAddresses = "registry.debug.allow_unroutable_addresses"
+
 	cfgDebugAllowRuntimeRegistration = "registry.debug.allow_runtime_registration"
 	cfgDebugBypassStake              = "registry.debug.bypass_stake" // nolint: gosec
 )
@@ -46,12 +50,14 @@ func New(ctx context.Context, timeSource epochtime.Backend, tmService service.Te
 
 func flagsToConfig() *api.Config {
 	return &api.Config{
+		DebugAllowUnroutableAddresses: viper.GetBool(CfgDebugAllowUnroutableAddresses),
 		DebugAllowRuntimeRegistration: viper.GetBool(cfgDebugAllowRuntimeRegistration),
 		DebugBypassStake:              viper.GetBool(cfgDebugBypassStake),
 	}
 }
 
 func init() {
+	Flags.Bool(CfgDebugAllowUnroutableAddresses, false, "allow unroutable addreses (UNSAFE)")
 	Flags.Bool(cfgDebugAllowRuntimeRegistration, false, "enable non-genesis runtime registration (UNSAFE)")
 	Flags.Bool(cfgDebugBypassStake, false, "bypass all stake checks and operations (UNSAFE)")
 

go/tendermint/apps/registry/registry.go

@@ -473,7 +473,7 @@ func (app *registryApplication) registerNode(
 		)
 		return err
 	}
-	newNode, err := registry.VerifyRegisterNodeArgs(app.logger, sigNode, untrustedEntity, ctx.Now(), ctx.IsInitChain(), kmOperator, regRuntimes)
+	newNode, err := registry.VerifyRegisterNodeArgs(app.cfg, app.logger, sigNode, untrustedEntity, ctx.Now(), ctx.IsInitChain(), kmOperator, regRuntimes)
 	if err != nil {
 		return err
 	}

go/worker/common/p2p/p2p.go

@@ -22,10 +22,23 @@ import (
 	"github.com/oasislabs/oasis-core/go/common/logging"
 	"github.com/oasislabs/oasis-core/go/common/node"
 	"github.com/oasislabs/oasis-core/go/common/version"
+	registry "github.com/oasislabs/oasis-core/go/registry"
+	registryAPI "github.com/oasislabs/oasis-core/go/registry/api"
 	"github.com/oasislabs/oasis-core/go/worker/common/configparser"
 )
 
-var protocolName = core.ProtocolID("/p2p/oasislabs.com/committee/" + version.CommitteeProtocol.String())
+var (
+	protocolName = core.ProtocolID("/p2p/oasislabs.com/committee/" + version.CommitteeProtocol.String())
+
+	forceAllowUnroutableAddresses bool
+)
+
+// DebugForceAllowUnroutableAddresses exists entirely for the benefit
+// of the byzantine node, which doesn't use viper properly to configure
+// various subcomponent behavior.
+func DebugForceAllowUnroutableAddresses() {
+	forceAllowUnroutableAddresses = true
+}
 
 // Handler is a handler for P2P messages.
 type Handler interface {
@@ -87,14 +100,24 @@ func (p *P2P) Info() node.P2PInfo {
 		addrs = p.registerAddresses
 	}
 
+	allowUnroutable := viper.GetBool(registry.CfgDebugAllowUnroutableAddresses)
+	if forceAllowUnroutableAddresses {
+		allowUnroutable = forceAllowUnroutableAddresses
+	}
+
 	var addresses []node.Address
 	for _, v := range addrs {
 		netAddr, err := manet.ToNetAddr(v)
 		if err != nil {
 			panic(err)
 		}
 		tcpAddr := (netAddr).(*net.TCPAddr)
-		addresses = append(addresses, node.Address{TCPAddr: *tcpAddr})
+		nodeAddr := node.Address{TCPAddr: *tcpAddr}
+		if err := registryAPI.VerifyAddress(nodeAddr, allowUnroutable); err != nil {
+			continue
+		}
+
+		addresses = append(addresses, nodeAddr)
 	}
 
 	id, err := peerIDToPublicKey(p.host.ID())