FlowCrypt / flowcrypt-browser

FlowCrypt Browser extension for Chrome and Firefox
https://flowcrypt.com
Other
373 stars 46 forks source link

Error "Could not find valid self-signature in key in key-id" #4556

Closed martgil closed 1 year ago

martgil commented 2 years ago

Description: An enterprise customer receives the following error: Could not find valid self-signature in \ when trying to use a certain private key. I check upon the key packets and it doesn't have a "Signature Packet" along with the other packets. That's all I've noticed so far so I tried reading the private key through custom code and got the same error.

image

dumped key info (redacted): ``` Old: Secret Key Packet(tag 5)(1862 bytes) Ver 4 - new Public key creation time - Tue May 2 12:20:12 PST 2017 Pub alg - RSA Encrypt or Sign(pub 1) RSA n(4096 bits) - ... RSA e(17 bits) - ... Sym alg - AES with 256-bit key(sym 9) Iterated and salted string-to-key(s2k 3): Hash alg - SHA1(hash 2) Salt - b7 34 33 89 6e b1 6e b8 Count - 4063232(coded count 191) IV - 8a 92 5c 56 80 e6 1d c7 c9 7f 00 b6 31 32 42 4e Encrypted RSA d Encrypted RSA p Encrypted RSA q Encrypted RSA u Encrypted SHA1 hash Old: User ID Packet(tag 13)(51 bytes) User ID - (JV) Old: User ID Packet(tag 13)(39 bytes) User ID - Old: User ID Packet(tag 13)(41 bytes) User ID - Old: Secret Subkey Packet(tag 7)(1862 bytes) Ver 4 - new Public key creation time - Tue May 2 12:20:14 PST 2017 Pub alg - RSA Encrypt or Sign(pub 1) RSA n(4096 bits) - ... RSA e(17 bits) - ... Sym alg - AES with 256-bit key(sym 9) Iterated and salted string-to-key(s2k 3): Hash alg - SHA1(hash 2) Salt - f7 1a 1f ac c1 52 cf db Count - 4063232(coded count 191) IV - f6 ae 89 43 05 a0 da 1e b7 9b ff f6 f5 38 04 57 Encrypted RSA d Encrypted RSA p Encrypted RSA q Encrypted RSA u Encrypted SHA1 hash ```

Reference: https://mail.google.com/mail/u/human@flowcrypt.com/#inbox/FMfcgzGpGwtXHZPgFNwtFHzJnXBQhChV

martgil commented 2 years ago

@rrrooommmaaa based on what I have known, the user had to set a primary user-id on the secret key? Is that correct?

Here is how I come up with that conclusion: $ pgpdump enterprise_secretkey.asc | grep primary # no text with keyword "primary" returned $ pgpdump my_secretkey.asc | grep primary # my key generated by openpgp.js
Hashed Sub: primary User ID(sub 25)(1 bytes)

A test from openpgp.js which helps me determine the assumption above: https://github.com/openpgpjs/openpgpjs/blob/bd1a7db46ff2bedff150866947a8058a7d0b78d7/test/general/key.js#L3410

rrrooommmaaa commented 2 years ago

According to RFC4880, p 12.1 self-signature isn't required for User ID, but it is required for each subkey

           Primary-Key
              [Revocation Self Signature]
              [Direct Key Signature...]
               User ID [Signature ...]
              [User ID [Signature ...] ...]
              [User Attribute [Signature ...] ...]
              [[Subkey [Binding-Signature-Revocation]
                      Primary-Key-Binding-Signature] ...]

The key can have 1 to ∞ User ID packets (0 to ∞ can be marked as primary -- "Primary UID" is a flag inside User ID packet). Is the problem only when trying to extract primary UID? Does the key have at least one User ID packet? Does the key have a Subkey packet?

martgil commented 2 years ago

I enumerated some other function calls that return an error "Could not find valid self-signature in key 3b64ccaea5646391" and here are they: // getDecryptionKeys() // getPrimaryUser() // getEncryptionKey() // getSigningKey()

Does the key have at least one User ID packet?

Yes, it has 3 to be exact.

Does the key have a Subkey packet?

It has a secret subkey packet but has an "Old" flag in it?

Old: Secret Subkey Packet(tag 7)(1862 bytes)
    Ver 4 - new
    Public key creation time - Tue May  2 12:20:14 PST 2017
    Pub alg - RSA Encrypt or Sign(pub 1)
    RSA n(4096 bits) - ...
    RSA e(17 bits) - ...
    Sym alg - AES with 256-bit key(sym 9)
    Iterated and salted string-to-key(s2k 3):
        Hash alg - SHA1(hash 2)
        Salt - f7 1a 1f ac c1 52 cf db 
        Count - 4063232(coded count 191)
    IV - f6 ae 89 43 05 a0 da 1e b7 9b ff f6 f5 38 04 57 
    Encrypted RSA d
    Encrypted RSA p
    Encrypted RSA q
    Encrypted RSA u
    Encrypted SHA1 hash

Btw, is there any chance that we also need to have the unlocked private key for troubleshooting?

rrrooommmaaa commented 2 years ago

Old: tells us that the format of the packet is old-style, this shouldn't matter.

It turns out that older software used to issue and use keys without self-certification signatures etc. (as allowed by RFC4880) Modern software (e.g. OpenPGP.js) actually requires self-certification (signature of type 0x10: Generic certification of a User ID and Public-Key packet). There is an instruction that might help adding it to the existing key (provided that the pass phrase for the private key is available): https://gpgtools.tenderapp.com/kb/gpg-keychain-faq/add-self-signature-to-an-old-key-which-does-not-have-one

tomholub commented 2 years ago

I'm adding more context. Customer is already using these keys with FlowCrypt. We have a special procedure for sig-less keys in the setup, where the key gets rewrapped and re-signed, with all proper signatures. Currently tested in setup - import key - fix key self signatures

I think that the customer is now newly trying to also import these types of keys in flowcrypt settings -> additional settings -> add key. Where we don't have the mechanism to fix the key, and therefore it fails.

@martgil can you confirm the key you received works when importing it during setup, but fails when adding it as additional key?

tomholub commented 2 years ago

Anyway I'm pretty sure this is the cause.

Notice in SetupImportKeyModule -> actionImportPrivateKeyHandle:

      } else if (e instanceof KeyCanBeFixed) {
        return await this.renderCompatibilityFixBlockAndFinalizeSetup(e.encrypted, options);

That was during setup. But have a look at key import:

      } else if (e instanceof KeyCanBeFixed) {
        return await Ui.modal.error(`This type of key cannot be set as additional keys yet. ${Lang.general.contactForSupportSentence(!!this.fesUrl)}`,
          false, Ui.testCompatibilityLink);

We just need to use a method similar to renderCompatibilityFixBlockAndFinalizeSetup when adding keys as well.

For the test: 1) set up extension without ekm with some normal key 2) add second key in settings, for this copy the armored key from 'setup - import key - fix uids' and reuse it in this test

ioanmo226 commented 2 years ago

@martgil Please forward this email to my email.

Reference: https://mail.google.com/mail/u/human@flowcrypt.com/#inbox/FMfcgzGpGwtXHZPgFNwtFHzJnXBQhChV

martgil commented 2 years ago

Hi Ioan, The email and key has been forwarded.

tomholub commented 2 years ago

@ioanmo226 please have a look at this issue this week. For automated test, there is already a setup test existing that has the "fix uids" procedure in it, used for initial setup.

You can design a test that uses a regular key for initial setup, and the key from that test when importing additional key in settings, to target this issue here.

ioanmo226 commented 2 years ago

I'm already into this and almost completed. :-) Implementation finished + test is 80-90% completed too.