Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: keybook #626

Merged
merged 4 commits into from
May 7, 2020
Merged

feat: keybook #626

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6610ef3
feat: keybook
vasco-santos Apr 28, 2020
1c2c432
chore: apply suggestions from code review
vasco-santos May 6, 2020
7969151
chore: add keys to keybook on connection upgraded
vasco-santos May 7, 2020
14bd946
chore: apply suggestions from code review
vasco-santos May 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 87 additions & 0 deletions doc/API.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@
* [`peerStore.addressBook.get`](#peerstoreaddressbookget)
* [`peerStore.addressBook.getMultiaddrsForPeer`](#peerstoreaddressbookgetmultiaddrsforpeer)
* [`peerStore.addressBook.set`](#peerstoreaddressbookset)
* [`peerStore.keyBook.delete`](#peerstorekeybookdelete)
* [`peerStore.keyBook.get`](#peerstorekeybookget)
* [`peerStore.keyBook.set`](#peerstorekeybookset)
* [`peerStore.protoBook.add`](#peerstoreprotobookadd)
* [`peerStore.protoBook.delete`](#peerstoreprotobookdelete)
* [`peerStore.protoBook.get`](#peerstoreprotobookget)
Expand Down Expand Up @@ -811,6 +814,89 @@ Add known `protocols` of a given peer.
peerStore.protoBook.add(peerId, protocols)
```


### peerStore.keyBook.delete

Delete the provided peer from the book.

`peerStore.keyBook.delete(peerId)`

#### Parameters

| Name | Type | Description |
|------|------|-------------|
| peerId | [`PeerId`][peer-id] | peerId to remove |

#### Returns

| Type | Description |
|------|-------------|
| `boolean` | true if found and removed |

#### Example

```js
peerStore.keyBook.delete(peerId)
// false
peerStore.keyBook.set(peerId, publicKey)
peerStore.keyBook.delete(peerId)
// true
```

### peerStore.keyBook.get

Get the known `PublicKey` of a provided peer.

`peerStore.keyBook.get(peerId)`

#### Parameters

| Name | Type | Description |
|------|------|-------------|
| peerId | [`PeerId`][peer-id] | peerId to get |

#### Returns

| Type | Description |
|------|-------------|
| `RsaPublicKey|Ed25519PublicKey|Secp256k1PublicKey` | Peer PublicKey |

#### Example

```js
peerStore.keyBook.get(peerId)
// undefined
peerStore.keyBook.set(peerId, publicKey)
peerStore.keyBook.get(peerId)
// PublicKey
```

### peerStore.keyBook.set

Set known `peerId`. This can include its Public Key.

`peerStore.keyBook.set(peerId, publicKey)`

#### Parameters

| Name | Type | Description |
|------|------|-------------|
| peerId | [`PeerId`][peer-id] | peerId to set |
| publicKey | [`RsaPublicKey|Ed25519PublicKey|Secp256k1PublicKey`][keys] | peer's public key |

#### Returns

| Type | Description |
|------|-------------|
| `KeyBook` | Returns the Key Book component |

#### Example

```js
const publicKey = peerId.pubKey
peerStore.keyBook.set(peerId, publicKey)
```

### peerStore.protoBook.delete

Delete the provided peer from the book.
Expand Down Expand Up @@ -1334,3 +1420,4 @@ This event will be triggered anytime we are disconnected from another peer, rega
[connection]: https://github.com/libp2p/js-interfaces/tree/master/src/connection
[multiaddr]: https://github.com/multiformats/js-multiaddr
[peer-id]: https://github.com/libp2p/js-peer-id
[keys]: https://github.com/libp2p/js-libp2p-crypto/tree/master/src/keys
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@
"aegir": "^21.9.0",
"chai": "^4.2.0",
"chai-as-promised": "^7.1.1",
"chai-bytes": "^0.1.2",
"cids": "^0.8.0",
"delay": "^4.3.0",
"dirty-chai": "^2.0.1",
Expand Down
14 changes: 9 additions & 5 deletions src/connection-manager/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,18 +171,22 @@ class ConnectionManager extends EventEmitter {
* @param {Connection} connection
*/
onConnect (connection) {
const peerId = connection.remotePeer.toB58String()
const storedConn = this.connections.get(peerId)
const peerId = connection.remotePeer
const peerIdStr = peerId.toB58String()
const storedConn = this.connections.get(peerIdStr)

if (storedConn) {
storedConn.push(connection)
} else {
this.connections.set(peerId, [connection])
this.connections.set(peerIdStr, [connection])
this.emit('peer:connect', connection)
}

if (!this._peerValues.has(peerId)) {
this._peerValues.set(peerId, this._options.defaultPeerValue)
this._libp2p.peerStore.addressBook.add(peerId, [connection.remoteAddr])
this._libp2p.peerStore.keyBook.set(peerId, peerId.pubKey)

if (!this._peerValues.has(peerIdStr)) {
this._peerValues.set(peerIdStr, this._options.defaultPeerValue)
}

this._checkLimit('maxConnections', this.size)
Expand Down
21 changes: 13 additions & 8 deletions src/peer-store/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ Several libp2p subsystems will perform operations, which will gather relevant in

In a libp2p node's life, it will discover peers through its discovery protocols. In a typical discovery protocol, addresses of the peer are discovered along with its peer id. Once this happens, the PeerStore should collect this information for future (or immediate) usage by other subsystems. When the information is stored, the PeerStore should inform interested parties of the peer discovered (`peer` event).

Taking into account a different scenario, a peer might perform/receive a dial request to/from a unkwown peer. In such a scenario, the PeerStore must store the peer's multiaddr once a connection is established.
Taking into account a different scenario, a peer might perform/receive a dial request to/from a unkwown peer. In such a scenario, the PeerStore must store the peer's multiaddr once a connection is established.

When a connection is being upgraded, more precisely after its encryption, or even in a discovery protocol, a libp2p node can get to know other parties public keys. In this scenario, libp2p will add the peer's public key to its `KeyBook`.
Copy link
Member Author

@vasco-santos vasco-santos May 6, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jacobheun from what we discussed, I could only not do this by now.

I think that it makes sense, but afaik noise uses static public keys, which I would not expect to have here. In addition, we will need to provide libp2p to the crypto module for adding the key to the keyBook or change the crypto interface to also require the public key, so that we can update this in the upgrader.js.

We should probably give some thought on this and follow up with a good solution.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noise has its own keycache and I dont think we need to worry about persistence of the crypto transport keys, just the libp2p id keys. I think this section is achieved by updating the peer after we've established a connection, because the crypto handshake will result in libp2p public key exchange and verification.

Copy link
Member Author

@vasco-santos vasco-santos May 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the inbound connection, we do have the public key after the connection being established. The same is not true for the outbound connection. Both for libp2p-secio and libp2p-noise

I went deeper on secio to figure out why this is happening and I found this: https://github.com/libp2p/js-libp2p-secio/blob/master/src/handshake/crypto.js#L66-L73

When an inbound connection, we go to the else statement while in an outbound connection we go to the if side of things. In the if part we basically do not add the public key. If I just add state.id.remote = remoteId inside the if and after the validation condition, it works. Is this the expected?

In noise, I could not follow the flow so easily yet...

Copy link
Member Author

@vasco-santos vasco-santos May 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Meanwhile, I already added the code for this here and a test. The validation of the outbound connection key being exchanged is currently commented as a result of what I described above

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: this is fixed in secio now, I'll look into it in noise as part of my work there.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you recommend here? change the test configuration to use secio for now and create an issue to track this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leave it for now, I'll work on getting a patch for noise to correct this. We can add a TODO item to the peer store epic to track getting this in.


After a connection is established with a peer, the Identify protocol will run automatically. A stream is created and peers exchange their information (Multiaddrs, running protocols and their public key). Once this information is obtained, it should be added to the PeerStore. In this specific case, as we are speaking to the source of truth, we should ensure the PeerStore is prioritizing these records. If the recorded `multiaddrs` or `protocols` have changed, interested parties must be informed via the `change:multiaddrs` or `change:protocols` events respectively.

Expand Down Expand Up @@ -42,7 +44,7 @@ The `addressBook` keeps the known multiaddrs of a peer. The multiaddrs of each p

`Map<string, Address>`

A `peerId.toString()` identifier mapping to a `Address` object, which should have the following structure:
A `peerId.toB58String()` identifier mapping to a `Address` object, which should have the following structure:

```js
{
Expand All @@ -52,17 +54,19 @@ A `peerId.toString()` identifier mapping to a `Address` object, which should hav

#### Key Book

The `keyBook` tracks the keys of the peers.
The `keyBook` tracks the public keys of the peers by keeping their [`PeerId`][peer-id].

**Not Yet Implemented**
`Map<string, PeerId`

A `peerId.toB58String()` identifier mapping to a `PeerId` of the peer. This instance contains the peer public key.

#### Protocol Book

The `protoBook` holds the identifiers of the protocols supported by each peer. The protocols supported by each peer are dynamic and will change over time.

`Map<string, Set<string>>`

A `peerId.toString()` identifier mapping to a `Set` of protocol identifier strings.
A `peerId.toB58String()` identifier mapping to a `Set` of protocol identifier strings.

#### Metadata Book

Expand All @@ -74,8 +78,9 @@ For the complete API documentation, you should check the [API.md](../../doc/API.

Access to its underlying books:

- `peerStore.protoBook.*`
- `peerStore.addressBook.*`
- `peerStore.keyBook.*`
- `peerStore.protoBook.*`

### Events

Expand Down Expand Up @@ -107,8 +112,6 @@ All the known peer protocols are stored with a key pattern as follows:

**KeyBook**

_NOT_YET_IMPLEMENTED_

All public keys are stored under the following pattern:

` /peers/keys/<b32 peer id no padding>`
Expand All @@ -127,3 +130,5 @@ Metadata is stored under the following key pattern:
- Further API methods will probably need to be added in the context of multiaddr validity and confidence.
- When improving libp2p configuration for specific runtimes, we should take into account the PeerStore recommended datastore.
- When improving libp2p configuration, we should think about a possible way of allowing the configuration of Bootstrap to be influenced by the persisted peers, as a way to decrease the load on Bootstrap nodes.

[peer-id]: https://github.com/libp2p/js-peer-id
36 changes: 11 additions & 25 deletions src/peer-store/book.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class Book {
* Set data into the datastructure, persistence and emit it using the provided transformers.
* @private
* @param {PeerId} peerId peerId of the data to store
* @param {Array<*>} data data to store.
* @param {*} data data to store.
* @param {Object} [options] storing options.
* @param {boolean} [options.emit = true] emit the provided data.
* @return {void}
Expand All @@ -57,22 +57,22 @@ class Book {

// Store data in memory
this.data.set(b58key, data)
this._setPeerId(peerId)

// Emit event
emit && this._ps.emit(this.eventName, {
peerId,
[this.eventProperty]: this.eventTransformer(data)
})
emit && this._emit(peerId, data)
}

/**
* Add known data of a provided peer.
* Emit data.
* @private
* @param {PeerId} peerId
* @param {Array<Data>|Data} data
* @param {*} data
*/
add (peerId, data) {
throw errcode(new Error('set must be implemented by the subclass'), 'ERR_NOT_IMPLEMENTED')
_emit (peerId, data) {
this._ps.emit(this.eventName, {
peerId,
[this.eventProperty]: this.eventTransformer(data)
})
}

/**
Expand Down Expand Up @@ -104,24 +104,10 @@ class Book {
return false
}

this._ps.emit(this.eventName, {
peerId,
[this.eventProperty]: []
})
this._emit(peerId, [])

return true
}

/**
* Set PeerId into peerStore datastructure.
* @private
* @param {PeerId} peerId
*/
_setPeerId (peerId) {
if (!this._ps.peerIds.get(peerId)) {
this._ps.peerIds.set(peerId.toB58String(), peerId)
}
}
}

module.exports = Book
22 changes: 12 additions & 10 deletions src/peer-store/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const { EventEmitter } = require('events')
const PeerId = require('peer-id')

const AddressBook = require('./address-book')
const KeyBook = require('./key-book')
const ProtoBook = require('./proto-book')

const {
Expand Down Expand Up @@ -42,16 +43,14 @@ class PeerStore extends EventEmitter {
this.addressBook = new AddressBook(this)

/**
* ProtoBook containing a map of peerIdStr to supported protocols.
* KeyBook containing a map of peerIdStr to their PeerId with public keys.
*/
this.protoBook = new ProtoBook(this)
this.keyBook = new KeyBook(this)

/**
* TODO: this should only exist until we have the key-book
* Map known peers to their peer-id.
* @type {Map<string, Array<PeerId>}
* ProtoBook containing a map of peerIdStr to supported protocols.
*/
this.peerIds = new Map()
this.protoBook = new ProtoBook(this)
}

/**
Expand All @@ -73,7 +72,7 @@ class PeerStore extends EventEmitter {

// AddressBook
for (const [idStr, addresses] of this.addressBook.data.entries()) {
const id = PeerId.createFromCID(idStr)
const id = this.keyBook.data.get(idStr) || PeerId.createFromCID(idStr)
peersData.set(idStr, {
id,
addresses,
Expand All @@ -84,10 +83,11 @@ class PeerStore extends EventEmitter {
// ProtoBook
for (const [idStr, protocols] of this.protoBook.data.entries()) {
const pData = peersData.get(idStr)
const id = this.keyBook.data.get(idStr) || PeerId.createFromCID(idStr)

if (!pData) {
peersData.set(idStr, {
id: PeerId.createFromCID(idStr),
id,
addresses: [],
protocols: Array.from(protocols)
})
Expand All @@ -104,8 +104,10 @@ class PeerStore extends EventEmitter {
*/
delete (peerId) {
const addressesDeleted = this.addressBook.delete(peerId)
const keyDeleted = this.keyBook.delete(peerId)
const protocolsDeleted = this.protoBook.delete(peerId)
return addressesDeleted || protocolsDeleted

return addressesDeleted || keyDeleted || protocolsDeleted
}

/**
Expand All @@ -118,7 +120,7 @@ class PeerStore extends EventEmitter {
throw errcode(new Error('peerId must be an instance of peer-id'), ERR_INVALID_PARAMETERS)
}

const id = this.peerIds.get(peerId.toB58String())
const id = this.keyBook.data.get(peerId.toB58String())
const addresses = this.addressBook.get(peerId)
const protocols = this.protoBook.get(peerId)

Expand Down
Loading