Move port allocation in network vat to PortAllocator (#9228)

<!-- < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < ☺
v                               ✰  Thanks for creating a PR! ✰
☺ > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >  -->

<!-- Most PRs should close a specific Issue. All PRs should at least
reference one or more Issues. Edit and/or delete the following lines as
appropriate (note: you don't need both `refs` and `closes` for the same
one): -->

closes: #9165

## Description

<!-- Add a description of the changes that this PR introduces and the
files that
are the most critical to review.
-->

`bindPort` gives a caller too much power to specify exactly what port
they want allocated. With this change, we create a central object,
`PortAllocator` that handles handing out ports to the caller. As of
today, we are allocating ports for:
1. `/ibc-port`
2. `/ibc-port/icacontroller`
3. `/ibc-port/pegasus`

### Security Considerations

<!-- Does this change introduce new assumptions or dependencies that, if
violated, could introduce security vulnerabilities? How does this PR
change the boundaries between mutually-suspicious components? What new
authorities are introduced by this change, perhaps by new API calls?
-->

This will prevent `squatting`, where callers can come and claim certain
ports that they have no association with

### Documentation Considerations

<!-- Give our docs folks some hints about what needs to be described to
downstream users.

Backwards compatibility: what happens to existing data or deployments
when this code is shipped? Do we need to instruct users to do something
to upgrade their saved data? If there is no upgrade path possible, how
bad will that be for users?

-->

We've removed the `bindPort` method from the network vat. All the
contracts inside of `agoric-sdk` have been changed to use
`portAllocator`

### Testing Considerations

<!-- Every PR should of course come with tests of its own functionality.
What additional tests are still needed beyond those unit tests? How does
this affect CI, other test automation, or the testnet?
-->

I've included some unit test testing the new allocate APIs

---------

Co-authored-by: Michael FIG <mfig@agoric.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

packages/boot/test/bootstrapTests/ibcClientMock.js

@@ -6,14 +6,14 @@ import { V as E } from '@agoric/vat-data/vow.js';
 /**
  * @param {ZCF} zcf
  * @param {{
- *   address: string,
- *   networkVat: ERef<ReturnType<typeof import('@agoric/vats/src/vat-network').buildRootObject>>;
+ *   portAllocator: ERef<PortAllocator>;
  * }} privateArgs
  * @param {import("@agoric/vat-data").Baggage} _baggage
  */
 export const start = async (zcf, privateArgs, _baggage) => {
-  const { address, networkVat } = privateArgs;
-  const myPort = await E(networkVat).bindPort(address);
+  const { portAllocator } = privateArgs;
+
+  const myPort = await E(portAllocator).allocateCustomIBCPort();
 
   const { log } = console;
   let connP;

packages/boot/test/bootstrapTests/ibcServerMock.js

@@ -12,14 +12,14 @@ const { log } = console;
  * @param {ZCF} zcf
  * @param {{
  *   address: string,
- *   networkVat: ERef<ReturnType<typeof import('@agoric/vats/src/vat-network').buildRootObject>>;
+ *   portAllocator: ERef<PortAllocator>;
  * }} privateArgs
  * @param {import("@agoric/vat-data").Baggage} _baggage
  */
 export const start = async (zcf, privateArgs, _baggage) => {
-  const { address, networkVat } = privateArgs;
+  const { portAllocator } = privateArgs;
 
-  const boundPort = await E(networkVat).bindPort(address);
+  const boundPort = await E(portAllocator).allocateCustomIBCPort();
 
   /** @type {Array<[label: string, resolve: (value: any) => void, reject: (reason: any) => void]>} */
   const queue = [];

packages/boot/test/bootstrapTests/test-net-ibc-upgrade.ts

@@ -113,7 +113,7 @@ const upgradeVats = async (t, EV, vatsToUpgrade) => {
 test.serial('upgrade at many points in network API flow', async t => {
   const { installation } = t.context;
   const { EV } = t.context.runUtils;
-  const networkVat = await EV.vat('bootstrap').consumeItem('networkVat');
+  const portAllocator = await EV.vat('bootstrap').consumeItem('portAllocator');
   const zoe: ZoeService = await EV.vat('bootstrap').consumeItem('zoe');
 
   const flow = entries({
@@ -122,7 +122,7 @@ test.serial('upgrade at many points in network API flow', async t => {
         installation.ibcServerMock,
         {},
         {},
-        { address: '/ibc-port/', networkVat },
+        { portAllocator },
       );
       t.truthy(started.creatorFacet, `${label} ibcServerMock`);
       return [label, { server: started.creatorFacet }];
@@ -140,7 +140,7 @@ test.serial('upgrade at many points in network API flow', async t => {
         installation.ibcClientMock,
         {},
         {},
-        { address: '/ibc-port/', networkVat },
+        { portAllocator },
       );
       t.truthy(started.creatorFacet, `${label} ibcClientMock`);
       return [label, { ...opts, client: started.creatorFacet }];

packages/boot/test/bootstrapTests/test-vats-restart.ts

@@ -105,20 +105,6 @@ test.serial('run network vat proposal', async t => {
   t.pass(); // reached here without throws
 });
 
-test.serial('register network protocol before upgrade', async t => {
-  const { EV } = t.context.runUtils;
-  const net = await EV.vat('bootstrap').consumeItem('networkVat');
-  const h1 = await EV(net).makeLoopbackProtocolHandler();
-
-  t.log('register P1');
-  await EV(net).registerProtocolHandler(['P1'], h1);
-
-  t.log('register P1 again? No.');
-  await t.throwsAsync(EV(net).registerProtocolHandler(['P1'], h1), {
-    message: /key "P1" already registered/,
-  });
-});
-
 test.serial('make IBC callbacks before upgrade', async t => {
   const { EV } = t.context.runUtils;
   const vatStore = await EV.vat('bootstrap').consumeItem('vatStore');
@@ -155,25 +141,6 @@ test.serial('use IBC callbacks after upgrade', async t => {
   t.truthy(h.bridgeHandler, 'bridgeHandler');
 });
 
-test.serial('networkVat registrations are durable', async t => {
-  const { EV } = t.context.runUtils;
-  const net = await EV.vat('bootstrap').consumeItem('networkVat');
-
-  const h2 = await EV(net).makeLoopbackProtocolHandler();
-  t.log('register P1 again? No.');
-  await t.throwsAsync(EV(net).registerProtocolHandler(['P1'], h2), {
-    message: /key "P1" already registered/,
-  });
-
-  t.log('IBC protocol handler already registered?');
-  await t.throwsAsync(
-    EV(net).registerProtocolHandler(['/ibc-port', '/ibc-hop'], h2),
-    {
-      message: /key "\/ibc-port" already registered in collection "prefix"/,
-    },
-  );
-});
-
 test.serial('read metrics', async t => {
   const { EV } = t.context.runUtils;
 

packages/inter-protocol/test/smartWallet/boot-test-utils.js

@@ -58,7 +58,6 @@ export const makeMock = log =>
 
       network: Far('network', {
         registerProtocolHandler: noop,
-        bindPort: () => harden({ addListener: noop }),
       }),
     },
   });

packages/network/README.md

@@ -57,15 +57,12 @@ E(home.ibcport[0]).connect(remoteEndpoint, connectionHandler)
 
 The other side of `connect()` is a "listening port". These ports are waiting for inbound connections to be established.
 
-To get a listening port, you need a `NetworkInterface` object (such as the one on your `ag-solo` under `home.network`) and ask it to `bindPort()` to an endpoint. You can either provide a specific port name, or allow the API to allocate a random one for you. The endpoint specifies the type of connection that this port will be able to accept (IBC, TCP, etc), and some properties of that connection. `bindPort()` uses a "multiaddress" to encode this information.
+To get a listening port, you need a `NetworkInterface` object (such as the one on your `ag-solo` under `home.network`) and ask it for a port, via the `PortAllocator`.
 
 ```js
 // ask for a random allocation - ends with a slash
-E(home.network).bindPort('/ibc-port/')
-  .then(port => usePort(port));
-
-// or ask for a specific port name
-E(home.network).bindPort('/ibc-port/my-cool-port-name')
+E(home.network).getPortAllocator()
+  .then(portAllocator => E(portAllocator).allocateCustomIBCPort())
   .then(port => usePort(port));
 ```
 
@@ -147,7 +144,7 @@ Note that if you want to listen on this port again, you can just call `port.addL
 
 ### Closing the Port Entirely
 
-Removing a listener doesn't release the port address to make it available for other `bindPort()` requests.  You can call:
+Removing a listener doesn't release the port address to make it available for other `PortAllocator` requests.  You can call:
 
 ```js
 port.revoke();

packages/network/src/network.js

@@ -46,6 +46,20 @@ export function getPrefixes(addr) {
   return ret;
 }
 
+/**
+ * Validate IBC port name
+ * @param {string} specifiedName
+ */
+function throwIfInvalidPortName(specifiedName) {
+  // Contains between 2 and 128 characters
+  // Can contain alphanumeric characters
+  // Valid symbols: ., ,, _, +, -, #, [, ], <, >
+  const portNameRegex = new RegExp('^[a-zA-Z0-9.,_+\\-#<>\\[\\]]{2,128}$');
+  if (!portNameRegex.test(specifiedName)) {
+    throw new Error(`Invalid IBC port name: ${specifiedName}`);
+  }
+}
+
 /**
  * @typedef {object} ConnectionOpts
  * @property {Endpoint[]} addrs
@@ -1426,3 +1440,61 @@ export function prepareLoopbackProtocolHandler(zone, { watch, allVows }) {
 
   return makeLoopbackProtocolHandler;
 }
+
+/**
+ *
+ * @param {import('@agoric/base-zone').Zone} zone
+ * @param {ReturnType<import('@agoric/vow').prepareVowTools>} powers
+ */
+export const preparePortAllocator = (zone, { watch }) =>
+  zone.exoClass(
+    'PortAllocator',
+    M.interface('PortAllocator', {
+      allocateCustomIBCPort: M.callWhen()
+        .optional(M.string())
+        .returns(Shape.Vow$(Shape.Port)),
+      allocateICAControllerPort: M.callWhen().returns(Shape.Vow$(Shape.Port)),
+      allocateCustomLocalPort: M.callWhen()
+        .optional(M.string())
+        .returns(Shape.Vow$(Shape.Port)),
+    }),
+    ({ protocol }) => ({ protocol, lastICAPortNum: 0n }),
+    {
+      allocateCustomIBCPort(specifiedName = '') {
+        const { state } = this;
+        let localAddr = `/ibc-port/`;
+
+        if (specifiedName) {
+          throwIfInvalidPortName(specifiedName);
+
+          localAddr = `/ibc-port/custom-${specifiedName}`;
+        }
+
+        // Allocate an IBC port with a unique generated name.
+        return watch(E(state.protocol).bindPort(localAddr));
+      },
+      allocateICAControllerPort() {
+        const { state } = this;
+        state.lastICAPortNum += 1n;
+        return watch(
+          E(state.protocol).bindPort(
+            `/ibc-port/icacontroller-${state.lastICAPortNum}`,
+          ),
+        );
+      },
+      allocateCustomLocalPort(specifiedName = '') {
+        const { state } = this;
+
+        let localAddr = `/local/`;
+
+        if (specifiedName) {
+          throwIfInvalidPortName(specifiedName);
+
+          localAddr = `/local/custom-${specifiedName}`;
+        }
+
+        // Allocate a local port with a unique generated name.
+        return watch(E(state.protocol).bindPort(localAddr));
+      },
+    },
+  );

packages/network/src/types.js

@@ -188,3 +188,5 @@
  * ) => PromiseVow<Connection>} outbound
  *   Create an outbound connection
  */
+
+/** @typedef {ReturnType<ReturnType<typeof import('@agoric/network').preparePortAllocator>>} PortAllocator */

packages/network/test/test-network-misc.js

@@ -14,6 +14,7 @@ import {
   prepareRouter,
   prepareLoopbackProtocolHandler,
   prepareNetworkProtocol,
+  preparePortAllocator,
 } from '../src/index.js';
 
 import '../src/types.js';
@@ -53,7 +54,7 @@ const prepareProtocolHandler = (
       },
       async generatePortID() {
         this.state.nonce += 1;
-        return `${this.state.nonce}`;
+        return `port-${this.state.nonce}`;
       },
       async onBind(port, localAddr) {
         t.assert(port, `port is supplied to onBind`);
@@ -167,6 +168,55 @@ test('handled protocol', async t => {
   await when(port.revoke());
 });
 
+test('verify port allocation', async t => {
+  const zone = makeDurableZone(
+    provideBaggage('network-verify-port-allocation'),
+  );
+  const powers = prepareVowTools(zone);
+  const { when } = powers;
+  const makeNetworkProtocol = prepareNetworkProtocol(zone, powers);
+  const makeEchoConnectionHandler = prepareEchoConnectionKit(zone);
+  const makeProtocolHandler = prepareProtocolHandler(
+    zone,
+    t,
+    makeEchoConnectionHandler,
+    powers,
+  );
+  const protocol = makeNetworkProtocol(makeProtocolHandler());
+  const makePortAllocator = preparePortAllocator(zone, powers);
+  const portAllocator = makePortAllocator({ protocol });
+
+  const ibcPort = await when(portAllocator.allocateCustomIBCPort());
+  t.is(ibcPort.getLocalAddress(), '/ibc-port/port-1');
+
+  const namedIbcPort = await when(
+    portAllocator.allocateCustomIBCPort('test-1'),
+  );
+  t.is(namedIbcPort.getLocalAddress(), '/ibc-port/custom-test-1');
+
+  const icaControllerPort1 = await when(
+    portAllocator.allocateICAControllerPort(),
+  );
+  t.is(icaControllerPort1.getLocalAddress(), '/ibc-port/icacontroller-1');
+
+  const icaControllerPort2 = await when(
+    portAllocator.allocateICAControllerPort(),
+  );
+  t.is(icaControllerPort2.getLocalAddress(), '/ibc-port/icacontroller-2');
+
+  const localPort = await when(portAllocator.allocateCustomLocalPort());
+  t.is(localPort.getLocalAddress(), '/local/port-5');
+
+  const namedLocalPort = await when(
+    portAllocator.allocateCustomLocalPort('local-1'),
+  );
+  t.is(namedLocalPort.getLocalAddress(), '/local/custom-local-1');
+
+  await t.throwsAsync(when(portAllocator.allocateCustomIBCPort('/test-1')), {
+    message: 'Invalid IBC port name: /test-1',
+  });
+});
+
 test('protocol connection listen', async t => {
   const zone = makeDurableZone(provideBaggage('network-protocol-connection'));
   const powers = prepareVowTools(zone);

packages/orchestration/src/proposals/orchestration-proposal.js

@@ -1,9 +1,7 @@
 // @ts-check
 import { V as E } from '@agoric/vat-data/vow.js';
-import { Far } from '@endo/far';
 
 /**
- * @import { AttenuatedNetwork } from '../types'
  * @import { OrchestrationService } from '../service.js'
  * @import { OrchestrationVat } from '../vat-orchestration.js'
  */
@@ -12,7 +10,7 @@ import { Far } from '@endo/far';
  * @param {BootstrapPowers & {
  *   consume: {
  *     loadCriticalVat: VatLoader<any>;
- *     networkVat: NetworkVat;
+ *     portAllocator: PortAllocator;
  *   };
  *   produce: {
  *     orchestration: Producer<any>;
@@ -29,7 +27,7 @@ import { Far } from '@endo/far';
  */
 export const setupOrchestrationVat = async (
   {
-    consume: { loadCriticalVat, networkVat },
+    consume: { loadCriticalVat, portAllocator: portAllocatorP },
     produce: {
       orchestrationVat,
       orchestration,
@@ -49,17 +47,10 @@ export const setupOrchestrationVat = async (
   orchestrationVat.reset();
   orchestrationVat.resolve(vats.orchestration);
 
-  await networkVat;
-  /** @type {AttenuatedNetwork} */
-  const network = Far('Attenuated Network', {
-    /** @param {string} localAddr */
-    async bindPort(localAddr) {
-      return E(networkVat).bindPort(localAddr);
-    },
-  });
+  const portAllocator = await portAllocatorP;
 
   const newOrchestrationKit = await E(vats.orchestration).makeOrchestration({
-    network,
+    portAllocator,
   });
 
   orchestration.reset();
@@ -88,7 +79,7 @@ export const getManifestForOrchestration = (_powers, { orchestrationRef }) => ({
     [setupOrchestrationVat.name]: {
       consume: {
         loadCriticalVat: true,
-        networkVat: true,
+        portAllocator: 'portAllocator',
       },
       produce: {
         orchestration: 'orchestration',