brycx / pasetors

PASETOrs: PASETO tokens in pure Rust
MIT License
83 stars 8 forks source link

Provide `TryFrom<AsymmetricSecretKey> for AsymmetricPublicKey<>` #46

Closed brycx closed 2 years ago

brycx commented 2 years ago

A user should be able to compute the public key from a given private key of the current versions v2, v3 and v4.

This should be straight forward with v2 and v4 since the ed25519 crates support the needed operations as part of their public API. In terms of v3, it seems this will require a dependency on fiat-crypto and implementing scalar multiplication with the curve base/generator. ring does not support computing a public key from the private and does not even have a type to represent a private key.

Based on discussion starting here: https://github.com/brycx/pasetors/issues/40#issuecomment-1098557241

brycx commented 2 years ago

@Eh2406 I'm going to continue some of the conversion here. You said the following regarding computing the public key from the private key:

The design document assumed this method would be available, and so does not call for the public key being stored. It can be stored in the users config file, but it opens up possibilities of somebody tampering with their config files and having them no longer match. This can definitely be worked around for now, but I would really like to have it fixed before this functionality is stabilized.

I just re-read the design-document which states:

The PASETO validates using the public key it looked up based on the key ID.

So according to the design document, you'll do a lookup of the public key, using the corresponding ID, of keys that are stored in hardware tokens? What ID is used here, that of the public pid or that of the secret key sid? Your requirements sound like you cannot store the public key hardware-side with the secret key, so I'm assuming it's sid you are going to use?

I'm confused because further down, the design-doc says that a pid will be generated, which is what will be put in the footer. In that case, that you'd have to take all private keys, compute the public, compute the pid and then do a lookup. Maybe I'm misunderstadning who has access to a public key? To be honest, I haven't read all prior conversation and all of the document in detail, so apologies if there's something obvious I missed.

Eh2406 commented 2 years ago

My use case is somewhat complicated and a little nonstandard. (Although I'm hoping it will inspire other people to copy the idea.) There are two components of the system a client (cargo) and a server (the registry). This functionality will be really useful for the client.

An interaction starts with the client. The client has a secret key. (The secret key can be stored on a hardware token, but then you're using the hardware tokens library to do crypto not pasetors.) The clients towards the secret key in a config file, which a user can edit but shouldn't have to. When it wants to authorize a request it creates a PASETO, which includes the pid in the footer. This is the point when it would be nice to go from a secret key to a public key to a pid. The config file could store say "secret key|public key", but that provides an opportunity for users config file to have a "secret key" and a "public key" that do not match.

To fill out the rest of the story, The server receives a PASETO, as part of the first contact from the client. It looks in the footer to find the pid. It checks its database to see if that pid has been registered and the public key it refers to. It then verifies that the PASETO was signed by that public key.

brycx commented 2 years ago

Thanks for clarifying, makes a lot more sense now.

Regarding this functionality, I'm trying to see if it's viable to get ring to support the conversion or if it has to be done in pasetors itself. If the latter is the only current option, then this might take a little bit because scalar multiplication is where things go awry quickly. I'll try to keep this issue updated during the investigation.

brycx commented 2 years ago

I've been thinking about the use-cases presented here a bit more and just wanted to add the following in case it hasn't been considered yet.

It can be stored in the users config file, but it opens up possibilities of somebody tampering with their config files and having them no longer match. (#40)

The config file could store say "secret key|public key", but that provides an opportunity for users config file to have a "secret key" and a "public key" that do not match.

You mention the possibility of having a mismatch between a client's secret key and public key, if both were to be stored in a config file. The following scenarios come to mind, based on a client-side config file. It assumes the storage you proposed and using the public key from the config.

good secret | bad public: Client signs the PASETO token, but the pid will not map to the correct public key stored by the server, because the wrong public key was added.

bad secret | good public: The signature of the PASETO token will be rejected by the server, because the pid mapped to the correct public key, but the wrong secret key was used to create the token.

bad secret | bad public: Again, the pid won't map to any public key stored by server or an incorrect one.

good secret | good public: Everything works.

If the threat-model includes an attacker tampering with the config file, where the user stores their secret key, then a mismatch between that and the public key should be the last thing to worry about. I do however see the case if there is say, file corruption, but that seems then equally likely to happen the secret key-portion of the config file.

Eh2406 commented 2 years ago

Absolutely correct. This does not matter to the threat-model and security of the system. This is only a UX problem, there are more ways for the users system to get messed up and it will probably be hard for them to debug. That being said, the problem starts with someone modifying there config. So maybe they get what they deserve.

Eh2406 commented 2 years ago

The client has a secret key.

In a meeting today it was pointed out to me that a user can provide cargo with a secret key as part of the CLI, if the user does not want to trust the cargo login --generate-keypair command. It seams like a bad UX to have the user need to pass both a public and secret on the CLI.

To repeat, this can definitely be worked around for now, but I would really like to have it fixed before this functionality is stabilized.

brycx commented 2 years ago

Thanks for updating. I completely understand that this is important for UX.

To repeat, this can definitely be worked around for now, but I would really like to have it fixed before this functionality is stabilized.

I need to discuss some approaches with others, regarding the scalar multiplication (if ring won't add funcitonality for it). As long as it's not the most pressing issue, I think we can accomodate this in a fair timeframe.

Eh2406 commented 2 years ago

I deeply appreciate how responsive you have bean to my requests!

brycx commented 2 years ago

(if ring won't add funcitonality for it)

There seems to be a (stale?) PR from 2020, milestoned for ring 0.17.0 (which is in prerelease): https://github.com/briansmith/ring/pull/889

This does seem like it would allow pasetors to convert from secret-key->public-key.

Eh2406 commented 2 years ago

Just wondering if there has bean any progress on this? I am getting to the point where I need to decide how to work around this.

brycx commented 2 years ago

Sorry, I forgot to update the issue with progress.

About a week ago I finished a project with my supervisor, where we wound up looking into all complete addition formulas, etc, needed for the scalarmult. I have most of the custom work done, but saw that Frank Denis released a new rust-p384 a few days prior (which also had the parts for scalarmult I was still missing). This integrates with RustCrypto traits, uses the same addition formulas and approach I had planned, so I'm trying to use this instead. It provides the benefit of already supporting point-compresssion, so we get rid of the custom code for this. Additionally, RustCrypto has for the ecdsa crate recently started adding experimental support for deterministic nonces, which we'd also like to have in pasetors if possible.

The only problem is that rust-p384 has a broken ECDSA implementation right now. I have a fix for one of the problems I've discovered but I know of one other thing that needs to be fixed, before it should be fully working. That issue is what I'm trying to figure out currently.

Eh2406 commented 2 years ago

This sounds like a lot of significant progress! Thank you for working on this!

brycx commented 2 years ago

After some back and forth with Frank Denis on rust-p384, the ECDSA impl is now working and this functionality has been added in the latest prerelease: https://crates.io/crates/pasetors/0.5.0-alpha.6

Feel free to re-open this issue or a new one if there's something not working.

Eh2406 commented 2 years ago

Thank you so much. You've been super responsive to all of my issues, and I really appreciate it. I updated my code from yesterday, and so far things seem to be working well!