ProtonMail / gopenpgp

A high-level OpenPGP library
https://gopenpgp.org
MIT License
1.06k stars 116 forks source link

Undisclosed v3 breaking change: KeyRing.Decrypt was removed #313

Open hpurmann opened 1 week ago

hpurmann commented 1 week ago

Hey all!

We are currently using v2 and make use of the KeyRing.Decrypt function. This function has disappeared in v3, but it's not mentioned in the changelog.

I'm wondering if this was a mistake or deliberate. And if deliberate, how can we adapt our implementation to fit v3?

lubux commented 1 week ago

Hi 👋 All OpenPGP operations can now only be executed via the new API. Reasoning:

GopenPGP v3 introduces a new unified API for high level OpenPGP operations. In comparison to GopenPGP v2, where similar functions were dispersed across different types and required varying implementations for the same operations, GopenPGP v3 consolidates these functions into a consistent interface. Now, operations such as Sign, Verify, Encrypt, Decrypt, and Key generation are each accessible through a unified, builder like API, simplifying integration and enhancing code readability across cryptographic workflows.

So for KeyRing.Decrypt it would be:

keyRing := ...
pgp := PGP()
decHandle, err := pgp.
    Decryption().
    DecryptionKeys(keyRing).
    VerificationKeys(...). // optional if signatures should be verified 
    VerifyTime(...). // optional if the verification time should be something else than the local time
    New()
if err != nil {
    panic(err)
}
decrypted, err := decHandle.Decrypt([]byte(armoredMessage), crypto.Armor)
if err != nil {
    panic(err)
}
fmt.Println(decrypted.String())
hpurmann commented 1 week ago

Thank you @lubux, that worked. I see that your quote is from the releases page. My bad, I only looked at the Changelog file.

hpurmann commented 1 week ago

Hey @lubux! The code change worked in CI (unit tests are decrypting a local file with a local key/passphrase) but it crashed on production with

gopenpgp: decrypting message with private keys failed: openpgp: incorrect key

I'm wondering where the difference in behavior might be. Have you seen any regression between v2 and v3 related to that?

With v2 we're using the library like this:

pgpMessage, err := crypto.NewPGPMessageFromArmored(string(bytes))
if err != nil {
    return nil, err
}

message, err := keyring.Decrypt(pgpMessage, nil, 0)
if err != nil {
    return nil, err
}
keyring.ClearPrivateParams()

return message.Data, nil

and in v3 we're doing

pgp := crypto.PGP()
decHandle, err := pgp.Decryption().DecryptionKeys(keyring).New()
if err != nil {
    return nil, err
}

message, err := decHandle.Decrypt(bytes, crypto.Armor)
if err != nil {
    return nil, err
}
keyring.ClearPrivateParams()

return message.Bytes(), nil
JasonQuinn commented 1 week ago

@hpurmann I am also seeing the same when testing this, some files work ok and some others using the same key return the openpgp: incorrect key error

lubux commented 4 days ago

Hi 👋 This error indicates that there is no matching valid key available for decrypting the message.
GopenPGP v3 introduces a new go-crypto fork API that improves interoperability, but it also modifies the core logic.

In version 3, decryption now involves checking the decryption keys for validity, if they are not valid they are not considered. Consequently, some subkeys might have invalid or outdated signatures or use a blocked hash functions. Do you have an example public key of a decryption key that is failing?

JasonQuinn commented 3 days ago

@lubux So they key is able to decrypt some of the files but not others using v3 but gpg on the cmd line can decrypt them all so they key seems fine to me. V2 is failing some files with "openpgp: invalid data: tag byte does not have MSB set" which works ok in V3, but other files which fail in V3 with "openpgp: incorrect key" work ok in V2. Unfortunately the only examples I have of it breaking are for production data so i can't share the key/files.

JasonQuinn commented 3 days ago

I figured out what the difference was. For the files which worked in V2 but failed in V3 the file was encrypted using the primary key instead of the sub key so i guess that was allowed to be decrypted in v2 but changed for v3?

lubux commented 2 days ago

Thanks for checking this. GopenPGP v3 does allow to decrypt with the primary key if it is marked as encryption capable. However, if it fails it means the primary key is most likely marked as signing key only. We are considering adding a flag to allow decryption with signing keys (https://github.com/ProtonMail/go-crypto/pull/251), if the client accepts the security risk. Using the same key for encryption and signing reduces security.

JasonQuinn commented 2 days ago

That makes sense, thank you