Add Capability to retrieve Custom KeyStores #4025

Binara-Sachin · 2024-07-04T22:41:56Z

Purpose

This PR enables retrieving custom keystores from the KeyStoreManager class.
Custom KeyStores can be configured in the Carbon.xml file which can then be retrieved using KeyStoreManager

Problem Statement: wso2-enterprise/iam-product-management#67
Git Issue: wso2/product-is#20564

CLAassistant · 2024-07-04T22:42:02Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

pamodaaw · 2024-07-15T08:00:07Z

core/org.wso2.carbon.core/src/main/java/org/wso2/carbon/core/RegistryResources.java

@@ -160,6 +160,20 @@ public static final class SecurityManagement {
        public static final String SERVER_INTERNAL_PRIVATE_KEY_PASSWORD = "Security.InternalKeyStore.KeyPassword";
        public static final String SERVER_INTERNAL_KEYSTORE_TYPE = "Security.InternalKeyStore.Type";

+        //Custom KeyStore related constants


Suggested change

//Custom KeyStore related constants

// Custom KeyStore related constants.

pamodaaw · 2024-07-15T08:01:35Z

core/org.wso2.carbon.core/src/main/java/org/wso2/carbon/core/util/KeyStoreManager.java

@@ -124,39 +133,21 @@ public static KeyStoreManager getInstance(int tenantId,
     * @return KeyStore object
     * @throws Exception If there is not a key store with the given name
     */
-    public KeyStore getKeyStore(String keyStoreName) throws Exception {
+    public KeyStore getKeyStore(String keyStoreName) {


Don't change the public method definitions

pamodaaw · 2024-07-15T08:06:19Z

core/org.wso2.carbon.core/src/main/java/org/wso2/carbon/core/util/KeyStoreManager.java

+
+            return getTenantKeyStore(keyStoreName);
+        } catch (Exception e) {
+            log.error("Error loading the key store : " + keyStoreName);


No need to log the error here, since we should be printing the error message from the place where the exception is handled

pamodaaw · 2024-07-15T08:07:58Z

core/org.wso2.carbon.core/src/main/java/org/wso2/carbon/core/util/KeyStoreUtil.java

@@ -111,4 +117,53 @@ public static Certificate getCertificate(String alias, KeyStore store) throws Ax
        }
    }

+    public static boolean isCustomKeyStore(String keyStoreName) {


Add javadoc comment for the method

pamodaaw · 2024-07-15T08:10:29Z

core/org.wso2.carbon.core/src/main/java/org/wso2/carbon/core/util/KeyStoreUtil.java

+        return false;
+    }
+
+    public static QName getQNameWithCarbonNS(String localPart) {


Javadoc comments are required for all the public methods. Specially if they are a util function.

pamodaaw · 2024-07-15T09:06:11Z

core/org.wso2.carbon.core/src/main/java/org/wso2/carbon/core/util/KeyStoreManager.java

+
+

unnecessary new lines

pamodaaw · 2024-07-15T09:07:34Z

core/org.wso2.carbon.core/src/main/java/org/wso2/carbon/core/util/KeyStoreManager.java

+     */
+    public PrivateKey getCustomKeyStorePrivateKey(String keyStoreName) throws Exception {
+
+        OMElement config = KeyStoreUtil.getCustomKeyStoreConfig(keyStoreName, this.getServerConfigService());


Let's add a validation to the keystore name to verify it's a customkeystore

pamodaaw · 2024-07-15T09:08:52Z

core/org.wso2.carbon.core/src/main/java/org/wso2/carbon/core/util/KeyStoreManager.java

+    /**
+     * This method is used to get public certificates of custom keystores
+     *
+     * @return Public certificate of a given custom keystore
+     * @throws Exception Permission denied for accessing primary key store
+     */


Suggested change

/**

* This method is used to get public certificates of custom keystores

*

* @return Public certificate of a given custom keystore

* @throws Exception Permission denied for accessing primary key store

*/

/**

* This method is used to get public certificates of custom keystores.

*

* @return Public certificate of a given custom keystore.

* @throws Exception Permission denied for accessing the primary key store.

*/

Please check other similar places

pamodaaw · 2024-07-15T09:10:14Z

core/org.wso2.carbon.core/src/main/java/org/wso2/carbon/core/util/KeyStoreManager.java

+     * @return custom key store object
+     * @throws Exception Carbon Exception for tenants other than tenant 0
+     */
+    public KeyStore getCustomKeyStore(String keyStoreName) throws Exception {


Do we need to have this method public? It would be better to add the validation for the keystore name to ensure that it is a custom keystore. Otherwise it will cause unexpected behavior

pamodaaw · 2024-07-15T09:14:23Z

core/org.wso2.carbon.core/src/main/java/org/wso2/carbon/core/RegistryResources.java

@@ -160,6 +160,20 @@ public static final class SecurityManagement {
        public static final String SERVER_INTERNAL_PRIVATE_KEY_PASSWORD = "Security.InternalKeyStore.KeyPassword";
        public static final String SERVER_INTERNAL_KEYSTORE_TYPE = "Security.InternalKeyStore.Type";

+        //Custom KeyStore related constants
+        public static final class CustomKeyStore {
+            public static final String KEYSTORE_PREFIX = "CustomKeyStore/";


Shall we have the prefix as CUSTOM/ only? so that the keystore name would be appended.

pamodaaw · 2024-07-15T09:20:04Z

core/org.wso2.carbon.core/src/main/java/org/wso2/carbon/core/util/KeyStoreManager.java

+     * @param alias        alias of the private key
+     * @return private key corresponding to the alias
+     */
+    public PrivateKey getTenantPrivateKey(String keyStoreName, String alias) throws Exception{


Do we expect to call this method outside this class?

ImalshaG · 2024-07-15T14:32:48Z

core/org.wso2.carbon.core/src/main/java/org/wso2/carbon/core/util/KeyStoreManager.java

+     * This method loads the private key of a given tenant key store
+     *
+     * @param keyStoreName name of the tenant key store
+     * @param alias        alias of the private key
+     * @return private key corresponding to the alias


Suggested change

* This method loads the private key of a given tenant key store

*

* @param keyStoreName name of the tenant key store

* @param alias alias of the private key

* @return private key corresponding to the alias

* This method loads the private key of a given tenant key store.

*

* @param keyStoreName Name of the tenant key store.

* @param alias Alias of the private key.

* @return Private key corresponding to the alias.

Add capability to retrieve custom keystores

9288599

Binara-Sachin added 5 commits July 5, 2024 04:12

Fix getTenantKeyStore method argument

44a531c

Change custom keystore implementation

f2f8eb1

Add custom keystore template to carbon.xml.j2 file

712bc15

Add get certificate methods

4ece3ba

Change access modifiers

616e1d0

Binara-Sachin mentioned this pull request Jul 15, 2024

Add IdentityKeyStoreResolver and Util Classes wso2/carbon-identity-framework#5794

Draft

pamodaaw reviewed Jul 15, 2024

View reviewed changes

core/org.wso2.carbon.core/src/main/java/org/wso2/carbon/core/util/KeyStoreManager.java

Comment on lines +477 to +478

Copy link

pamodaaw Jul 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unnecessary new lines

pamodaaw reviewed Jul 15, 2024

View reviewed changes

ImalshaG reviewed Jul 15, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Capability to retrieve Custom KeyStores #4025

Add Capability to retrieve Custom KeyStores #4025

Binara-Sachin commented Jul 4, 2024 •

edited

Loading

CLAassistant commented Jul 4, 2024

pamodaaw Jul 15, 2024

pamodaaw Jul 15, 2024

pamodaaw Jul 15, 2024

pamodaaw Jul 15, 2024

pamodaaw Jul 15, 2024

pamodaaw Jul 15, 2024

pamodaaw Jul 15, 2024

pamodaaw Jul 15, 2024

pamodaaw Jul 15, 2024 •

edited

Loading

pamodaaw Jul 15, 2024

pamodaaw Jul 15, 2024

ImalshaG Jul 15, 2024

	//Custom KeyStore related constants
	// Custom KeyStore related constants.

Add Capability to retrieve Custom KeyStores #4025

Are you sure you want to change the base?

Add Capability to retrieve Custom KeyStores #4025

Conversation

Binara-Sachin commented Jul 4, 2024 • edited Loading

Purpose

CLAassistant commented Jul 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pamodaaw Jul 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Binara-Sachin commented Jul 4, 2024 •

edited

Loading

pamodaaw Jul 15, 2024 •

edited

Loading