openlawlibrary / taf

The Archive Framework
GNU Affero General Public License v3.0
10 stars 10 forks source link

Implement and standardize a `@yubikey` decorator for CLI commands #521

Open n-dusan opened 2 months ago

n-dusan commented 2 months ago

Proposal:

Create a @yubikey decorator:

Add YubiKey verification:

Consistency for all commands:

Tasks

n-dusan commented 2 months ago

What do you think @renatav? Could this be a good first issue for someone?

renatav commented 2 months ago

There are some complexities related to how pins are stored and used. I wouldn't say that this is a good first issue. Might even require a bit of refactoring

renatav commented 1 month ago

I just remembered that the pin option was removed intentionally a while back due to security concerns. I am not sure if we want to do this

renatav commented 1 month ago

I've been thinking about this and I can think of quite a few problems with this approach. Firstly, I'd argue that our current signing logic is more sophisticated than what is proposed in this issue. For example, if you have not inserted your yubikey, you will be asked to insert it. If it's not the correct one, you will be asked to enter a different yubikey. If the threshold is greater than 1, you will be asked to enter another yubikey. Before even trying to load the keys from the yubikey, the signing code will try to load it from keystore files. If we were to move that code to a CLI decorator, we'd also need to check which roles would need to be signed before even executing the API function and I don't like the idea of a CLI decorator handling such complex tasks. Additionally, let's say that you are adding a new delegated target role. You'll have to sign both the parent metadata and the new metadata. Also, the threshold might be greater than 1, especially if you are updating the root metadata. Additionally, we removed the pin option for security reasons.

@n-dusan Is the main problem here that it's not possible to execute these commands from a script? Can that script use the API functions and not the CLI? We could think of another way to make that possible. If that's the core issue, I'd vote for closing this issue as wontfix and opening another one.

renatav commented 1 month ago

Sorry, I misunderstood this. So this should not only be used in TAF, but we want to be able to import it outside of TAF? It should handle simpler cases where we know there is only one role/signing key? It's not about using it from a script? Since I was not involved in the definition of this requirement, I don't really know what the best path forward would be. It might be simple if relying on the existing yubikeys database is sufficient. If we want to handle cases like the one I listed above, I am not in favor of a CLI decorator handling it all. For whoever ends up implementing this, take a look at how load_signing_keys works