Use standard IP cidr format for allow/deny lists

[achingbrain]

Currently the connection manager treats multiaddrs in the allow/deny lists as string prefixes (e.g. "/ip4/52.55"), it should use the standard IP cidr format instead (e.g. "/ip4/52.55.0.0/ipcidr/16") which will allow us to validate these as multiaddrs.

This will likely involve changes to the @multiformats/multiaddr module to allow using address ranges.

[achingbrain]

Related issue: libp2p/js-libp2p-utils#12

[mpetrunic]

This will likely involve changes to the @multiformats/multiaddr module to allow using address ranges.

@achingbrain Doesn't multiaddr already have ipcidr for this purpose?

Seems like we are missing https://github.com/multiformats/go-multiaddr/blob/master/protocols.go#L112 in js?

Should we add cidr based filtering implementation in js-multiaddr (like go) or in js-libp2p-utils?

[SgtPooki]

do we still need this @achingbrain ?

[achingbrain]

Yes

[SgtPooki]

JS Colo 2024 rough discussion

It looks like we already have the code in js-multiaddr, https://github.com/multiformats/js-multiaddr/blob/d4164fbce730d3b43a10682b651bcc96db845e7c/src/convert.ts#L120C17-L135, so we don't need to change it

Should only need changes in js-libp2p

convertToIpNet defined at https://github.com/multiformats/js-multiaddr/blob/d4164fbce730d3b43a10682b651bcc96db845e7c/src/convert.ts#L120C17-L135

Action items:

1. Convert .allow and .deny properties of connection-manager to be IpNet[].
   • convertToIpNet at

     js-libp2p/packages/libp2p/src/connection-manager/index.ts

     Lines 200 to 201 in 4fd7eb2

     	this.allow = (init.allow ?? []).map(ma => multiaddr(ma))
     	this.deny = (init.deny ?? []).map(ma => multiaddr(ma))

2. replace startsWith in

   js-libp2p/packages/libp2p/src/connection-manager/connection-pruner.ts

   Lines 101 to 104 in 6ccbb06

   	// check allow list
   	const connectionInAllowList = this.allow.some((ma) => {
   	return connection.remoteAddr.toString().startsWith(ma.toString())
   	})

   with Instance<IpNet>.contains(ip)
3. replace startsWith in

   js-libp2p/packages/libp2p/src/connection-manager/index.ts

   Lines 534 to 537 in 4fd7eb2

   	// check allow list
   	const allowConnection = this.allow.some(ma => {
   	return maConn.remoteAddr.toString().startsWith(ma.toString())
   	})

   with Instance<IpNet>.contains(ip)
4. replace startsWith in

   js-libp2p/packages/libp2p/src/connection-manager/index.ts

   Lines 524 to 527 in 4fd7eb2

   	// check deny list
   	const denyConnection = this.deny.some(ma => {
   	return maConn.remoteAddr.toString().startsWith(ma.toString())
   	})

   with Instance<IpNet>.contains(ip)

[acul71]

@SgtPooki @achingbrain
Hello I'm Luca Cohort-1.
I would like to work on this issue (I already started to modify the code)

[wemeetagain]

@acul71 go for it! open a PR when you're ready.

[acul71]

@SgtPooki @wemeetagain

I modified the code following #1510 (comment)
When I run tests (npm run test:node) I got this error:

LUCA1234 init.allow= [ '/ip4/83.13.55.32' ]
  62 passing (3s)
  1 failing
  1) Connection Manager
       should not close connection that is on the allowlist when pruning:
     Error: Invalid multiaddr
      at convertToIpNet (/home/luca/Informatica/Learning/PNL_Launchpad_Curriculum/Libp2p/js-libp2p/node_modules/@multiformats/multiaddr/src/convert.ts:132:11)
      at /home/luca/Informatica/Learning/PNL_Launchpad_Curriculum/Libp2p/js-libp2p/packages/libp2p/src/connection-manager/index.ts:229:47
      at Array.map (<anonymous>)
      at new DefaultConnectionManager (/home/luca/Informatica/Learning/PNL_Launchpad_Curriculum/Libp2p/js-libp2p/packages/libp2p/src/connection-manager/index.ts:229:37)
      at new Libp2p (/home/luca/Informatica/Learning/PNL_Launchpad_Curriculum/Libp2p/js-libp2p/packages/libp2p/src/libp2p.ts:123:50)
      at createLibp2p (/home/luca/Informatica/Learning/PNL_Launchpad_Curriculum/Libp2p/js-libp2p/packages/libp2p/src/index.ts:202:16)
      at createNode (/home/luca/Informatica/Learning/PNL_Launchpad_Curriculum/Libp2p/js-libp2p/packages/libp2p/test/fixtures/creators/peer.ts:49:16)
      at Context2.<anonymous> (/home/luca/Informatica/Learning/PNL_Launchpad_Curriculum/Libp2p/js-libp2p/packages/libp2p/test/connection-manager/index.spec.ts:206:14)

Is '/ip4/83.13.55.32' a valid multiaddress?
Checked here:

this.allow = (init.allow ?? []).map(ma => convertToIpNet(multiaddr(ma)))

This is the test

it('should not close connection that is on the allowlist when pruning', async () => {
    const max = 2
    const remoteAddr = multiaddr('/ip4/83.13.55.32/tcp/59283')
    console.log("LUCA9999") // luca
    libp2p = await createNode({
      config: createBaseOptions({
        connectionManager: {
          maxConnections: max,
          allow: [
            '/ip4/83.13.55.32'
          ]
        }
      }),
      started: false
    })

This is the function processing the multiaddress (convertToIpNet)

export function convertToIpNet (multiaddr: Multiaddr): IpNet {
  let mask: string | undefined
  let addr: string | undefined
  multiaddr.stringTuples().forEach(([code, value]) => {
    if (code === ip4Protocol.code || code === ip6Protocol.code) {
      addr = value
    }
    if (code === ipcidrProtocol.code) {
      mask = value
    }
  })
  if (mask == null || addr == null) {
    throw new Error('Invalid multiaddr')
  }
  return new IpNet(addr, mask)
}

[SgtPooki]

Is there any further work to do here? #2783 was completed and this can probably be closed unless i'm missing something.

@achingbrain

[achingbrain]

No, this is complete and has shipped.