RSA encryption padding change from PKCS1Padding to OAEPWithSHA-256And… #834

utkrishtsahu · 2025-05-19T05:13:19Z

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

semgrepcode-auth0 · 2025-05-19T05:14:28Z

auth0/src/main/java/com/auth0/android/authentication/storage/CryptoUtil.java

+                                "("+ OLD_PKCS1_RSA_TRANSFORMATION +") for migration due to " +
+                                "InvalidKeyException with OAEP.");
+                        KeyStore.PrivateKeyEntry rsaKeyEntry = getRSAKeyEntry();
+                        Cipher rsaPkcs1Cipher = Cipher.getInstance(OLD_PKCS1_RSA_TRANSFORMATION);


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.}

The primary digest used for the RSA-OAEP padding scheme is SHA-256, which is secure.

SHA1 is included only as a supported digest for the Mask Generation Function (MGF1), an internal component of OAEP.

This is a standard practice to ensure maximum compatibility across different Android OS versions and hardware security modules, as some implementations require SHA1 to be available for MGF1.

semgrepcode-auth0 · 2025-05-19T05:14:29Z

auth0/src/main/java/com/auth0/android/authentication/storage/CryptoUtil.java

+                        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);


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.}

The primary digest used for the RSA-OAEP padding scheme is SHA-256, which is secure.

SHA1 is included only as a supported digest for the Mask Generation Function (MGF1), an internal component of OAEP.

This is a standard practice to ensure maximum compatibility across different Android OS versions and hardware security modules, as some implementations require SHA1 to be available for MGF1.

auth0/src/main/java/com/auth0/android/authentication/storage/CryptoUtil.java

Copilot

Pull Request Overview

This PR updates the RSA encryption padding from PKCS1Padding to OAEPWithSHA-256AndMGF1Padding for improved security. The change includes backward compatibility handling to migrate existing PKCS1-encrypted AES keys to the new OAEP format.

Changes RSA transformation from PKCS1Padding to OAEPWithSHA-256AndMGF1Padding
Increases RSA key size from 2048 to 4096 bits
Implements migration logic for existing PKCS1-encrypted AES keys to maintain compatibility

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
CryptoUtil.java	Updates RSA transformation, key size, and implements migration logic for PKCS1 to OAEP transition
CryptoUtilTest.java	Refactors tests to support new RSA transformation and adds migration test coverage

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-08-19T06:55:53Z

auth0/src/main/java/com/auth0/android/authentication/storage/CryptoUtil.java

    private static final String ALGORITHM_AES = "AES";
    private static final int AES_KEY_SIZE = 256;
-    private static final int RSA_KEY_SIZE = 2048;
+    private static final int RSA_KEY_SIZE = 4096;


Increasing RSA key size from 2048 to 4096 bits will significantly impact performance for key generation and encryption/decryption operations. Consider whether this change is necessary for security requirements or if 2048 bits is sufficient for the application's threat model.

Suggested change

private static final int RSA_KEY_SIZE = 4096;

private static final int RSA_KEY_SIZE = 2048;

Why do we need to increase the size ? Was this suggested by the security team ?

yes suggested by security team https://auth0team.atlassian.net/browse/SEC-4858?assignee=712020%3A746f4583-dada-45dc-8f76-07f273104490&selectedIssue=SDK-6001

Copilot · 2025-08-19T06:55:53Z

auth0/src/main/java/com/auth0/android/authentication/storage/CryptoUtil.java

+                storage.store(KEY_ALIAS, newEncodedEncryptedAES);
+                storage.remove(OLD_KEY_ALIAS);
+                return decryptedAESKey;
+            } catch (Exception e) {


Catching generic Exception is too broad and may hide specific error conditions. Consider catching specific exceptions like BadPaddingException, IllegalBlockSizeException, NoSuchAlgorithmException, etc., to handle different failure scenarios appropriately.

Suggested change

} catch (Exception e) {

} catch (NoSuchAlgorithmException | NoSuchPaddingException | InvalidKeyException | BadPaddingException | IllegalBlockSizeException | KeyStoreException | UnrecoverableEntryException e) {

Copilot · 2025-08-19T06:55:54Z

auth0/src/main/java/com/auth0/android/authentication/storage/CryptoUtil.java

-            Log.e(TAG, "Error while creating the AES key.", e);
+            Log.e(TAG, "Error while creating the new AES key.", e);
            throw new IncompatibleDeviceException(e);
+        } catch (Exception e) {


Catching generic Exception is too broad. This catch block should handle specific exceptions that can occur during RSA encryption or storage operations, such as CryptoException or IncompatibleDeviceException.

Suggested change

} catch (Exception e) {

} catch (InvalidKeyException

| NoSuchPaddingException

| IllegalBlockSizeException

| BadPaddingException

| KeyStoreException

| UnrecoverableEntryException

| CertificateException

| IOException

| ProviderException e) {

pmathew92 · 2025-08-19T06:58:55Z

auth0/src/main/java/com/auth0/android/authentication/storage/CryptoUtil.java

    // 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";


Add a comment to state why OLD_PKCS1_RSA_TRANSFORMATION is being used

pmathew92 · 2025-08-19T07:03:02Z

auth0/src/main/java/com/auth0/android/authentication/storage/CryptoUtil.java

                        .setKeySize(RSA_KEY_SIZE)
-                        .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_RSA_PKCS1)
+                        .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_RSA_OAEP)
+                        .setDigests(KeyProperties.DIGEST_SHA256, KeyProperties.DIGEST_SHA1)


Hope this setDigest addition was also reviewed by security ?

pmathew92 · 2025-08-19T09:04:06Z

@utkrishtsahu The current implementation doesn't handle migration for existing users token . Please handle that and ensure it doesn't break

…gration logic for existing user

auth0/src/main/java/com/auth0/android/authentication/storage/CryptoUtil.java

    // https://developer.android.com/training/articles/keystore.html#SupportedCiphers
-    private static final String RSA_TRANSFORMATION = "RSA/ECB/PKCS1Padding";
+    private static final String RSA_TRANSFORMATION = "RSA/ECB/OAEPWithSHA-256AndMGF1Padding";
+    private static final String OLD_PKCS1_RSA_TRANSFORMATION = "RSA/ECB/PKCS1Padding";


The problem is the use of "RSA/ECB/PKCS1Padding" in any Cipher.getInstance call, specifically at lines 408 and 446, in conjunction with OLD_PKCS1_RSA_TRANSFORMATION. The correct approach is to only use "RSA/ECB/OAEPWithSHA-256AndMGF1Padding" (already present as RSA_TRANSFORMATION).

However, decryption of data that was encrypted previously using PKCS1 cannot be completed without using PKCS1. Thus, if backward compatibility and migration are critical, we must minimize the PKCS1 window:

Use PKCS1 decryption only inside strictly-controlled migration routines,

Re-encrypt immediately using OAEP,

Remove old (PKCS1) storage as soon as possible,

Never use PKCS1 padding except for this tightly bounded migration path.

The current code already follows this pattern: PKCS1 is only used for decryption in migration code, and the re-encryption uses OAEP immediately after. This is acceptable if legacy support is necessary, and not fixable without breaking backward compatibility. But if possible (e.g., on a new install, or after some period), the migration code and any PKCS1 support should be removed entirely.

Action:

If you can guarantee there is no PKCS1-wrapped ciphertext left (i.e., all users/data migrated), remove all code that uses PKCS1 padding entirely.

If not, restrict PKCS1 use to the minimum as in the current code, and clearly document that PKCS1 padding is only used for migration, which will be phased out.

For stricter security, optionally throw/log a warning if PKCS1 is ever used and include a feature flag to disable PKCS1 decrypt migration.

We can also clarify in the code with comments, and (optionally) add a runtime check to refuse decryption with PKCS1 unless in a migration context. If you implement version checks etc., do it here.

In summary: No code changes necessary outside of documentation, deprecation warnings, or removing migration code when migration is complete. Full OAEP-only support is best; PKCS1 use is strictly for necessary, one-time migration.

auth0/src/main/java/com/auth0/android/authentication/storage/CryptoUtil.java

+
+                        if (rsaKey != null && keyAliasUsed != null) {
+                            // Decrypt using OLD PKCS1 padding
+                            Cipher rsaPkcs1Cipher = Cipher.getInstance(OLD_PKCS1_RSA_TRANSFORMATION);


The core flag is at:

Cipher rsaPkcs1Cipher = Cipher.getInstance(OLD_PKCS1_RSA_TRANSFORMATION);

The context makes this necessary for decryption during migration from old (insecure) data. The code then immediately re-encrypts using RSAEncrypt, which should internally use OAEP padding (this logic is correct).

Required fix:

Add a warning comment to clarify that the use of PKCS1 here is intentional, only for decryption of legacy data, and should never be copied for encryption.

Make sure any nearby encryption operations (such as RSAEncrypt) use OAEP and, if not, update them.

If possible within the snippet, add a comment or refactor variable names to reduce the risk of pattern duplication elsewhere.

Imports, methods, and definitions:

No new imports required.

Only comment additions/clarification within the given snippet.

If the OLD_PKCS1_RSA_TRANSFORMATION definition is present in the snippet, comment its meaning.

auth0/src/main/java/com/auth0/android/authentication/storage/CryptoUtil.java

+            try {
+                byte[] encryptedOldAESBytes = Base64.decode(encodedOldAES, Base64.DEFAULT);
+                KeyStore.PrivateKeyEntry rsaKeyEntry = getRSAKeyEntry();
+                Cipher rsaPkcs1Cipher = Cipher.getInstance(OLD_PKCS1_RSA_TRANSFORMATION);


To fix the vulnerability, ensure RSA encryption and decryption only use OAEP padding, not PKCS#1. However, this migration logic exists because some encrypted keys from the past might only be unlocked with PKCS#1. Normally, you'd want to remove this migration code altogether if you can safely assume all users have migrated, or mitigate its risk by restricting who/what can call this code and logging all its use for monitoring. If you must retain migration logic, never allow attacker-controlled input to reach this point, and clearly document the deprecation and intention to remove this code in the near future. If possible, fail closed: don't perform legacy PKCS#1 decryption and show an error.

Best fix without loss of functionality: Remove the PKCS#1 decryption logic (lines 442–455). If a legacy AES key is present, log that migration is no longer supported and generate a new key instead. This disables legacy decryption but maintains safe default behavior (key rotation instead of possibly insecure migration).

Required changes:

In getAESKey (relevant snippet lines 441–459), remove the code block that attempts to decrypt using PKCS#1 and migrate. If OLD_KEY_ALIAS is found, treat it as undecryptable: log this scenario, delete the old key, and proceed to generate a new AES key.

No new imports are needed.

No new methods or variables are required.

utkrishtsahu · 2025-12-17T06:08:42Z

@claude

utkrishtsahu requested a review from a team as a code owner May 19, 2025 05:13

semgrepcode-auth0 bot reviewed May 19, 2025

View reviewed changes

github-advanced-security bot found potential problems May 19, 2025

View reviewed changes

utkrishtsahu force-pushed the SDK-6001 branch from 4f67785 to fb25660 Compare July 7, 2025 04:53

github-advanced-security bot found potential problems Aug 11, 2025

View reviewed changes

auth0/src/main/java/com/auth0/android/authentication/storage/CryptoUtil.java Fixed Show fixed Hide fixed

pmathew92 requested a review from Copilot August 19, 2025 06:54

Copilot AI reviewed Aug 19, 2025

View reviewed changes

pmathew92 reviewed Aug 19, 2025

View reviewed changes

feat: Add PKCS1 to OAEP migration with 4096-bit RSA keys fixed the mi…

d9bce21

…gration logic for existing user

utkrishtsahu force-pushed the SDK-6001 branch from 212b04c to d9bce21 Compare December 17, 2025 05:57

github-advanced-security bot found potential problems Dec 17, 2025

View reviewed changes

UT cases update fix of 4096-bit RSA keys

e641aa1

@@ -54,6 +54,12 @@
                 // Transformations available since API 18
                 // https://developer.android.com/training/articles/keystore.html#SupportedCiphers
                 private static final String RSA_TRANSFORMATION = "RSA/ECB/OAEPWithSHA-256AndMGF1Padding";
+                /**
+                 * !!! WARNING !!!
+                 * "RSA/ECB/PKCS1Padding" is deprecated due to vulnerabilities (see Bleichenbacher attacks, etc),
+                 * and should only be used here for *legacy key migration only*. All new data must use OAEP padding.
+                 * REMOVE SUPPORT FOR THIS AS SOON AS ALL DATA IS MIGRATED.
+                 */
                 private static final String OLD_PKCS1_RSA_TRANSFORMATION = "RSA/ECB/PKCS1Padding";
                 // https://developer.android.com/reference/javax/crypto/Cipher.html
                 @SuppressWarnings("SpellCheckingInspection")
@@ -405,6 +411,7 @@
                                     if (rsaKey != null && keyAliasUsed != null) {
                                         // Decrypt using OLD PKCS1 padding
+                                        // Legacy PKCS1 padding decryption for migration ONLY. See warning on OLD_PKCS1_RSA_TRANSFORMATION.
                                         Cipher rsaPkcs1Cipher = Cipher.getInstance(OLD_PKCS1_RSA_TRANSFORMATION);
                                         rsaPkcs1Cipher.init(Cipher.DECRYPT_MODE, rsaKey.getPrivateKey());
                                         byte[] decryptedAESKey = rsaPkcs1Cipher.doFinal(encryptedAESBytes);
@@ -443,6 +450,7 @@
                         try {
                             byte[] encryptedOldAESBytes = Base64.decode(encodedOldAES, Base64.DEFAULT);
                             KeyStore.PrivateKeyEntry rsaKeyEntry = getRSAKeyEntry();
+                            // Legacy PKCS1 padding decryption for migration ONLY. See warning on OLD_PKCS1_RSA_TRANSFORMATION.
                             Cipher rsaPkcs1Cipher = Cipher.getInstance(OLD_PKCS1_RSA_TRANSFORMATION);
                             rsaPkcs1Cipher.init(Cipher.DECRYPT_MODE, rsaKeyEntry.getPrivateKey());
                             byte[] decryptedAESKey = rsaPkcs1Cipher.doFinal(encryptedOldAESBytes);

@@ -404,7 +404,8 @@
                                     }
                                     if (rsaKey != null && keyAliasUsed != null) {
-                                        // Decrypt using OLD PKCS1 padding
+                                        // WARNING: Using PKCS1 padding here is intentional and ONLY for decrypting legacy data
+                                        // Do NOT use PKCS1 padding for encryption in new code; always use OAEP padding instead.
                                         Cipher rsaPkcs1Cipher = Cipher.getInstance(OLD_PKCS1_RSA_TRANSFORMATION);
                                         rsaPkcs1Cipher.init(Cipher.DECRYPT_MODE, rsaKey.getPrivateKey());
                                         byte[] decryptedAESKey = rsaPkcs1Cipher.doFinal(encryptedAESBytes);

@@ -440,22 +440,9 @@
                     }
                     String encodedOldAES = storage.retrieveString(OLD_KEY_ALIAS);
                     if (!TextUtils.isEmpty(encodedOldAES)) {
-                        try {
-                            byte[] encryptedOldAESBytes = Base64.decode(encodedOldAES, Base64.DEFAULT);
-                            KeyStore.PrivateKeyEntry rsaKeyEntry = getRSAKeyEntry();
-                            Cipher rsaPkcs1Cipher = Cipher.getInstance(OLD_PKCS1_RSA_TRANSFORMATION);
-                            rsaPkcs1Cipher.init(Cipher.DECRYPT_MODE, rsaKeyEntry.getPrivateKey());
-                            byte[] decryptedAESKey = rsaPkcs1Cipher.doFinal(encryptedOldAESBytes);
-                            byte[] encryptedAESWithOAEP = RSAEncrypt(decryptedAESKey);
-                            String newEncodedEncryptedAES = new String(Base64.encode(encryptedAESWithOAEP, Base64.DEFAULT), StandardCharsets.UTF_8);
-                            storage.store(KEY_ALIAS, newEncodedEncryptedAES);
-                            storage.remove(OLD_KEY_ALIAS);
-                            return decryptedAESKey;
-                        } catch (Exception e) {
-                            Log.e(TAG, "Could not migrate the legacy AES key. A new key will be generated.", e);
-                            deleteAESKeys();
-                        }
+                        // Legacy AES key migration via insecure RSA/PKCS1 decryption is no longer supported.
+                        Log.w(TAG, "Found legacy AES key, but PKCS#1 decryption is disabled. The key will be cleared and a new one generated.");
+                        deleteAESKeys();
                     }
                     try {

	private static final int RSA_KEY_SIZE = 4096;
	private static final int RSA_KEY_SIZE = 2048;

	} catch (Exception e) {
	} catch (NoSuchAlgorithmException \| NoSuchPaddingException \| InvalidKeyException \| BadPaddingException \| IllegalBlockSizeException \| KeyStoreException \| UnrecoverableEntryException e) {

-        } catch (Exception e) {
+        } catch (InvalidKeyException
+                | NoSuchPaddingException
+                | IllegalBlockSizeException
+                | BadPaddingException
+                | KeyStoreException
+                | UnrecoverableEntryException
+                | CertificateException
+                | IOException
+                | ProviderException e) {

RSA encryption padding change from PKCS1Padding to OAEPWithSHA-256And… #834

Are you sure you want to change the base?

RSA encryption padding change from PKCS1Padding to OAEPWithSHA-256And… #834

Conversation

utkrishtsahu commented May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

References

Testing

Checklist

Uh oh!

semgrepcode-auth0 bot May 19, 2025

Choose a reason for hiding this comment

Uh oh!

utkrishtsahu Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

semgrepcode-auth0 bot May 19, 2025

Choose a reason for hiding this comment

Uh oh!

utkrishtsahu Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 19, 2025

Choose a reason for hiding this comment

Uh oh!

pmathew92 Aug 19, 2025

Choose a reason for hiding this comment

Uh oh!

utkrishtsahu Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 19, 2025

Choose a reason for hiding this comment

Uh oh!

pmathew92 Aug 19, 2025

Choose a reason for hiding this comment

Uh oh!

pmathew92 Aug 19, 2025

Choose a reason for hiding this comment

Uh oh!

pmathew92 commented Aug 19, 2025

Uh oh!

Check failure

Uh oh!

Uh oh!

Copilot Autofix

Check failure

Uh oh!

Copilot Autofix

Check failure

Uh oh!

Copilot Autofix

utkrishtsahu commented Dec 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

utkrishtsahu commented May 19, 2025 •

edited

Loading

utkrishtsahu Dec 17, 2025 •

edited

Loading