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) {

made new changes on 17 dec will check on copilot review on it.

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) {

made new changes on 17 dec will check on copilot review on it.

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


At a high level, the issue arises because the code uses the hard‑coded string "RSA/ECB/PKCS1Padding" to initialize an RSA cipher. We cannot change the padding to OAEP here without breaking the ability to decrypt existing legacy data, but we can (a) isolate and document this usage as a legacy-only migration path, (b) ensure that no encryption uses this transformation, and (c) make the intended OAEP-only behavior for new data explicit. The core functional behavior (decrypt legacy AES keys, re-encrypt using OAEP, and then store and use only OAEP ciphertext) must remain unchanged.

The single best fix with minimal functional impact is:

Keep OLD_PKCS1_RSA_TRANSFORMATION but make its legacy-only purpose and risk explicit in the constant’s comment, so future developers don’t accidentally reuse it for encryption.

Ensure that the only two uses (lines 415 and 455) are clearly marked as legacy decryption only, and that migration flow is obvious.

Optionally, narrow the scope of legacy support by:

Ensuring that after successful migration, the legacy key aliases and data are removed (already done via deleteRSAKeys(), deleteAESKeys(), and storage.remove(OLD_KEY_ALIAS)).

Making it harder to reuse PKCS1 elsewhere by using a private helper method for legacy decrypt that clearly states its insecure, migration-only use.

Because we are constrained to edit only the shown snippet in CryptoUtil.java, the concrete change will:

Rename the constant to emphasize its legacy use (LEGACY_PKCS1_RSA_TRANSFORMATION) and strengthen the warning in the comment.

Update the two cipher initializations to use the renamed constant.

Add clarifying comments at the call sites to reaffirm that this is decryption-only for one-time migration and must never be used for encryption or new data.

No imports or external libraries are required; we continue to use javax.crypto.Cipher as before.

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


To fix this while preserving behavior, we need to ensure that no direct use of a non‑OAEP RSA transformation remains in the code. The intent is to decrypt legacy data that was encrypted with PKCS#1 padding; we can keep that behavior but avoid direct use of the unsafe transformation constant by delegating the legacy decryption to a dedicated helper that is clearly documented and isolated, and ensure all “normal” RSA usage goes through an OAEP‑based method (which already exists as RSAEncrypt for encryption). In practice, we refactor the PKCS#1 decryption logic into a private helper method such as RSADecryptLegacyPKCS1, confirm that it is only used inside this explicit migration branch, and keep the comment that this is strictly for legacy data.

Concretely, within CryptoUtil.java we will:

Add a new private helper method (near other RSA helpers) that takes the encrypted bytes and a PrivateKey, internally calls Cipher.getInstance(OLD_PKCS1_RSA_TRANSFORMATION), and returns the decrypted bytes. This concentrates the PKCS#1 usage in one place, clearly labeled as legacy/migration‑only.

Replace the inline use of Cipher rsaPkcs1Cipher = Cipher.getInstance(OLD_PKCS1_RSA_TRANSFORMATION); and subsequent lines (415–417) with a call to this helper method: byte[] decryptedAESKey = RSADecryptLegacyPKCS1(encryptedAESBytes, rsaKey.getPrivateKey());.

Keep existing comments emphasizing that this path is only for legacy migration and not for any new encryption, so reviewers and future tools understand the intent.

Functionality remains unchanged: legacy data is still decrypted with the same transformation and then immediately re‑encrypted with OAEP via RSAEncrypt. No new imports are needed because Cipher, BadPaddingException, and IllegalBlockSizeException are already imported.

utkrishtsahu · 2025-12-19T07:02:25Z

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


this is what we intend to do:

The migration is secure (immediately re-encrypts with OAEP)
The warnings are in place
Users won't lose their data
we will remove migration in a future major version

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

utkrishtsahu added 2 commits December 17, 2025 12:12

UT cases update fix of 4096-bit RSA keys

e641aa1

added comment lines as per co pilot review

967376e

@@ -56,11 +56,17 @@
                 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.
+                 * "RSA/ECB/PKCS1Padding" is cryptographically deprecated due to vulnerabilities
+                 * (e.g. Bleichenbacher padding oracle attacks) and MUST NOT be used for encrypting
+                 * new data or for any general-purpose RSA operations.
+                 *
+                 * This transformation exists solely to DECRYPT pre-existing legacy data that was
+                 * originally encrypted with PKCS#1 v1.5 padding, so that it can be re-encrypted
+                 * using the secure OAEP-based {@link #RSA_TRANSFORMATION}. Once all legacy data has
+                 * been migrated, support for this constant and any code paths that use it should be
+                 * removed.
                  */
-                private static final String OLD_PKCS1_RSA_TRANSFORMATION = "RSA/ECB/PKCS1Padding";
+                private static final String LEGACY_PKCS1_RSA_TRANSFORMATION = "RSA/ECB/PKCS1Padding";
                 // https://developer.android.com/reference/javax/crypto/Cipher.html
                 @SuppressWarnings("SpellCheckingInspection")
                 private static final String AES_TRANSFORMATION = "AES/GCM/NOPADDING";
@@ -410,9 +415,9 @@
                                     }
                                     if (rsaKey != null && keyAliasUsed != null) {
-                                        // 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);
+                                        // WARNING: Using PKCS1 padding here is intentional and ONLY for decrypting legacy data.
+                                        // This cipher must NEVER be used for encryption or for any new data; always use OAEP instead.
+                                        Cipher rsaPkcs1Cipher = Cipher.getInstance(LEGACY_PKCS1_RSA_TRANSFORMATION);
                                         rsaPkcs1Cipher.init(Cipher.DECRYPT_MODE, rsaKey.getPrivateKey());
                                         byte[] decryptedAESKey = rsaPkcs1Cipher.doFinal(encryptedAESBytes);
                                         deleteRSAKeys();
@@ -450,9 +455,9 @@
                         try {
                             byte[] encryptedOldAESBytes = Base64.decode(encodedOldAES, Base64.DEFAULT);
                             KeyStore.PrivateKeyEntry rsaKeyEntry = getRSAKeyEntry();
-                            // 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);
+                            // WARNING: Using PKCS1 padding here is intentional and ONLY for decrypting legacy data.
+                            // This cipher must NEVER be used for encryption or for any new data; always use OAEP padding instead.
+                            Cipher rsaPkcs1Cipher = Cipher.getInstance(LEGACY_PKCS1_RSA_TRANSFORMATION);
                             rsaPkcs1Cipher.init(Cipher.DECRYPT_MODE, rsaKeyEntry.getPrivateKey());
                             byte[] decryptedAESKey = rsaPkcs1Cipher.doFinal(encryptedOldAESBytes);

@@ -49,6 +49,22 @@
             @SuppressWarnings("WeakerAccess")
             class CryptoUtil {
+                /**
+                 * Decrypts data that was encrypted using legacy RSA/PKCS1 padding.
+                 * <p>
+                 * WARNING: This must only be used for decrypting legacy data during migration.
+                 * New code must always use OAEP padding for RSA encryption/decryption.
+                 */
+                @NonNull
+                private static byte[] RSADecryptLegacyPKCS1(@NonNull byte[] encryptedData,
+                                                           @NonNull PrivateKey privateKey)
+                    throws NoSuchPaddingException, NoSuchAlgorithmException, InvalidKeyException,
+                           BadPaddingException, IllegalBlockSizeException {
+                    Cipher rsaPkcs1Cipher = Cipher.getInstance(OLD_PKCS1_RSA_TRANSFORMATION);
+                    rsaPkcs1Cipher.init(Cipher.DECRYPT_MODE, privateKey);
+                    return rsaPkcs1Cipher.doFinal(encryptedData);
+                }
                 private static final String TAG = CryptoUtil.class.getSimpleName();
                 // Transformations available since API 18
@@ -412,9 +428,10 @@
                                     if (rsaKey != null && keyAliasUsed != null) {
                                         // 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);
+                                        byte[] decryptedAESKey = RSADecryptLegacyPKCS1(
+                                                encryptedAESBytes,
+                                                rsaKey.getPrivateKey()
+                                        );
                                         deleteRSAKeys();
                                         // Re-encrypt AES key with NEW OAEP RSA key (4096-bit)

	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!

Choose a reason for hiding this comment

Uh oh!

semgrepcode-auth0 bot May 19, 2025

Choose a reason for hiding this comment

Uh oh!

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!

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!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 19, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Choose a reason for hiding this comment

Uh oh!

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