secure-systems-lab / securesystemslib

Cryptographic and general-purpose routines for Secure Systems Lab projects at NYU
MIT License
48 stars 49 forks source link

signer API: review exceptions #468

Open jku opened 1 year ago

jku commented 1 year ago

This is after #456:

Signer API exceptions may not be 100% coherent: it's an API that's grown over time. It's also not as critical that the exceptions are well designed as e.g. python-tuf verification API (as signing happens in much more controlled environments: new keys may be added but that never happens as a surprise to user). Still, would be good to review the exception usage.

One especially tricky case is Signer implementation specific exceptions -- like a KMS signer raising on network error. Currently we just document this as a possibility.

jku commented 1 year ago

There is, AFAICT, no really nice solution to this. We will not be able to handle all exceptions that e.g. google-cloud-kms might raise in a generic and useful way. So we can either tell callers to handle all errors or we can handle all the errors ourselves, and raise a generic SignerException...

The latter would normally be preferable (it's what we do in python-tuf serialization for example) but I can also imagine cases where caller wants to handle some specific errors: in that case wrapping the actual error just makes things more difficult for the caller.

So I'm leaning towards documenting Signers like

callers should prepare for sign() and import_() to raise Signer implementation specific errors that may be undocumented:

it's not amazing but I can live with that

lukpueh commented 1 year ago

it's not amazing but I can live with that

Agreed, also because of what you said above about sign and import happening in a more controlled environment.

lukpueh commented 1 year ago

Let's also review if the exceptions we do know are properly documented and tested.