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.

utkrishtsahu · 2025-06-16T01:47:36Z

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


To fix the problem, update the migration code to use OAEP padding for RSA decryption, if possible. This means replacing the use of "RSA/ECB/PKCS1Padding" with "RSA/ECB/OAEPWithSHA-256AndMGF1Padding" (or another OAEP variant compatible with the legacy data). However, if the legacy data was encrypted with PKCS#1 padding, it cannot be decrypted with OAEP; in that case, the only safe option is to ensure the migration code is not exposed to untrusted input and is removed after migration. If you must keep the migration code, add comments to clarify its purpose and restrict its use. If you can re-encrypt legacy data with OAEP, do so and remove PKCS#1 padding support.

Best fix:

Restrict the use of PKCS#1 padding to migration only, and add a clear comment explaining its legacy purpose.

Ensure the migration code is not exposed to untrusted input.

If possible, remove the migration code after all legacy data has been migrated.

If you must keep the migration code, consider logging a warning and/or restricting its invocation.

Required changes:

Add a comment above the definition and use of OLD_PKCS1_RSA_TRANSFORMATION explaining its legacy/migration-only purpose.

Optionally, add a runtime check or log to ensure the migration code is not used for new data.

No new imports or methods are required.

that was the PR which we are fixing to use OAEP padding.

this usage is intentional and strictly limited to a one-time data migration path for existing users.

Lets keep a backlog ticket to remove this once all users are migrated to the 3.10.0 or later

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

…MGF1Padding

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 ANDROID_KEY_STORE = "AndroidKeyStore";
    private static final String ALGORITHM_RSA = "RSA";
    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 ?

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

            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

@@ -124,7 +125,8 @@ KeyStore.PrivateKeyEntry getRSAKeyEntry() throws CryptoException, IncompatibleDe
                        .setCertificateNotBefore(start.getTime())
                        .setCertificateNotAfter(end.getTime())
                        .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

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 added 2 commits July 7, 2025 10:19

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

97d5f2a

…MGF1Padding

added UT test cases

fb25660

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

utkrishtsahu added 2 commits July 29, 2025 12:48

Changing RSA_KEY_SIZE to 4096 as requested by security team

967e166

Changing the migration logic as follows:

212b04c

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

View reviewed changes

auth0/src/main/java/com/auth0/android/authentication/storage/CryptoUtil.java Dismissed Show dismissed Hide dismissed

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

@@ -57,6 +57,8 @@
                 // https://developer.android.com/reference/javax/crypto/Cipher.html
                 @SuppressWarnings("SpellCheckingInspection")
                 private static final String AES_TRANSFORMATION = "AES/GCM/NOPADDING";
+                // Legacy: Used only for migration of old AES keys encrypted with PKCS#1 padding.
+                // Do NOT use for new encryption operations. Remove after migration is complete.
                 private static final String OLD_PKCS1_RSA_TRANSFORMATION = "RSA/ECB/PKCS1Padding";
                 private static final String ANDROID_KEY_STORE = "AndroidKeyStore";
@@ -375,6 +377,8 @@
                         try {
                             byte[] encryptedOldAESBytes = Base64.decode(encodedOldAES, Base64.DEFAULT);
                             KeyStore.PrivateKeyEntry rsaKeyEntry = getRSAKeyEntry();
+                            // Legacy migration: Decrypt old AES key encrypted with PKCS#1 padding.
+                            // This block should only be used for migration and not for new data.
                             Cipher rsaPkcs1Cipher = Cipher.getInstance(OLD_PKCS1_RSA_TRANSFORMATION);
                             rsaPkcs1Cipher.init(Cipher.DECRYPT_MODE, rsaKeyEntry.getPrivateKey());
                             byte[] decryptedAESKey = rsaPkcs1Cipher.doFinal(encryptedOldAESBytes);

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

		throw new IncompatibleDeviceException(e);
		} catch (Exception 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

Uh oh!

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!

Check failure

Uh oh!

Copilot Autofix

utkrishtsahu Jun 16, 2025

Choose a reason for hiding this comment

Uh oh!

utkrishtsahu Jul 7, 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!

pmathew92 Aug 19, 2025

Choose a reason for hiding this comment

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!

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!

Uh oh!

utkrishtsahu commented May 19, 2025 •

edited

Loading

utkrishtsahu Jul 7, 2025 •

edited

Loading