Conversation
| P384, | ||
| /// ECDSA P-521 / ES512 | ||
| P521, | ||
| } |
There was a problem hiding this comment.
This is a bit of a mess right now -- not your PR, but the general situation with JwsAlgorithm and similar types.
The simple thing first: I would avoid saying "E2EI" or "end-to-end identity" in new API, that's a thing from the past and we would like to move away from that terminology. Instead, we should use credential-type names, e.g. "X509Credential" or similar, that make it much more explicit what the API is about.
That said, here I would perhaps just use "JwsAlgorithm", though even that is not ideal as the enum values are just curve names, not proper JWS algorithm names (I thought about using Curve but I don't think that is really sufficient in this context). It's really a mess and to resolve it properly we would need to rework the way we handle JwsAlgorithm, config and related types (for that we have a item already, I think created by Felix). 😞
There was a problem hiding this comment.
Alright, renamed it to just JwsAlgorithm and removed any references to e2ei.
| pub signing_key_pem: String, | ||
| /// PEM-encoded certificate chain, starting with the end-entity certificate. | ||
| pub certificate_chain_pem: Vec<String>, | ||
| } |
There was a problem hiding this comment.
We should not be exposing this. From the perspective of a CoreCrypto user, the acquisiton flow results in a valid credential, not the (signing key, certificate chain) pair. That is what is going under the hood, as an implementation detail, but the caller of our API should never see the signing key, only the (opaque) credential, that can then be added to the client, used in a conversation etc.
There was a problem hiding this comment.
Right of course! Wasn't thinking initially and just exposed everything as is, but why isn't the result type a credential in acquisition flow?
There was a problem hiding this comment.
The problem is that we can't do that right now without introducing a circular dependency because the credential type is currently defined in the crypto crate. Ideally, we would have the credential type in the e2e-identity crate (I'd like to rename that crate too, but that's not critical for CC10 as it is an impl detail), however moving it would probably not be a small change. In a way, the situation is similar to the one with the keystore trait definitions. Right now for crypto-ffi this should not be a problem because it depends on both e2e-identity and crypto crates.
| /// Initial state of the E2EI X509 credential acquisition flow. | ||
| #[derive(uniffi::Object)] | ||
| pub struct X509CredentialAcquisition { | ||
| inner: Mutex<Option<wire_e2e_identity::X509CredentialAcquisition>>, |
There was a problem hiding this comment.
The intention was to remove the X509CredentialAcquisition once the flow completes so that you can't call finalize twice. But I guess the idea was to be able to resume from an intermediate state after deserializing the acquisition in the web case so this doesn't work.
There was a problem hiding this comment.
Replaced the option with an enum which would need to be extended when add support for serializing / deserializing.
| #[uniffi::export] | ||
| impl X509CredentialAcquisitionDpopChallengeCompleted { | ||
| /// Complete the OIDC challenge and return the acquired signing key and certificate chain. | ||
| pub async fn complete_oidc_challenge(&self) -> CoreCryptoResult<X509CredentialAcquisitionResult> { |
There was a problem hiding this comment.
Heh, it is not intended that clients know about the underlying process details, such as which challenges we have and to call them. Please see the RFC CC2 and in particular how .finalize() is supposed to be used. The state we're in should be tracked internally.
typfel
force-pushed
the
jacob/feat/acqusition-flow-bindings
branch
5 times, most recently
from
b26da68 to
cb299bb
Compare
typfel
force-pushed
the
jacob/feat/acqusition-flow-bindings
branch
from
c8be0a4 to
7d2fd34
Compare
3 participants
What's new in this PR
Expose the acquisition flow in the bindings
PR Submission Checklist for internal contributors
SQPIT-764feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.