golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.28k stars 17.7k forks source link

proposal: crypto/rsa: allow hash.Hash for OAEP and MGF1 to be specified independently for encryption #65716

Open mmauv opened 9 months ago

mmauv commented 9 months ago

Proposal Details

It is currently impossible to independently choose the hash functions used by rsa.EncryptOAEP for OAEP and MGF1. The issue was already raised in #19974; however, it has only been fixed in the decryption functions.

This functionality is needed to wrap keys for the Android Keystore secure import. The Android developer documentation specifies that encryptedTransportKey is a 256-bit AES key, [...] encrypted in RSA-OAEP mode (SHA-256 digest, SHA-1 MGF1 digest) (https://developer.android.com/reference/android/security/keystore/WrappedKeyEntry). This specification requires being able to encrypt using RSA-OAEP with different algorithms for OAEP and MGF1.

gopherbot commented 9 months ago

Change https://go.dev/cl/564755 mentions this issue: crypto/rsa: allow hash.Hash for OAEP and MGF1 to be specified independently for encryption

mauri870 commented 9 months ago

cc @FiloSottile @rolandshoemaker

feyounger commented 9 months ago

The current go has not been modified yet. You can copy the required functions from the crypto/rsa package and adjust them rsa-ecb.

mmauv commented 9 months ago

The current go has not been modified yet. You can copy the required functions from the crypto/rsa package and adjust them rsa-ecb.

Thanks, that's the only solution I found while waiting for the change...

rsc commented 2 months ago

This proposal has been added to the active column of the proposals project and will now be reviewed at the weekly proposal review meetings. — rsc for the proposal review group

rolandshoemaker commented 2 months ago

It seems like what we probably want to do here is to add a PublicKey.Encrypt method that can then mirror the PrivateKey.Decrypt method, which can then take the pre-existing OAEPOptions struct.

Concretely the API change would be:

func (*PublicKey) Encrypt(rand io.Reader, plaintext []byte, opts crypto.EncrypterOpts) (ciphertext []byte, err error)

This would also require that we define crypto.EncrypterOpts in crypto as any. Possibly we may also want to define a crypto.Encrypter interface, to match crypto.Decrypter?

rsc commented 2 months ago

/cc @FiloSottile

aclements commented 2 months ago

It sounds like there's some need for this, albeit niche. There's still a question of whether we want to do this or not. We should check if Android actually cares about the MGF1 hash.

What would the documentation be for (*PublicKey).Encrypt? How would the documentation for the top-level EncryptOAEP function change?

gopherbot commented 4 days ago

Change https://go.dev/cl/630655 mentions this issue: crypto/internal/fips140/rsa: support separate MGF1 hash for EncryptOAEP