ProtonMail / go-crypto

Fork of go/x/crypto, providing an up-to-date OpenPGP implementation
https://pkg.go.dev/github.com/ProtonMail/go-crypto
BSD 3-Clause "New" or "Revised" License
328 stars 99 forks source link

openpgp.CheckDetachedSignature failing when it was succeeding with a previous version #187

Closed ncw closed 10 months ago

ncw commented 10 months ago

We use this library in rclone to validate the downloads for rclone's self update feature - thank you :-)

Unfortunately in this commit 5503f24f010878669817f6dd4fc0d2bccd67bcd0 by @twiss openpgp.CheckDetachedSignature stopped working for us (commit found with git bisect).

commit 5503f24f010878669817f6dd4fc0d2bccd67bcd0
Author: Daniel Huigens
Date:   Tue Mar 21 16:03:18 2023 +0100

    Require key flags to use keys (#155)

    Don't allow using primary keys without a valid self-signature with
    key flags indicating the usage is allowed.

 openpgp/keys.go | 25 +++++++++++++------------

Reverting this commit on top of main causes openpgp.CheckDetachedSignature to work for us again.

The validation code is here: https://github.com/rclone/rclone/blob/master/cmd/selfupdate/verify.go and contains the PGP key in use.

This has worked for ages (since March 2021), first with x/crypto/pgp and then with your replacement package which we've been using since 24 June 2022 as x/crypto/pgp is no longer maintained.

Problems could be caused by:

If I could fix this by updating the signatures on the server that would be perfect!

The signed messages verify with gpg if I import just that key into a new homedir and do gpg --homedir /tmp/gpg --decrypt build/MD5SUMS

Though there is a warning there - maybe that is what this is about.

gpg: Signature made Tue 17 Oct 2023 18:13:58 BST
gpg:                using DSA key FBF737ECE9F8AB18604BD2AC93935E02FF3B54FA
gpg: Good signature from "Nick Craig-Wood <nick@craig-wood.com>" [unknown]
gpg: WARNING: This key is not certified with a trusted signature!
gpg:          There is no indication that the signature belongs to the owner.

Any help appreciated!

twiss commented 10 months ago

Hi @ncw :wave: The issue is indeed with the key. Currently it doesn't have any key flags listing the allowed usages, and we switched from allowing all usages by default to disallowing them.

If you have the private key, it should be possible to fix this by editing the key with gpg (https://unix.stackexchange.com/a/707006), and enabling the sign capability. (You may, however, have to backdate the new binding signature to a date before the signatures were created in order to make existing signatures valid. For that, you might be able to use faketime or similar to set the date before running the command, though I haven't tested this.)

Let me know if that works for you, if not we can consider adding a config option for this.

P.S. If you're able, it may be worth generating a new key entirely, as it's a 1024-bit DSA key which is not that much nowadays :)

ncw commented 10 months ago

The issue is indeed with the key. Currently it doesn't have any key flags listing the allowed usages, and we switched from allowing all usages by default to disallowing them.

Ah ha!

If you have the private key, it should be possible to fix this by editing the key with gpg (https://unix.stackexchange.com/a/707006), and enabling the sign capability. (You may, however, have to backdate the new binding signature to a date before the signatures were created in order to make existing signatures valid. For that, you might be able to use faketime or similar to set the date before running the command, though I haven't tested this.)

Let me know if that works for you, if not we can consider adding a config option for this.

Interestingly when I try to edit the key with gpg it claims it already has the SCA Sign Certify Authenticate usage

ec  dsa1024/93935E02FF3B54FA
     created: 2001-09-27  expires: never       usage: SCA 
     trust: ultimate      validity: ultimate
ssb  elg2048/F8A0E5D36A4DDF21
     created: 2001-09-27  expires: never       usage: E   
[ultimate] (1)* Nick Craig-Wood <nick@craig-wood.com>

gpg> change-usage
Changing usage of the primary key.

Possible actions for a DSA key: Sign Certify Authenticate 
Current allowed actions: Sign Certify Authenticate 

   (S) Toggle the sign capability
   (A) Toggle the authenticate capability
   (Q) Finished

So it looks like the key is correct already according to GPG. I wonder if the key flags shouldn't apply to DSA keys?

I tried toggling the S capability then adding it back again, but openpgp.CheckDetachedSignature is giving the same error openpgp: signature made by unknown entity.

When I try to use faketime the signing hangs forever. I guess this is something to do with the communication with gpg-agent. I tried it will gpg-agent killed but no difference. You don't seem to be able to use gpg without the agent alas.

Can I patch the key after I've loaded it into memory?

I tried this sort of thing but I couldn't get it to work

    keyRing, err := openpgp.ReadArmoredKeyRing(strings.NewReader(ncwPublicKeyPGP))
    if err != nil {
        return fmt.Errorf("unsupported signing key: %w", err)
    }
    for i, entity := range keyRing {
        for j, subkey := range entity.Subkeys {
            subkey.Sig.FlagsValid = true
            // subkey.Sig.FlagCertify = true
            subkey.Sig.FlagSign = true
            // subkey.Sig.FlagEncryptCommunications = true
            // subkey.Sig.FlagEncryptStorage = true
            // subkey.Sig.FlagSplitKey = true
            // subkey.Sig.FlagAuthenticate = true
            // subkey.Sig.FlagGroupKey = true
        }
    }

P.S. If you're able, it may be worth generating a new key entirely, as it's a 1024-bit DSA key which is not that much nowadays :)

I have a new key already, I just haven't figured out the transition strategy yet so I need the old one to work for just a little longer!

Thank you for your very useful and informative reply.

andrewgdotcom commented 10 months ago

So it looks like the key is correct already according to GPG. I wonder if the key flags shouldn't apply to DSA keys?

The key algorithm shouldn't make any difference here.

I tried toggling the S capability then adding it back again, but openpgp.CheckDetachedSignature is giving the same error openpgp: signature made by unknown entity.

Did you save and exit gnupg between removing the S capability and adding it back? gnupg doesn't make any actual changes until you save.

twiss commented 10 months ago

If you don't get it to work in gpg, doing it with go-crypto itself should indeed also be possible, you just need to set entity.Identities["Nick Craig-Wood"].SelfSignature.FlagSign and .FlagsValid. (The subkey is for encryption, instead.) Then you can call SerializePrivate in order to re-sign the binding signatures, and then call Serialize to get the public key. Or indeed you can just set those properties on the key in memory, that should also work.

ncw commented 10 months ago

I tried toggling the S capability then adding it back again, but openpgp.CheckDetachedSignature is giving the same error openpgp: signature made by unknown entity.

Did you save and exit gnupg between removing the S capability and adding it back? gnupg doesn't make any actual changes until you save.

I thought I had (in fact I did it twice previously) but I tried it again and this time it worked! For the record I did

I then repeated that to toggle the signing back on.

I also verified that the replacement key I made works fine without any --edit-key so as you said, its my old key that is the problem.

So thank you @andrewgdotcom and @twiss for your help - this gives me enough to work with to fix the rclone problem :-)

I'll close this now.