Skip to content

Consolidate the P2P identity between the Serai node and Coordinator #822

@kayabaNerve

Description

@kayabaNerve

The Serai node used to randomly derive its P2P identity, but this was corrected recently and is now as follows:

fn node_key(keystore: Option<&Keystore>) -> NodeKeyConfig {
let mut seed = match keystore {
/*
If we have a keystore, set our `node_key` to be deterministic to it. This avoids having to
actively manage multiple key materials for the node's operation.
Because the auxiliary key is defined over Ristretto, yet the P2P network uses Ed25519, we
are unable to use the existing derivation scheme (though one can technically define a
mapping between the two groups). As we do not require _public_ derivation of node keys,
though that is a requirement for subkeys, we simply use a domain-separated hash of a
derived private key to generate the necessary key material.
*/
Some(keystore) => {
let node_key_entropy = keystore.pair(sp_core::crypto::KeyTypeId(*b"p2ed"));
let node_key_entropy = Zeroizing::new(node_key_entropy.to_raw_vec());
/*
This effects a hardened derivation, which may or may not be necessary. Specifically,
the sr25519 VRF is premised on the computation of Diffie-Hellmans, and transports
frequently derive a symmetric encryption key from a Diffie-Hellman. If the P2P protocol
grants the adversary a DH oracle (they can provide an arbitrary point and receive the
DH for a validator's key and that point, such as if they open a transport, claim an
arbitrary point is in fact their own key (causing the validator to perform the DH), and
then somehow learn the symmetric key from there), then usage of a P2P key with a known
relation to the key used with BABE may break the security of BABE (due to granting the
adversary the ability to learn random numbers before everyone else).
Similar concerns exist any time we wish to provide keys of known relation for usage in
arbitrary protocols. The usage within the runtime itself is only safe as all the keys
are solely used for signing (as fine to compose) except for the singular usage for the
BABE VRF (which is fine to compose with signing). This has to be kept in mind when we
now start using keys, derived in any manner, with other protocols such as the P2P layer
however.
*/
Zeroizing::new(sp_core::blake2_256(&node_key_entropy))
}
// If we do not have a keystore, generate a seed now
// TODO: `Secret::File` with `net_config_path`
None => {
use rand_core::{RngCore as _, OsRng};
let mut seed = Zeroizing::new([0; 32]);
OsRng.fill_bytes(seed.as_mut());
seed
}
};
// Perform a rejection sample such that this will keep sampling keys until it finds one
loop {
use sc_network::config::{Secret, ed25519::SecretKey};
let Ok(node_key) = SecretKey::try_from_bytes(seed.clone()) else {
*seed = sp_core::blake2_256(seed.as_slice());
continue;
};
return NodeKeyConfig::Ed25519(Secret::Input(node_key));
}
}

These PeerIds are already signed by the Substrate node as part of the Authority Discovery module. These are then exposed over an RPC method for the Coordinator to use for peer finding, allowing it deferring its own peer finding to Substrate. However, the Coordinator currently discards the yielded PeerIds on assumption they're distinct, before beginning its own authentication protocol where the dialed addresses yield new PeerIds which are again checked to be signed by a validator.

It'd be better tor reuse the PeerId between the Substrate P2P network and the coordinator, or at least a version supporting public derivation, so that we can maintain the integrity of dial outs (we don't dial an address, expecting a validator, but then receive a distinct peer somehow) and remove our own authentication layer there. We do need a solution for dial ins, but we can have whoever does the dial open the connection by a single-sided signature justifying the connection.

This would simplify, and tighten, the Coordinator's P2P stack.

It also may be justified not to use the current derivation but use a proper strategy as seen for the rest of the keys on the network, though either technically work, so it solely would be a nicety.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions