Support signing with subkeys

[trishankatdatadog]

• Add key expiry when adding encryption/signing subkeys
• Check for key expiry when signature is valid and has not expired
• Implicitly choose either the first unexpired signing subkey or master key in DetachSign*() and and ArmoredDetachSign*()

Signed-off-by: Trishank Karthik Kuppusamy [email protected]

[trishankatdatadog]

I'm not sure whether signature verification should return an error immediately if either the signature or the key has expired. (I should note that this was already the previous behavior, where a valid but expired signature causes verification to error out.) Should we try all keys to see whether we find any good, unexpired signature from an unexpired key?

Cc @SantiagoTorres @lukpueh

[twiss]

Hey 👋 Thanks for the PR!

There is an existing Entity.SigningKey() function, that returns a valid signing subkey if it exists or the primary key, that is used when inline (non-detached) signing, here:
https://github.com/ProtonMail/crypto/blob/1fa7f403fb9c3aa7b47443e1e3b39c80d6eedcb6/openpgp/write.go#L204

I would rather do the same thing in detachSign, instead of forcing the caller to manually pass a subkey themselves (and adding new functions for that), if that solves your use case?

I'm not sure whether signature verification should return an error immediately if either the signature or the key has expired. (I should note that this was already the previous behavior, where a valid but expired signature causes verification to error out.) Should we try all keys to see whether we find any good, unexpired signature from an unexpired key?

I think returning an error as you've done is fine. At that point, we know that the signature verifies correctly, which means that that key signed it, and if that key is expired, we can error. The only possible way that checking other keys would help would be if the key had multiple identical subkeys, which I think shouldn't normally happen.

[trishankatdatadog]

Hi @twiss!

I would rather do the same thing in detachSign, instead of forcing the caller to manually pass a subkey themselves (and adding new functions for that), if that solves your use case?

This might incidentally work for some cases, but not all, where the user would like to use a specific signing subkey if multiple were available, no?

I think returning an error as you've done is fine. At that point, we know that the signature verifies correctly, which means that that key signed it, and if that key is expired, we can error. The only possible way that checking other keys would help would be if the key had multiple identical subkeys, which I think shouldn't normally happen.

The way I understand the function now, it looks like it gets the list of key per signature packet. Therefore, multiple different keys in the keyring might have signed off on the same data. What is the expected OpenPGP semantics in such a case? For example, should a good, unexpired signature from an unexpired key veto a valid, expired signature from an unexpired key?

[twiss]

This might incidentally work for some cases, but not all, where the user would like to use a specific signing subkey if multiple were available, no?

Do you have a specific use case for that in mind?

IMO, we should use the latest valid signing subkey, and that should be a good enough heuristic for most use cases.

The way I understand the function now, it looks like it gets the list of key per signature packet. Therefore, multiple different keys in the keyring might have signed off on the same data. What is the expected OpenPGP semantics in such a case? For example, should a good, unexpired signature from an unexpired key veto a valid, expired signature from an unexpired key?

The function currently doesn't actually support multiple signatures, and only looks at the first one. Though yeah, maybe it should, and then it should probably work as you suggest, yes.

[trishankatdatadog]

Do you have a specific use case for that in mind?

Sure. Suppose that the ACME company master key has two signing subkeys under it: one for signing Linux binaries, and another for signing official documents.

IMO, we should use the latest valid signing subkey, and that should be a good enough heuristic for most use cases.

Probably, but it certainly doesn't cover all use cases, and the proposed new functions are there deliberately not to break backwards-compatibility, and offer additional power to users who want it.

The function currently doesn't actually support multiple signatures, and only looks at the first one. Though yeah, maybe it should, and then it should probably work as you suggest, yes.

I will need to do some testing with GPG to see what it does.

Come to think of it, we might need another function in the future that returns a mapping of keyids to errors, and allow users to decide what to do, but we can worry about that later.

[twiss]

Sure. Suppose that the ACME company master key has two signing subkeys under it: one for signing Linux binaries, and another for signing official documents.

IMHO, if these purposes are significantly different, they should have two separate keys for them, perhaps both signed by a "root key". In OpenPGP, there is a concept of different "users" in a single key, but there is no mechanism of linking a specific subkey to a specific user. So there is no easy way to indicate that a specific subkey is meant for a specific purpose, and most implementations don't assign any significance to the specific subkey that signed a signature, so I think it's a bad idea to use OpenPGP in this way.

Probably, but it certainly doesn't cover all use cases, and the proposed new functions are there deliberately not to break backwards-compatibility, and offer additional power to users who want it.

Yeah, I understand, but I'm just wondering if the use case is strong enough for adding these functions. What is your own use case for them?

[trishankatdatadog]

IMHO, if these purposes are significantly different, they should have two separate keys for them, perhaps both signed by a "root key". In OpenPGP, there is a concept of different "users" in a single key, but there is no mechanism of linking a specific subkey to a specific user. So there is no easy way to indicate that a specific subkey is meant for a specific purpose, and most implementations don't assign any significance to the specific subkey that signed a signature, so I think it's a bad idea to use OpenPGP in this way.

There is Signer's User ID to map signatures to a particular UID under a master key, but TBF I've rarely seen this used. I'm also not convinced it's a bad idea: on a technical side, it's not clear at all why we should prevent certain subkeys from signing particular data. At least there's a workaround as you suggest (different signing subkeys per master key), but this seems unnecessarily restrictive except to maintain a simple API (which is not unreasonable by itself).

Yeah, I understand, but I'm just wondering if the use case is strong enough for adding these functions. What is your own use case for them?

Let me make sure that we don't need this. I was under the impression that we did for some arcane reason, but let me double check.

[trishankatdatadog]

Let me make sure that we don't need this. I was under the impression that we did for some arcane reason, but let me double check.

I checked, and we do have a valid use case, but it can be solved using a separate signing subkey per master key (which incidentally results in separation of duties). I have adapted the signing code to choose either the first unexpired signing subkey or the master key. Let me know what you think.

[twiss]

Thanks, looks good to me! 👍 I have just one tiny nitpick below.

[twiss]

Thanks! 🙏