RSA encryption padding change from PKCS1Padding to OAEPWithSHA-256And… #834
Conversation
![semgrepcode-auth0[bot]](https://avatars.githubusercontent.com/in/977730?s=60&v=4){kind=small}
| "("+ OLD_PKCS1_RSA_TRANSFORMATION +") for migration due to " + | ||
| "InvalidKeyException with OAEP."); | ||
| KeyStore.PrivateKeyEntry rsaKeyEntry = getRSAKeyEntry(); | ||
| Cipher rsaPkcs1Cipher = Cipher.getInstance(OLD_PKCS1_RSA_TRANSFORMATION); |
There was a problem hiding this comment.
Semgrep identified an issue in your code:
Weak cryptographic algorithm detected. The DES, 3DES, RC2, RC4, MD4, MD5, SHA1, BLOWFISH, Dual_EC_DRBG, and SHA1PRNG algorithms are considered insecure and should not be used. Using weak cryptographic algorithms can compromise the confidentiality and integrity of sensitive data. It is recommended to adopt only cryptographic features and algorithms offered by the Android platform that are internationally recognized as strong. It is also fundamental to ensure that the encryption parameters (crypto key, IV, etc.) are generated randomly using a cryptographically strong PRNG function. In addition, if it is needed to store an encryption parameter on the device, a secure storage mechanism like the Android KeyStore must be used.
To resolve this comment:
🔧 No guidance has been designated for this issue. Fix according to your organization's approved methods.
💬 Ignore this finding
Reply with Semgrep commands to ignore this finding.
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by android_weak_cryptographic_algorithm.
You can view more details about this finding in the Semgrep AppSec Platform.
| Log.d(TAG, "Attempting to decrypt AES key with PKCS1Padding ("+ | ||
| OLD_PKCS1_RSA_TRANSFORMATION +") for migration."); | ||
| KeyStore.PrivateKeyEntry rsaKeyEntry = getRSAKeyEntry(); | ||
| Cipher rsaPkcs1Cipher = Cipher.getInstance(OLD_PKCS1_RSA_TRANSFORMATION); |
There was a problem hiding this comment.
Semgrep identified an issue in your code:
Weak cryptographic algorithm detected. The DES, 3DES, RC2, RC4, MD4, MD5, SHA1, BLOWFISH, Dual_EC_DRBG, and SHA1PRNG algorithms are considered insecure and should not be used. Using weak cryptographic algorithms can compromise the confidentiality and integrity of sensitive data. It is recommended to adopt only cryptographic features and algorithms offered by the Android platform that are internationally recognized as strong. It is also fundamental to ensure that the encryption parameters (crypto key, IV, etc.) are generated randomly using a cryptographically strong PRNG function. In addition, if it is needed to store an encryption parameter on the device, a secure storage mechanism like the Android KeyStore must be used.
To resolve this comment:
🔧 No guidance has been designated for this issue. Fix according to your organization's approved methods.
💬 Ignore this finding
Reply with Semgrep commands to ignore this finding.
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by android_weak_cryptographic_algorithm.
You can view more details about this finding in the Semgrep AppSec Platform.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| // https://developer.android.com/reference/javax/crypto/Cipher.html | ||
| @SuppressWarnings("SpellCheckingInspection") | ||
| private static final String AES_TRANSFORMATION = "AES/GCM/NOPADDING"; | ||
| private static final String OLD_PKCS1_RSA_TRANSFORMATION = "RSA/ECB/PKCS1Padding"; |
Check failure
Code scanning / CodeQL
Use of RSA algorithm without OAEP High
Copilot Autofix
AI 29 days ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
There was a problem hiding this comment.
that was the PR which we are fixing to use OAEP padding.
| "("+ OLD_PKCS1_RSA_TRANSFORMATION +") for migration due to " + | ||
| "InvalidKeyException with OAEP."); | ||
| KeyStore.PrivateKeyEntry rsaKeyEntry = getRSAKeyEntry(); | ||
| Cipher rsaPkcs1Cipher = Cipher.getInstance(OLD_PKCS1_RSA_TRANSFORMATION); |
Check failure
Code scanning / CodeQL
Use of RSA algorithm without OAEP High
| Log.d(TAG, "Attempting to decrypt AES key with PKCS1Padding ("+ | ||
| OLD_PKCS1_RSA_TRANSFORMATION +") for migration."); | ||
| KeyStore.PrivateKeyEntry rsaKeyEntry = getRSAKeyEntry(); | ||
| Cipher rsaPkcs1Cipher = Cipher.getInstance(OLD_PKCS1_RSA_TRANSFORMATION); |
Check failure
Code scanning / CodeQL
Use of RSA algorithm without OAEP High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 29 days ago
To address the issue, the fallback decryption logic using OLD_PKCS1_RSA_TRANSFORMATION should be replaced with a secure alternative. Since the fallback is for legacy data, the best approach is to migrate the data securely to use OAEP padding. This can be achieved by decrypting the data with PKCS1 (if necessary) and immediately re-encrypting it with OAEP. This ensures that future operations do not rely on the insecure PKCS1 padding.
The specific changes involve:
- Replacing the use of
OLD_PKCS1_RSA_TRANSFORMATIONwith a secure OAEP-based transformation. - Adding logic to re-encrypt the decrypted data with OAEP after successful decryption with PKCS1.
- Logging the migration process to ensure traceability.
| @@ -422,5 +422,16 @@ | ||
| rsaPkcs1Cipher.init(Cipher.DECRYPT_MODE, rsaKeyEntry.getPrivateKey()); | ||
| decryptedAESKey = rsaPkcs1Cipher.doFinal(encryptedAESBytes); | ||
| byte[] legacyDecryptedAESKey = rsaPkcs1Cipher.doFinal(encryptedAESBytes); | ||
| Log.i(TAG, "Successfully decrypted legacy AES key using PKCS1Padding." | ||
| + " Migration is needed."); | ||
| + " Re-encrypting with OAEP for migration."); | ||
|
|
||
| // Re-encrypt the key with OAEP | ||
| Cipher rsaOaepCipher = Cipher.getInstance(RSA_TRANSFORMATION); | ||
| rsaOaepCipher.init(Cipher.ENCRYPT_MODE, rsaKeyEntry.getCertificate().getPublicKey()); | ||
| decryptedAESKey = rsaOaepCipher.doFinal(legacyDecryptedAESKey); | ||
|
|
||
| // Update storage with the re-encrypted key | ||
| String reEncryptedAESKey = Base64.encodeToString(decryptedAESKey, Base64.DEFAULT); | ||
| storage.storeString(KEY_ALIAS, reEncryptedAESKey); | ||
|
|
||
| Log.i(TAG, "Successfully migrated AES key to OAEP."); | ||
| migrationNeeded = true; |
There was a problem hiding this comment.
that was the PR which we are fixing to use OAEP padding.
RSA encryption padding change from PKCS1Padding to OAEPWithSHA-256And
Changes
Updated padding for RSA encryption from PKCS1Padding to OAEPWithSHA-256And also checked for migration for the same.
References
Testing
Please describe how this can be tested by reviewers. Be specific about anything not tested and reasons why. Since this library has unit testing, tests should be added for new functionality and existing tests should complete without errors.
[X ] This change adds unit test coverage
[X ] This change adds integration test coverage
[ X] This change has been tested on the latest version of the platform/language or why not
Checklist
I have read the Auth0 general contribution guidelines
I have read the Auth0 Code of Conduct
All existing and new tests complete without errors