Use StandardPolicy by default and allow NullPolicy when the user wants it

[kushaldas]

Right now the implementation uses NullPolicy through out the codebase. But, this is insecure (allows md5 or sha1) and should not be used. This will only required when dealing with old keys.

Any new key created using the module should be fine and use StandardPolicy as default.

We add a policy parameter while creating the KeyStore object, and only if None is used as argument, we should use the NullPolicy or else StandardPolicy as default.

[nwalfield]

My strong recommendation, as discussed elsewhere today, is to expose a safe-by-default API with as few knobs (in the vein of NaCL) as possible. Specifically, please don't use the NullPolicy, and avoid adding a policy parameter.

[kushaldas]

My strong recommendation, as discussed elsewhere today, is to expose a safe-by-default API with as few knobs (in the vein of NaCL) as possible. Specifically, please don't use the NullPolicy, and avoid adding a policy parameter.

As @nwalfield suggested, we should remove all references to the NullPolicy from the codebase. Now, in few cases if the user wants to interact using old keys or material where we have sha1, how to allow that path? We can add that via https://docs.sequoia-pgp.org/sequoia_openpgp/policy/struct.StandardPolicy.html#method.accept_hash adding only that hash into the policy. I will dig more.

[kushaldas]

After I moved the code to start using only StandardPolicy + sha1, I am getting this error for a test case.

    def test_decrypt_multiple_recipient_data():
        with open("tests/files/double_recipient.asc", "rb") as f:
            data = f.read()
    
        jp = jce.Johnny(_get_cert_data("tests/files/secret.asc"))
>       cleartext = jp.decrypt_bytes(data, "redhat")
E       ValueError: Failed to decrypt: Policy rejected symmetric encryption algorithm

Now, I generated this encrypted file using gpg2 executable.

❯ gpg2 --version
gpg (GnuPG) 2.2.20
libgcrypt 1.8.5

And the steps were as mentioned below:

# The following data was generated while encrypting for 2 UID. Then we will try to decrypt
# using the second secret key.
# ❯ gpg2 --homedir=/tmp/a --import tests/files/hellopublic.asc
# ❯ gpg2 --homedir=/tmp/a --import tests/files/public.asc
# ❯ gpg2 --homedir=/tmp/a  -r 8ADA07F0A0F7BA99 -r 209940B9669ED621 -e -a > tests/files/double_reipient.asc
# gpg: WARNING: unsafe permissions on homedir '/tmp/a'
# gpg: 3CE170115CF4322E: There is no assurance this key belongs to the named user

# sub  rsa4096/3CE170115CF4322E 2020-07-09 Test user <[email protected]>
# Primary key fingerprint: BB2D 3F20 2332 8637 1C31  23D5 2099 40B9 669E D621
# Subkey fingerprint: 9EBF CA46 5490 663C 22AE  F144 3CE1 7011 5CF4 322E

# It is NOT certain that the key belongs to the person named
# in the user ID.  If you *really* know what you are doing,
# you may answer the next question with yes.

# Use this key anyway? (y/N) y
# gpg: 76E7E83323D9A3AF: There is no assurance this key belongs to the named user

# sub  rsa4096/76E7E83323D9A3AF 2020-10-02 test key
# Primary key fingerprint: 6AC6 957E 2589 CB8B 5221  F650 8ADA 07F0 A0F7 BA99
# Subkey fingerprint: 3080 B349 B5C2 CF8E 1869  B2F5 76E7 E833 23D9 A3AF

# It is NOT certain that the key belongs to the person named
# in the user ID.  If you *really* know what you are doing,
# you may answer the next question with yes.

# Use this key anyway? (y/N) y
# Hello World! for 2.

Now, if I encrypt the text using sq, and replace the test data file, the test passes.

./target/debug/sq encrypt -o double.asc --recipient-key-file ../rust/johnnycanencrypt/tests/files/store/hellopublic.asc --recipient-key-file ../rust/johnnycanencrypt/tests/files/store/public.asc msg.txt 

I think this means gpg2 is choosing a weak (or maybe broken algorithm) and sq is not.

Update: It seems gpg2 is using TripleDES as the symmetric encryption algorithm.

If we don't allow, then it means we can not decrypt any of the old encrypted files via python-gpg.

[nwalfield]

Trying it out using sq, I see:

$ sq decrypt --secret-key-file tests/files/secret.asc tests/files/double_recipient.asc
Enter password to decrypt key Test user [email protected] (2099 40B9 669E D621):
Error: Decryption failed

Caused by:
0: Policy rejected symmetric encryption algorithm
1: TripleDES (EDE-DES, 168 bit key derived from 192)) is not considered secure since 2017-01-01T00:00:00Z

So, it appears that the message is encrypted using 3DES. NIST deprecated 3DES in 2017.

If we don't allow, then it means we can not decrypt any of the old encrypted files via python-gpg.

Perhaps you can't. But, I'm not sure why this is so bad. Is Secure Drop really being used to read old messages?

[kushaldas]

Trying it out using sq, I see:

$ sq decrypt --secret-key-file tests/files/secret.asc tests/files/double_recipient.asc
Enter password to decrypt key Test user [email protected] (2099 40B9 669E D621):
Error: Decryption failed
Caused by:
0: Policy rejected symmetric encryption algorithm
1: TripleDES (EDE-DES, 168 bit key derived from 192)) is not considered secure since 2017-01-01T00:00:00Z

So, it appears that the message is encrypted using 3DES. NIST deprecated 3DES in 2017.

Thank you for this example, now I learned how to get better errors. I read the source of StandardPolicy and added missing TripleDES to verify that it was being used.

If we don't allow, then it means we can not decrypt any of the old encrypted files via python-gpg.

Perhaps you can't. But, I'm not sure why this is so bad. Is Secure Drop really being used to read old messages?

Yes, many times the sources are communicating over many months. Or people may want to use this to decrypt something encrypted many years ago.

@nwalfield can we allow TripleDES for the decrypt step only? Will that be a security compromise?

Also it is sad to see that gpg2 is using TripleDES even in 2020.

[nwalfield]

Looking at tests/files/hellopublic.asc, there is a Hash preferences subpacket, but not Symmetric Algorithm preferences subpacket, which would explain why gpg is falling back to 3DES. How did you create that key?

$ sq packet dump tests/files/hellopublic.asc
...
User ID Packet, new CTB, 8 bytes
Value: test key

Signature Packet, new CTB, 575 bytes
Version: 4
Type: PositiveCertification
Pk algo: RSA (Encrypt or Sign)
Hash algo: SHA512
Hashed area:
Signature creation time: 2020-10-02 13:48:48 UTC (critical)
Hash preferences: SHA512
Primary User ID: true (critical)
Key flags: C (critical)
Features: MDC
Unhashed area:
Issuer: 8ADA 07F0 A0F7 BA99
Issuer Fingerprint: 6AC6 957E 2589 CB8B 5221 F650 8ADA 07F0 A0F7 BA99
Digest prefix: CAA0
Level: 0 (signature over data)
...

[nwalfield]

FWIW, I just created a key with 'sq key generate' and the Symmetric algo preferences subpacket is present.

$ sq --force key generate --userid 'Foo' --export /tmp/foo.pgp
$ sq packet dump /tmp/foo.pgp
Secret-Key Packet, new CTB, 88 bytes
...
User ID Packet, new CTB, 3 bytes
Value: Foo

Signature Packet, new CTB, 140 bytes
...
Symmetric algo preferences: AES256, AES128

[kushaldas]

Looking at tests/files/hellopublic.asc, there is a Hash preferences subpacket, but not Symmetric Algorithm preferences subpacket, which would explain why gpg is falling back to 3DES. How did you create that key?

I used this call https://github.com/kushaldas/johnnycanencrypt/blob/master/src/lib.rs#L378 , maybe I must add the symmetric algo preference there.

[kushaldas]

I just now created another set of keys, and they also have proper Symmetric algo preferences: AES256, AES128 values. I wonder if anything changed in sequoia from 0.17.0 release.

[nwalfield]

maybe I must add the symmetric algo preference there.

The CertBuilder adds the appropriate subpacket. See https://docs.sequoia-pgp.org/src/sequoia_openpgp/cert/builder.rs.html#1094 .

I wonder if anything changed in sequoia from 0.17.0 release.

Yes, there was a bug. The preferred algorithm packets were added to the subkey binding signatures instead of the user id self signatures.

[kushaldas]

Yes, there was a bug. The preferred algorithm packets were added to the subkey binding signatures instead of the user id self signatures.

Ah, this explains. Means I just have to recreate the keys, and update all tests now :)

[nwalfield]

It was fixed in this commit: https://gitlab.com/sequoia-pgp/sequoia/-/commit/87b02b2bae6cc8ee838c8f46208a56339ebf3316