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.

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

⚗️ COMIT nodes need an identifying key #637

Closed
LLFourn opened this Issue Jan 11, 2019 · 9 comments

Comments

Projects
None yet
3 participants
@LLFourn
Copy link
Contributor

LLFourn commented Jan 11, 2019

Decide how to handle COMIT node identification.
This MUST be compatible with noise, ie, if Alice already knows Bob's id, she needs to be certain she is connected to Bob when negotiating the noise connection.

Most likely public/private key pair, yet to be decided:

  • what handshake protocol
  • how to store locally
  • how to backup (if needed)
  • how to alias
  • how to generate

Notes:
The current proposal is to use a public key which would also be used to set up transport encryption with the [noise protocol] (http://www.noiseprotocol.org/). This is precisely what The BOLT specification does. We would use it in the same way except not necessarily with the same parameters (hash function/curve).

This is needed so we can uniquely identify nodes. Right now nodes are identified with ip and port and there is no transport layer encryption/authentication. This also makes testing multiple nodes on the same machine difficult.

DoD:

  • Proposal in comments
  • Update #662

Child of #744

@luckysori luckysori referenced this issue Jan 13, 2019

Merged

Add inbound connections to client pool #630

1 of 1 task complete
@thomaseizinger

This comment has been minimized.

Copy link
Member

thomaseizinger commented Jan 15, 2019

I'd say there is not really a way around having a key-pair for your COMIT node so that is sort of a given.

There is several questions to be answered/discussed:

  • Users should be able to connect to another node and simply "trust"/"learn" their public key as part of the connection setup.
  • Users should be able to specify an expected public key upfront and fail the connection setup if the other party doesn't identify with the expected public key.

For the 2nd case, the user's COMIT node needs to have list of socket addresses and public keys somewhere.

  • This probably needs some sort of configuration file.
  • The HTTP API needs to be able to accept different combinations of connection information:
    • public key + socket address pair (for connecting to unknown user with pubkey-verification)
    • socket address (for connecting to unknown user without verification or with public key stored in config file)
    • public key (for connecting to known user where the socket address is stored in the config file)
@LLFourn

This comment has been minimized.

Copy link
Contributor Author

LLFourn commented Jan 15, 2019

Whenever I someone shares a COMIT node id it should probably include an ID, IP and port. Can you think of use case for just sharing an IP/port?

Obviously you really have to not care at all who you are talking to if you just connect to an IP and port because it can be MITM attacked. It's also tricky to implement both types of handshakes in noise.

@D4nte D4nte changed the title [Epic] COMIT nodes need an identifying public key ⚗️ COMIT nodes need an identifying key Jan 16, 2019

@LLFourn

This comment has been minimized.

Copy link
Contributor Author

LLFourn commented Jan 16, 2019

Theres's two main choices:

  1. Noise_XK_25519_ChaChaPoly_BLAKE2b
    • It's part of the noise specification
    • It's more efficient
  2. Noise_XK_secp256k1_ChaChaPoly_SHA256
    • PR is here but not merged mcginty/snow#38 (it LGTM)
    • We could use the same public key on the lightning network if we were to implement BOLT.

@LLFourn LLFourn referenced this issue Jan 16, 2019

Closed

Implement the noise handshake protocol #668

2 of 2 tasks complete

@D4nte D4nte added the groomed label Jan 16, 2019

@D4nte D4nte added sprint-backlog and removed groomed labels Jan 16, 2019

@D4nte D4nte added this to the Sprint 6 🔄📖 milestone Jan 16, 2019

@LLFourn LLFourn self-assigned this Jan 16, 2019

@LLFourn

This comment has been minimized.

Copy link
Contributor Author

LLFourn commented Jan 16, 2019

Proposal:

  • Use noise Noise_XK_25519_ChaChaPoly_BLAKE2b or Noise_XK_secp256k1_ChaChaPoly_SHA256.
  • Your node's private key and therefore public key would be derived from the 32 byte COMIT seed.
  • Storage and how you generate is then however you store the seed

Note if we go with Noise_XK_25519_ChaChaPoly_BLAKE2b I would probably use BLAKE2b for all secret secret derivation. We need to put more thought into secret derivation

I think "aliasing" is a feature we should design until we have a motivation for it at the application level. At this stage I can see that an application might like to tag peers or add metadata to them. But I think it makes most sense to do this on the application level.

@LLFourn

This comment has been minimized.

Copy link
Contributor Author

LLFourn commented Jan 16, 2019

Sent a meeting invite to discuss this above

@D4nte

This comment has been minimized.

Copy link
Member

D4nte commented Jan 16, 2019

"Storage and how you generate is then however you store the seed".
Do you mean that the seed should be entered somewhere every time you start up the node?
I'd expect to have a key file present beside the configuration to allow re-use of same id.

@LLFourn

This comment has been minimized.

Copy link
Contributor Author

LLFourn commented Jan 16, 2019

@D4nte No. I mean that the key is simply derived from the seed. Seed storage/security improvements are outside the scope of this proposal.

@D4nte

This comment has been minimized.

Copy link
Member

D4nte commented Jan 16, 2019

@LLFourn please see body ticket "how to store locally". Please include a proposal regarding this matter.

@LLFourn

This comment has been minimized.

Copy link
Contributor Author

LLFourn commented Jan 20, 2019

That I'm closing this for now with the bias towards Noise_XK_25519_ChaChaPoly_BLAKE2b but the change in direction discussed Friday may further influence this decision.

Note: Further discussion about key derivation can go here #672

@LLFourn LLFourn closed this Jan 20, 2019

@wafflebot wafflebot bot removed the work-in-progress label Jan 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment