frame-session: Introduce a proper proof of key ownership

[bkchr]

frame_session::set_keys supports providing a proof as second parameter. This proof was not yet checked. This pull request introduces a verification of this proof and also a way of generating this proof. The proof in FRAME are concatenated Signatures. These Signatures are in the same order as the public keys in the SessionKeys struct. Each signature is signing the owner, proofing that the generating party has access over the private key associated to the public session key. The owner in FRAME is the account id of the account that will call set_keys, aka the account of the validator.

This pull request is changing the SessionKeys runtime api. This still requires a RFC, as this is a public interface of the Polkadot runtime.

@jacogr polkadot-js should provide a way to use the runtime api directly to generate the sessions keys.
@paritytech/docs-audit this will require updates of the validator documentation to tell them what to pass for owner and that they need to pass proof to set_keys.

[bkchr]

bot fmt

[command-bot]

@bkchr https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3833889 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh". Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 21-2be17067-a4d6-4111-9ee5-f00880c1886f to cancel this command or bot cancel to cancel all commands in this pull request.

[command-bot]

@bkchr Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3833889 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3833889/artifacts/download.

[bkchr]

@jacogr polkadot-js should provide a way to use the runtime api directly to generate the sessions keys.

Scrape that, we need a new RPC for this.

[kianenigma]

https://github.com/orgs/paritytech/teams/docs-audit this will require updates of the validator documentation to tell them what to pass for owner and that they need to pass proof to set_keys.

This is mostly in the Polkadot wiki, I will ping the w3f folks and also setup a system where they can listen to mentioned of this docs-audit team. But all in all, you should do your best to update the docs around this in the rust docs of pallet session at first.

[davxy]

Looks good

[davxy]

Comment?

[davxy]

Maybe we can add an example here?

Something like: "For example, "session pallet" set_keys requires owner to be the account used to sign the extrinsic."

[davxy]

Nitpick. Makes sense to implement Default for OpaqueGeneratedSessionKeys? I see a couple of use cases around

[paritytech-cicd-pr]

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: test-linux-stable 1/3
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7035748

[lexnv]

Logic looks good to me! 👍

[ordian]

cc @drskalman @AlistairStewart

[drskalman]

This is a very important work and I'm not sure if my comment should be considered after its merger for future improvement or not.

1. Proof of possession with BLS signature is secure only if it is done with different signature scheme than the one is used by the signer to sign other messages (using different hash function for example). The attack is described on page 15 of [1].

• [1] Ristenpart, T., & Yilek, S. (2007). The power of
  proofs-of-possession: Securing multiparty signatures against
  rogue-key attacks. In , Annual {{International Conference}} on the
  {{Theory}} and {{Applications}} of {{Cryptographic Techniques} (pp.
  228–245). : Springer.

2. In our flavor of BLS signature, The verification of proof of possession function not only the verifies that the signature is correct but also verifies that the signer has used the same private key to generate the public keys in $G1$ and $G2$:
   https://github.com/w3f/bls/blob/master/src/double_pop.rs#L67. This is not being tested by calling normal verify on the signature.

In that regard, it seems that the correct way of implementing the macro is perhaps to implement the proof of possession generation and verification inside each crypto module and then call that function instead of generic sign and verify and perhaps call generic sign and verify inside that function for all key types except for BLS.

3. @AlistairStewart is of opinion that everybody in the world calls this "Proof of Possession" and not "Ownership proof". Calling it "ownership proof" make it very hard for any outsider to spot and find the code. I'm not sure what is cost of renaming these functions though.