Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: RSA encryption uses hardcoded exponent, ignores key's actual exponent #126

Open
wmjdgla opened this issue Jan 13, 2022 · 0 comments
Open

Comments

@wmjdgla
Copy link

wmjdgla commented Jan 13, 2022

Crypto::Encrypt stores the key's public part and exponent in RSA_KEY theKey and then passes it to RsaEncrypt:

RSA_KEY theKey;
theKey.publicKey = &rsaPubKeyBuf;
theKey.exponent = rsaParms->exponent;
UINT32 bufferSize = 4096;
BYTE encryptionBuffer[4096];
BYTE null { 0 };
const BYTE *encoding = &null;
if (!encodingParms.empty())
encoding = &encodingParms[0];
size_t encBlobSize = RsaEncrypt(&theKey, TPM_ALG_ID::OAEP, pubKey.nameAlg,

RsaEncrypt consumes the public part, but uses the hardcoded buffer BYTE exponent[] {1, 0, 1};, corresponding to 2^16 + 1, as the exponent:

BYTE exponent[] {1, 0, 1};
bn_mod = BN_bin2bn(key->publicKey->buffer, key->publicKey->size, NULL);
bn_exp = BN_bin2bn(exponent, 3, NULL);
keyX = RSA_new();
keyX->n = bn_mod;
keyX->e = bn_exp;

To fix, note that RsaEncrypt expects both the public part and exponent to be in big endian format. The TPM stores the public part as a raw byte buffer, so it remains big endian by default during unmarshaling and thus can be passed directly to RsaEncrypt. However the exponent is stored as a UINT32, so it'll be converted to little endian when unmarshaled - meaning we'll have to convert it to big endian before passing it to RsaEncrypt. To improve clarity, I would suggest to change the type of RSA_KEY struct's exponent member from UINT32 to a TPM2B just like its sibling members.

Also note that as per TPM Library Spec:

A TPM compatible with this specification and supporting RSA shall support two primes and an exponent of zero. An exponent of zero indicates that the exponent is the default of 2^16 + 1. Support for other values is optional.

So Crypto::Encrypt should check for this and set the exponent value accordingly before passing it to RsaEncrypt.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant