feat: Add identity stage1 signing

[eftychis]

Adds a crate that currently simply provides a signing function, loading an Ed25519 key (assuming a PEM encoding). We implement the Signer trait we added beforehand.

We also set up an identity profile locally per project for now. This is done for backward and forward compatibility until the format crystalizes and we are ready to expose to the user identity profiles.

[eftychis]

Requires 461 to merge beforehand (DONE).

• Principal => Make it a type in ic_http_agent/src/types (in feat: internal: Add Principal Identifier to spec types #461)
• Remove Provider and IdentityWallet
• Create a Signer implementation that reads a PEM (or any format) and sign requests.
• Remove any mention or design related to Revocation. We will add later.
• key stored in project
• Reduce size
  • Remove documentation
  • Remove examples

[hansl]

Rename this just identity.rs?

[hansl]

Actually why have this here (I just saw this was in dfx) instead of re-using the ic_identity_manager?

[eftychis]

We define the trait in this crate. This would avoid building the whole agent to build the identity crate. And in effect, rebuilding the identity crate every time a small change happens in the agent, which should receive more frequent updates. (I know now types lives under agent, but as suggested and also requested by @chenyan-dfinity, we should have a spec types crate or something along these lines).

[hansl]

Put yourself in someone making a webserver which talks to the IC as a backend (in Rust). They would use ic_http_agent to talk to the IC, but since the IC needs a signature, they'll need to provide a Signer somehow. Should they copy-paste this whole file everywhere, or just use ic_identity_manager?

Alternatively, what's the case for a program that uses ic_identity_manager without using ic_http_agent at all?

[hansl]

In my PR, the separation was clear; ic_pem_identity was for using a PEM (it was an implementation of an identity, with the goal to have maybe ic_yubikey_identity later or similar). The goal was also to reduce transitive dependencies; we don't need to have yubikey-related code if we use pem files.

In this case, I don't know what ic_identity_manager is (still). Is it a manager? Is it the only implementation we'll provide for identity and Signing?

[eftychis]

It's a lib, it is to be used indirectly. I have gotten a bit mixed messages as to the usage of the agent and its API from the last few discussions, so I stand a bit confused. May I suggest we write down how we want people to use it and define the hierarchy and the pros and cons of each? Rushing ahead a bit, perhaps a thin layer defining an API ahead optimizes for both external users and build times.

I think for now it is fine to optimize for build time and leave the trait implementation there, and then move it in agent_api or something? Or maybe just in the agent. (I think the agent crate as you noted has gotten a bit overloaded, so perhaps clarifying its scope will help. We should think as crates as our compilation, scoping and test buddies.)

ic_identity_manager should house all the implementations, as was mentioned in the documentation in the previous PR, (e.g. like how actix, has multiple actor types). I can run ahead and say we need a common type that deals with multiple principals and associated key pairs and their sources (but I would like to avoid starting this long conversation again). I think it should be the only implementation provided by us at least. (And while someone might decide to write their own agent, they probably should be using our types and identity functionality and not rewrite them. Although I see little point for anyone to rewrite any of the above anytime soon just to say there is a non-dfinity implementation. Thoughts? Perhaps not to be discussed here.) The Signer trait is there to provide separation and mocking ability.

[blynn]

I prefer to avoid comments like this, as they don't age well.

Imagine you join a company, and you see this comment in the code base. Who wrote it? Who discussed it with whom? What is the purpose of this comment?

If this issue matters, then: "TODO(eftychis): Escalate to Dom and return error instead of panic." or whatever, or better still, file a ticket "Return error instead of panic".

If it doesn't matter that much, then omit the comment. It should be clear from the code that it panics, and just as clear that in theory someone could make it return an error instead.

[blynn]

Approving this, as it seems to be blocking people, and the disagreement doesn't seem to involve functionality, but rather just how things are organized. This can be fixed later. (I hate technical debt, but it's the lesser of two evils right now.)