Add FIPS support #4461
Add FIPS support #4461
Conversation
![wso2-engineering[bot]](https://avatars.githubusercontent.com/in/1856828?s=60&v=4){kind=small}
| private static String getAlgorithm() { | ||
| if (StringUtils.isNotEmpty(System.getProperty(SECURITY_JCE_PROVIDER))) { | ||
| return "SHA-256"; | ||
| } else { | ||
| return "SHA-1"; | ||
| } |
There was a problem hiding this comment.
Log Improvement Suggestion No: 2
| private static String getAlgorithm() { | |
| if (StringUtils.isNotEmpty(System.getProperty(SECURITY_JCE_PROVIDER))) { | |
| return "SHA-256"; | |
| } else { | |
| return "SHA-1"; | |
| } | |
| private static String getAlgorithm() { | |
| if (StringUtils.isNotEmpty(System.getProperty(SECURITY_JCE_PROVIDER))) { | |
| log.debug("Using SHA-256 algorithm for JCE provider: " + System.getProperty(SECURITY_JCE_PROVIDER)); | |
| return "SHA-256"; | |
| } else { | |
| log.debug("Using default SHA-1 algorithm"); | |
| return "SHA-1"; | |
| } |
| private static String getKeyManagerType() { | ||
| String provider = System.getProperty(JCE_PROVIDER); | ||
| if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { | ||
| return PKIX; | ||
| } else { | ||
| return KeyManagerFactory.getDefaultAlgorithm(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Log Improvement Suggestion No: 4
| private static String getKeyManagerType() { | |
| String provider = System.getProperty(JCE_PROVIDER); | |
| if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { | |
| return PKIX; | |
| } else { | |
| return KeyManagerFactory.getDefaultAlgorithm(); | |
| } | |
| } | |
| private static String getKeyManagerType() { | |
| String provider = System.getProperty(JCE_PROVIDER); | |
| if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { | |
| log.debug("JCE provider detected, using PKIX key manager type"); | |
| return PKIX; | |
| } else { | |
| String defaultAlgorithm = KeyManagerFactory.getDefaultAlgorithm(); | |
| log.debug("Using default key manager type: " + defaultAlgorithm); | |
| return defaultAlgorithm; | |
| } |
| private static String getKeyManagerType() { | ||
| String provider = System.getProperty(JCE_PROVIDER); | ||
| if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { | ||
| return PKIX; | ||
| } else { | ||
| return KeyManagerFactory.getDefaultAlgorithm(); | ||
| } |
There was a problem hiding this comment.
Log Improvement Suggestion No: 5
| private static String getKeyManagerType() { | |
| String provider = System.getProperty(JCE_PROVIDER); | |
| if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { | |
| return PKIX; | |
| } else { | |
| return KeyManagerFactory.getDefaultAlgorithm(); | |
| } | |
| private static String getKeyManagerType() { | |
| String provider = System.getProperty(JCE_PROVIDER); | |
| if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { | |
| log.info("Using PKIX KeyManager type for JCE provider: " + provider); | |
| return PKIX; | |
| } else { | |
| log.debug("Using default KeyManager algorithm: " + KeyManagerFactory.getDefaultAlgorithm()); | |
| return KeyManagerFactory.getDefaultAlgorithm(); | |
| } |
| public InboundWebsocketSSLConfiguration buildSSLConfiguration(InboundProcessorParams params) { | ||
| keyStoreType = params.getProperties().getProperty(InboundWebsocketConstants.INBOUND_SSL_KEY_STORE_TYPE); | ||
| trustStoreType = params.getProperties().getProperty(InboundWebsocketConstants.INBOUND_SSL_TRUST_STORE_TYPE); |
There was a problem hiding this comment.
Log Improvement Suggestion No: 7
| public InboundWebsocketSSLConfiguration buildSSLConfiguration(InboundProcessorParams params) { | |
| keyStoreType = params.getProperties().getProperty(InboundWebsocketConstants.INBOUND_SSL_KEY_STORE_TYPE); | |
| trustStoreType = params.getProperties().getProperty(InboundWebsocketConstants.INBOUND_SSL_TRUST_STORE_TYPE); | |
| public InboundWebsocketSSLConfiguration buildSSLConfiguration(InboundProcessorParams params) { | |
| keyStoreType = params.getProperties().getProperty(InboundWebsocketConstants.INBOUND_SSL_KEY_STORE_TYPE); | |
| trustStoreType = params.getProperties().getProperty(InboundWebsocketConstants.INBOUND_SSL_TRUST_STORE_TYPE); | |
| log.info("Building SSL configuration with keyStoreType: " + keyStoreType + ", trustStoreType: " + trustStoreType); |
| File keyStore = new File(keyStoreFile); | ||
| if (!keyStore.exists()) { | ||
| throw new IllegalArgumentException("KeyStore File " + keyStoreFile + " not found"); |
There was a problem hiding this comment.
Log Improvement Suggestion No: 9
| File keyStore = new File(keyStoreFile); | |
| if (!keyStore.exists()) { | |
| throw new IllegalArgumentException("KeyStore File " + keyStoreFile + " not found"); | |
| File keyStore = new File(keyStoreFile); | |
| if (!keyStore.exists()) { | |
| log.error("KeyStore file not found at path: " + keyStoreFile); | |
| throw new IllegalArgumentException("KeyStore File " + keyStoreFile + " not found"); |
| private final String[] sslProtocols; | ||
| private static final String PKIX = "PKIX"; | ||
| private static final String JCE_PROVIDER = "security.jce.provider"; | ||
|
|
There was a problem hiding this comment.
Log Improvement Suggestion No: 10
| public SSLHandlerFactory(InboundWebsocketSSLConfiguration sslConfiguration) { | |
| log.info("Initializing SSLHandlerFactory with keystore: " + sslConfiguration.getKeyStore().getName()); |
| if(StringUtils.isNotEmpty(System.getProperty(JCE_PROVIDER))) { | ||
| digest = getDigest(request, SHA_256_DIGEST_ALGORITHM); |
There was a problem hiding this comment.
Log Improvement Suggestion No: 12
| if(StringUtils.isNotEmpty(System.getProperty(JCE_PROVIDER))) { | |
| digest = getDigest(request, SHA_256_DIGEST_ALGORITHM); | |
| if(StringUtils.isNotEmpty(System.getProperty(JCE_PROVIDER))) { | |
| log.debug("Using SHA-256 digest algorithm as JCE provider is configured"); | |
| digest = getDigest(request, SHA_256_DIGEST_ALGORITHM); |
| } else { | ||
| digest = getDigest(request, MD5_DIGEST_ALGORITHM); |
There was a problem hiding this comment.
Log Improvement Suggestion No: 13
| } else { | |
| digest = getDigest(request, MD5_DIGEST_ALGORITHM); | |
| } else { | |
| log.debug("Using MD5 digest algorithm as JCE provider is not configured"); | |
| digest = getDigest(request, MD5_DIGEST_ALGORITHM); |
| private static String getAlgorithm() { | ||
| String provider = System.getProperty(JCE_PROVIDER); | ||
| if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { | ||
| return SHA_256_DIGEST_ALGORITHM; | ||
| } else { | ||
| return MD5_DIGEST_ALGORITHM; | ||
| } |
There was a problem hiding this comment.
Log Improvement Suggestion No: 14
| private static String getAlgorithm() { | |
| String provider = System.getProperty(JCE_PROVIDER); | |
| if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { | |
| return SHA_256_DIGEST_ALGORITHM; | |
| } else { | |
| return MD5_DIGEST_ALGORITHM; | |
| } | |
| private static String getAlgorithm() { | |
| String provider = System.getProperty(JCE_PROVIDER); | |
| if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { | |
| log.debug("JCE provider detected. Using SHA-256 algorithm for digest generation."); | |
| return SHA_256_DIGEST_ALGORITHM; | |
| } else { | |
| log.debug("No JCE provider configured. Using MD5 algorithm for digest generation."); | |
| return MD5_DIGEST_ALGORITHM; | |
| } |
| private static String getAlgorithm() { | ||
| String provider = System.getProperty(JCE_PROVIDER); | ||
| if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { | ||
| return SHA_256_DIGEST_ALGORITHM; | ||
| } else { | ||
| return MD5_DIGEST_ALGORITHM; | ||
| } | ||
| } |
There was a problem hiding this comment.
Log Improvement Suggestion No: 15
| private static String getAlgorithm() { | |
| String provider = System.getProperty(JCE_PROVIDER); | |
| if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { | |
| return SHA_256_DIGEST_ALGORITHM; | |
| } else { | |
| return MD5_DIGEST_ALGORITHM; | |
| } | |
| } | |
| private static String getAlgorithm() { | |
| String provider = System.getProperty(JCE_PROVIDER); | |
| if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { | |
| if (log.isDebugEnabled()) { | |
| log.debug("JCE provider is configured. Using SHA-256 digest algorithm."); | |
| } | |
| return SHA_256_DIGEST_ALGORITHM; | |
| } else { | |
| if (log.isDebugEnabled()) { | |
| log.debug("JCE provider is not configured. Using MD5 digest algorithm."); | |
| } | |
| return MD5_DIGEST_ALGORITHM; | |
| } |
| if (toAddress != null) { | ||
| digest = getDigest(body, toAddress, headers, MD5_DIGEST_ALGORITHM); | ||
| digest = getDigest(body, toAddress, headers, getAlgorithm()); | ||
| } else { | ||
| digest = getDigest(body, MD5_DIGEST_ALGORITHM); | ||
| digest = getDigest(body, getAlgorithm()); | ||
| } |
There was a problem hiding this comment.
Log Improvement Suggestion No: 16
| if (toAddress != null) { | |
| digest = getDigest(body, toAddress, headers, MD5_DIGEST_ALGORITHM); | |
| digest = getDigest(body, toAddress, headers, getAlgorithm()); | |
| } else { | |
| digest = getDigest(body, MD5_DIGEST_ALGORITHM); | |
| digest = getDigest(body, getAlgorithm()); | |
| } | |
| if (toAddress != null) { | |
| if (log.isDebugEnabled()) { | |
| log.debug("Generating digest with toAddress for cache key generation."); | |
| } | |
| digest = getDigest(body, toAddress, headers, getAlgorithm()); | |
| } else { | |
| if (log.isDebugEnabled()) { | |
| log.debug("Generating digest without toAddress for cache key generation."); | |
| } | |
| digest = getDigest(body, getAlgorithm()); | |
| } |
| new QName(WebsocketConstants.TRUST_STORE_TYPE)); | ||
| if (trustStoreTypeElem != null) { | ||
| trustStoreType = trustStoreTypeElem.getText(); | ||
| } else { | ||
| trustStoreType = "JKS"; | ||
| } |
There was a problem hiding this comment.
Log Improvement Suggestion No: 17
| new QName(WebsocketConstants.TRUST_STORE_TYPE)); | |
| if (trustStoreTypeElem != null) { | |
| trustStoreType = trustStoreTypeElem.getText(); | |
| } else { | |
| trustStoreType = "JKS"; | |
| } | |
| if (trustStoreTypeElem != null) { | |
| trustStoreType = trustStoreTypeElem.getText(); | |
| log.info("Trust store type set to: " + trustStoreType); | |
| } else { | |
| trustStoreType = "JKS"; | |
| log.debug("Trust store type not specified, defaulting to JKS"); | |
| } |
| final String storePassword = trustStorePasswordElem.getText(); | ||
| sslCtx = SslContextBuilder.forClient() | ||
| .trustManager(SSLUtil.createTrustmanager(location, storePassword)).build(); | ||
| if (trustStoreTypeElem != null) { | ||
| trustStoreType = trustStoreTypeElem.getText(); | ||
| } else { | ||
| trustStoreType = "JKS"; | ||
| } |
There was a problem hiding this comment.
Log Improvement Suggestion No: 18
| final String storePassword = trustStorePasswordElem.getText(); | |
| sslCtx = SslContextBuilder.forClient() | |
| .trustManager(SSLUtil.createTrustmanager(location, storePassword)).build(); | |
| if (trustStoreTypeElem != null) { | |
| trustStoreType = trustStoreTypeElem.getText(); | |
| } else { | |
| trustStoreType = "JKS"; | |
| } | |
| if (trustStoreTypeElem != null) { | |
| trustStoreType = trustStoreTypeElem.getText(); | |
| log.debug("Using trust store type: " + trustStoreType); | |
| } else { | |
| trustStoreType = "JKS"; | |
| log.debug("Trust store type not specified for connection, using default: JKS"); | |
| } |
| public static SSLContext createServerSSLContext(final String keyStoreLocation, final String keyStorePwd) { | ||
| try { | ||
| if (serverSSLCtx == null) { | ||
| KeyStore keyStore = KeyStore.getInstance(KEY_STORE_TYPE); | ||
| KeyStore keyStore = KeyStore.getInstance(WebsocketConnectionFactory.getTrustStoreType()); | ||
| keyStore.load(new FileInputStream(keyStoreLocation), keyStorePwd.toCharArray()); | ||
| KeyManagerFactory keyManagerFactory = KeyManagerFactory.getInstance(KEY_MANAGER_TYPE); | ||
| KeyManagerFactory keyManagerFactory = KeyManagerFactory.getInstance(getKeyManagerType()); |
There was a problem hiding this comment.
Log Improvement Suggestion No: 19
| public static SSLContext createServerSSLContext(final String keyStoreLocation, final String keyStorePwd) { | |
| try { | |
| if (serverSSLCtx == null) { | |
| KeyStore keyStore = KeyStore.getInstance(KEY_STORE_TYPE); | |
| KeyStore keyStore = KeyStore.getInstance(WebsocketConnectionFactory.getTrustStoreType()); | |
| keyStore.load(new FileInputStream(keyStoreLocation), keyStorePwd.toCharArray()); | |
| KeyManagerFactory keyManagerFactory = KeyManagerFactory.getInstance(KEY_MANAGER_TYPE); | |
| KeyManagerFactory keyManagerFactory = KeyManagerFactory.getInstance(getKeyManagerType()); | |
| public static SSLContext createServerSSLContext(final String keyStoreLocation, final String keyStorePwd) { | |
| try { | |
| if (serverSSLCtx == null) { | |
| log.info("Creating server SSL context with keystore: " + keyStoreLocation); | |
| KeyStore keyStore = KeyStore.getInstance(WebsocketConnectionFactory.getTrustStoreType()); | |
| keyStore.load(new FileInputStream(keyStoreLocation), keyStorePwd.toCharArray()); |
| private static String getAlgorithm() { | ||
| String provider = System.getProperty(JCE_PROVIDER); | ||
| if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { | ||
| return "SHA-256"; | ||
| } else { | ||
| return "SHA-1"; | ||
| } | ||
| return Constants.BOUNCY_CASTLE_PROVIDER; | ||
| } |
There was a problem hiding this comment.
Log Improvement Suggestion No: 21
| private static String getAlgorithm() { | |
| String provider = System.getProperty(JCE_PROVIDER); | |
| if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { | |
| return "SHA-256"; | |
| } else { | |
| return "SHA-1"; | |
| } | |
| return Constants.BOUNCY_CASTLE_PROVIDER; | |
| } | |
| private static String getAlgorithm() { | |
| String provider = System.getProperty(JCE_PROVIDER); | |
| if (log.isDebugEnabled()) { | |
| log.debug("JCE provider property value: " + (provider != null ? "[REDACTED]" : "null")); | |
| } | |
| if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { | |
| return "SHA-256"; | |
| } else { | |
| return "SHA-1"; | |
| } |
| CipherHolder cipherHolder = new CipherHolder(); | ||
| cipherHolder.setCipherText(Base64.encode(originalCipher)); | ||
| cipherHolder.setTransformation(transformation); | ||
| cipherHolder.setThumbPrint(calculateThumbprint(certificate, "SHA-1"), "SHA-1"); | ||
| String algorithm = getAlgorithm(); | ||
| cipherHolder.setThumbPrint(calculateThumbprint(certificate, algorithm), algorithm); |
There was a problem hiding this comment.
Log Improvement Suggestion No: 22
| CipherHolder cipherHolder = new CipherHolder(); | |
| cipherHolder.setCipherText(Base64.encode(originalCipher)); | |
| cipherHolder.setTransformation(transformation); | |
| cipherHolder.setThumbPrint(calculateThumbprint(certificate, "SHA-1"), "SHA-1"); | |
| String algorithm = getAlgorithm(); | |
| cipherHolder.setThumbPrint(calculateThumbprint(certificate, algorithm), algorithm); | |
| CipherHolder cipherHolder = new CipherHolder(); | |
| cipherHolder.setCipherText(Base64.encode(originalCipher)); | |
| cipherHolder.setTransformation(transformation); | |
| String algorithm = getAlgorithm(); | |
| if (log.isDebugEnabled()) { | |
| log.debug("Using algorithm for thumbprint calculation: " + algorithm); | |
| } | |
| cipherHolder.setThumbPrint(calculateThumbprint(certificate, algorithm), algorithm); |
| private String generateSaltValue() { | ||
| String saltValue = null; | ||
| try { | ||
| SecureRandom secureRandom = SecureRandom.getInstance(SHA_1_PRNG); | ||
| SecureRandom secureRandom = SecureRandom.getInstance(getAlgorithm()); |
There was a problem hiding this comment.
Log Improvement Suggestion No: 23
| private String generateSaltValue() { | |
| String saltValue = null; | |
| try { | |
| SecureRandom secureRandom = SecureRandom.getInstance(SHA_1_PRNG); | |
| SecureRandom secureRandom = SecureRandom.getInstance(getAlgorithm()); | |
| private String generateSaltValue() { | |
| String saltValue = null; | |
| try { | |
| log.debug("Generating salt value for password hashing"); | |
| SecureRandom secureRandom = SecureRandom.getInstance(getAlgorithm()); |
| private static String getAlgorithm() { | ||
| if (org.apache.commons.lang.StringUtils.isNotEmpty(System.getProperty(JCE_PROVIDER))) { | ||
| return DRBG; | ||
| } else { | ||
| return SHA_1_PRNG; | ||
| } |
There was a problem hiding this comment.
Log Improvement Suggestion No: 24
| private static String getAlgorithm() { | |
| if (org.apache.commons.lang.StringUtils.isNotEmpty(System.getProperty(JCE_PROVIDER))) { | |
| return DRBG; | |
| } else { | |
| return SHA_1_PRNG; | |
| } | |
| private static String getAlgorithm() { | |
| if (org.apache.commons.lang.StringUtils.isNotEmpty(System.getProperty(JCE_PROVIDER))) { | |
| if (log.isDebugEnabled()) { | |
| log.debug("JCE provider property is set, using DRBG algorithm for SecureRandom"); | |
| } | |
| return DRBG; | |
| } else { | |
| if (log.isDebugEnabled()) { | |
| log.debug("JCE provider property not set, using SHA1PRNG algorithm for SecureRandom"); | |
| } | |
| return SHA_1_PRNG; |
| String saltValue = null; | ||
| try { | ||
| SecureRandom secureRandom = SecureRandom.getInstance(SHA_1_PRNG); | ||
| SecureRandom secureRandom = SecureRandom.getInstance(getAlgorithm()); |
There was a problem hiding this comment.
Log Improvement Suggestion No: 25
| SecureRandom secureRandom = SecureRandom.getInstance(getAlgorithm()); | |
| SecureRandom secureRandom = SecureRandom.getInstance(getAlgorithm()); | |
| if (log.isDebugEnabled()) { | |
| log.debug("Using SecureRandom algorithm: " + getAlgorithm() + " for salt generation"); | |
| } |
| private static String getAlgorithm() { | ||
| String provider = System.getProperty(JCE_PROVIDER); | ||
| if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { | ||
| return DRBG; | ||
| } else { | ||
| return SHA_1_PRNG; | ||
| } | ||
| } |
There was a problem hiding this comment.
Log Improvement Suggestion No: 26
| private static String getAlgorithm() { | |
| String provider = System.getProperty(JCE_PROVIDER); | |
| if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { | |
| return DRBG; | |
| } else { | |
| return SHA_1_PRNG; | |
| } | |
| } | |
| private static String getAlgorithm() { | |
| String provider = System.getProperty(JCE_PROVIDER); | |
| if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { | |
| if (log.isDebugEnabled()) { | |
| log.debug("JCE provider property detected: " + provider + ", using DRBG algorithm"); | |
| } | |
| return DRBG; | |
| } else { | |
| return SHA_1_PRNG; | |
| } | |
| } |
|
|
||
| private static String getAlgorithm() { | ||
| String provider = System.getProperty(JCE_PROVIDER); | ||
| if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { | ||
| return "DRBG"; | ||
| } else { | ||
| return "SHA1PRNG"; | ||
| } | ||
| } |
There was a problem hiding this comment.
Log Improvement Suggestion No: 27
| private static String getAlgorithm() { | |
| String provider = System.getProperty(JCE_PROVIDER); | |
| if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { | |
| return "DRBG"; | |
| } else { | |
| return "SHA1PRNG"; | |
| } | |
| } | |
| private static String getAlgorithm() { | |
| String provider = System.getProperty(JCE_PROVIDER); | |
| if (log.isDebugEnabled()) { | |
| log.debug("JCE provider property value: " + (provider != null ? "[REDACTED]" : "null")); | |
| } | |
| if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { | |
| log.info("Using DRBG algorithm for secure random generation"); | |
| return "DRBG"; | |
| } else { | |
| log.info("Using SHA1PRNG algorithm for secure random generation"); | |
| return "SHA1PRNG"; | |
| } | |
| } |
| try { | ||
| // the secure random | ||
| SecureRandom prng = SecureRandom.getInstance("SHA1PRNG"); | ||
| SecureRandom prng = SecureRandom.getInstance(getAlgorithm()); |
There was a problem hiding this comment.
Log Improvement Suggestion No: 28
| SecureRandom prng = SecureRandom.getInstance(getAlgorithm()); | |
| SecureRandom prng = SecureRandom.getInstance(getAlgorithm()); | |
| if (log.isDebugEnabled()) { | |
| log.debug("Generating random password for user: " + username); | |
| } |
| try { | ||
| String cacertsPasswd = properties.getProperty(PROP_ID_CACERT_PASS, "changeit"); | ||
| cacerts = KeyStore.getInstance(KeyStore.getDefaultType()); | ||
| cacerts = KeyStore.getInstance(getKeyType(properties)); | ||
| cacerts.load(cacertsIs, cacertsPasswd.toCharArray()); |
There was a problem hiding this comment.
Log Improvement Suggestion No: 29
| try { | |
| String cacertsPasswd = properties.getProperty(PROP_ID_CACERT_PASS, "changeit"); | |
| cacerts = KeyStore.getInstance(KeyStore.getDefaultType()); | |
| cacerts = KeyStore.getInstance(getKeyType(properties)); | |
| cacerts.load(cacertsIs, cacertsPasswd.toCharArray()); | |
| try { | |
| String cacertsPasswd = properties.getProperty(PROP_ID_CACERT_PASS, "changeit"); | |
| cacerts = KeyStore.getInstance(getKeyType(properties)); | |
| log.debug("Loading CA certificates from keystore"); | |
| cacerts.load(cacertsIs, cacertsPasswd.toCharArray()); |
| private static String getAlgorithm() { | ||
| String provider = System.getProperty(JCE_PROVIDER); | ||
| if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { | ||
| return "SHA-256"; | ||
| } else { | ||
| return "SHA-1"; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Log Improvement Suggestion No: 30
| private static String getAlgorithm() { | |
| String provider = System.getProperty(JCE_PROVIDER); | |
| if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { | |
| return "SHA-256"; | |
| } else { | |
| return "SHA-1"; | |
| } | |
| } | |
| private static String getAlgorithm() { | |
| String provider = System.getProperty(JCE_PROVIDER); | |
| if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { | |
| log.debug("Using SHA-256 algorithm for JCE provider: " + provider); | |
| return "SHA-256"; | |
| } else { | |
| log.debug("Using default SHA-1 algorithm"); | |
| return "SHA-1"; | |
| } |
| private static String getCipherAlgorithm() { | ||
| String provider = System.getProperty(JCE_PROVIDER); | ||
| if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { | ||
| return "RSA/ECB/OAEPWithSHA-1AndMGF1Padding"; | ||
| } else { | ||
| return "RSA/ECB/OAEPwithSHA1andMGF1Padding"; | ||
| } | ||
| } |
There was a problem hiding this comment.
Log Improvement Suggestion No: 31
| private static String getCipherAlgorithm() { | |
| String provider = System.getProperty(JCE_PROVIDER); | |
| if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { | |
| return "RSA/ECB/OAEPWithSHA-1AndMGF1Padding"; | |
| } else { | |
| return "RSA/ECB/OAEPwithSHA1andMGF1Padding"; | |
| } | |
| } | |
| private static String getCipherAlgorithm() { | |
| String provider = System.getProperty(JCE_PROVIDER); | |
| if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { | |
| log.debug("JCE provider is configured. Using RSA/ECB/OAEPWithSHA-1AndMGF1Padding cipher algorithm."); | |
| return "RSA/ECB/OAEPWithSHA-1AndMGF1Padding"; | |
| } else { | |
| log.debug("JCE provider is not configured. Using default RSA/ECB/OAEPwithSHA1andMGF1Padding cipher algorithm."); | |
| return "RSA/ECB/OAEPwithSHA1andMGF1Padding"; | |
| } | |
| } |
| } else { | ||
| cipher = Cipher.getInstance("RSA/ECB/OAEPwithSHA1andMGF1Padding"); | ||
| cipher = Cipher.getInstance(getCipherAlgorithm()); | ||
| } |
There was a problem hiding this comment.
Log Improvement Suggestion No: 32
| } else { | |
| cipher = Cipher.getInstance("RSA/ECB/OAEPwithSHA1andMGF1Padding"); | |
| cipher = Cipher.getInstance(getCipherAlgorithm()); | |
| } | |
| } else { | |
| cipher = Cipher.getInstance(getCipherAlgorithm()); | |
| log.debug("Cipher initialized with algorithm: " + getCipherAlgorithm()); | |
| } |
| private static void addBcProviders() { | ||
| String jceProvider = System.getProperty(SECURITY_JCE_PROVIDER); | ||
| if (jceProvider != null) { |
There was a problem hiding this comment.
Log Improvement Suggestion No: 33
| private static void addBcProviders() { | |
| String jceProvider = System.getProperty(SECURITY_JCE_PROVIDER); | |
| if (jceProvider != null) { | |
| private static void addBcProviders() { | |
| String jceProvider = System.getProperty(SECURITY_JCE_PROVIDER); | |
| if (jceProvider != null) { | |
| logger.info("Initializing JCE provider: " + jceProvider); |
| } else { | ||
| throw new RuntimeException("Unsupported JCE provider: " + jceProvider); |
There was a problem hiding this comment.
Log Improvement Suggestion No: 34
| } else { | |
| throw new RuntimeException("Unsupported JCE provider: " + jceProvider); | |
| } else { | |
| logger.error("Unsupported JCE provider: " + jceProvider); | |
| throw new RuntimeException("Unsupported JCE provider: " + jceProvider); |
There was a problem hiding this comment.
AI Agent Log Improvement Checklist
- The log-related comments and suggestions in this review were generated by an AI tool to assist with identifying potential improvements. Purpose of reviewing the code for log improvements is to improve the troubleshooting capabilities of our products.
- Please make sure to manually review and validate all suggestions before applying any changes. Not every code suggestion would make sense or add value to our purpose. Therefore, you have the freedom to decide which of the suggestions are helpful.
✅ Before merging this pull request:
- Review all AI-generated comments for accuracy and relevance.
- Complete and verify the table below. We need your feedback to measure the accuracy of these suggestions and the value they add. If you are rejecting a certain code suggestion, please mention the reason briefly in the suggestion for us to capture it.
WalkthroughThe pull request introduces cryptographic agility by adding support for optional BouncyCastle FIPS providers and dynamic algorithm selection. It replaces hardcoded cryptographic algorithms (SHA-1, MD5, SHA1PRNG) with runtime-selected stronger alternatives (SHA-256, DRBG) when a JCE provider is configured. SSL/TLS configurations now support configurable keystore/truststore types. Build scripts manage FIPS jar installation and verification. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Main as Main.start()
participant BC as BouncyCastle Providers
participant Security as java.security.Security
participant Runtime as Runtime System
App->>Main: Startup
Main->>Main: addBcProviders()
alt security.jce.provider property set
Main->>BC: Initialize BC or BC-FIPS provider via reflection
BC->>Security: Register provider instance
Security->>Runtime: Update available providers
Main->>Main: Log successful registration
else property not set
Main->>Runtime: Skip provider registration
end
Main->>App: Startup complete
rect rgb(200, 220, 255)
Note over App,Runtime: Dynamic Algorithm Selection (Runtime)
end
App->>App: getAlgorithm() / getKeyManagerType()
alt security.jce.provider property non-empty
App->>App: Select SHA-256 / DRBG / PKIX
else property empty or missing
App->>App: Select SHA-1 / SHA1PRNG / default algorithm
end
App->>Runtime: Initialize crypto with selected algorithm
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.39.7)components/org.wso2.micro.integrator.security/src/main/java/org/wso2/micro/integrator/security/user/core/jdbc/JDBCUserStoreManager.java[] Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
components/org.wso2.micro.integrator.security/src/main/java/org/wso2/micro/integrator/security/user/core/jdbc/JDBCUserStoreManager.java (1)
2407-2419: Update error message to reflect dynamic algorithm selection.The error message at line 2416 is hardcoded to reference "SHA1PRNG" but the code now dynamically selects between SHA1PRNG and DRBG based on the JCE provider configuration. This will be misleading when DRBG is being used.
Apply this diff to fix the error message:
} catch (NoSuchAlgorithmException e) { - throw new RuntimeException("SHA1PRNG algorithm could not be found."); + throw new RuntimeException(getAlgorithm() + " algorithm could not be found."); }Additionally, note that a past review comment suggested adding debug logging before generating the salt value (see comment on lines 2407-2410). Consider implementing that logging to aid in troubleshooting FIPS-related issues.
components/org.wso2.micro.integrator.security/src/main/java/org/wso2/micro/integrator/security/user/core/system/SystemUserRoleManager.java (1)
390-392: Update error message to reflect dynamic algorithm selection.The hardcoded error message "SHA1PRNG algorithm could not be found" is now misleading since the code may use DRBG. When DRBG is requested but unavailable, this message will confuse debugging efforts.
- } catch (NoSuchAlgorithmException e) { - throw new RuntimeException("SHA1PRNG algorithm could not be found."); - } + } catch (NoSuchAlgorithmException e) { + throw new RuntimeException("SecureRandom algorithm '" + getAlgorithm() + "' could not be found.", e); + }components/org.wso2.micro.integrator.security/src/main/java/org/wso2/micro/integrator/security/util/ServerCrypto.java (2)
131-145: Critical: cacerts loading will fail with FIPS keystores.When a JCE provider is configured,
getKeyType(properties)returns"BCFKS"by default. However, the JDK'scacertsfile atjava.home/lib/security/cacertsis in JKS or PKCS12 format, not BCFKS format. This mismatch will cause the keystore loading to fail.The exception is caught and handled by falling back to
trustStores.get(0), but this silently masks a configuration issue and may lead to incomplete certificate validation.Use the JDK's default keystore type for loading cacerts:
try { String cacertsPasswd = properties.getProperty(PROP_ID_CACERT_PASS, "changeit"); - cacerts = KeyStore.getInstance(getKeyType(properties)); + cacerts = KeyStore.getInstance(KeyStore.getDefaultType()); cacerts.load(cacertsIs, cacertsPasswd.toCharArray());
485-493: Update error message to reflect dynamic algorithm.The error message on line 489 still references "SHA1", but the algorithm is now dynamically selected and could be SHA-256.
} catch (NoSuchAlgorithmException ex) { throw new WSSecurityException(1, "noSKIHandling", - new Object[]{"Wrong certificate version (<3) and no " - + "SHA1 message digest availabe"}); + new Object[]{"Wrong certificate version (<3) and no " + + getAlgorithm() + " message digest available"}); }
♻️ Duplicate comments (5)
components/org.wso2.micro.integrator.security/src/main/java/org/wso2/micro/integrator/security/user/core/jdbc/JDBCUserStoreManager.java (1)
4343-4349: Add debug logging and improve algorithm selection robustness.The
getAlgorithm()method currently lacks observability and assumes DRBG availability without verification. This can make troubleshooting difficult in production.Issue 1: Missing debug logging (as noted in past review comments)
A past review comment on lines 4343-4348 explicitly requested debug logging. This is important for diagnosing issues with FIPS configuration.
Issue 2: No verification that DRBG is available
When the JCE provider is configured, the code assumes DRBG is available but doesn't verify this. If DRBG isn't supported by the configured provider,
SecureRandom.getInstance()will fail with a runtime exception.Issue 3: Missing documentation
The method lacks javadoc explaining the algorithm selection logic and prerequisites.
Consider this improved implementation:
+ /** + * Returns the SecureRandom algorithm based on JCE provider configuration. + * Returns DRBG when a JCE provider is configured (for FIPS compliance), otherwise SHA1PRNG. + * + * @return algorithm name for SecureRandom + */ private static String getAlgorithm() { if (org.apache.commons.lang.StringUtils.isNotEmpty(System.getProperty(JCE_PROVIDER))) { + if (log.isDebugEnabled()) { + log.debug("JCE provider property is set, using DRBG algorithm for SecureRandom"); + } return DRBG; } else { + if (log.isDebugEnabled()) { + log.debug("JCE provider property not set, using SHA1PRNG algorithm for SecureRandom"); + } return SHA_1_PRNG; } }Alternatively, consider verifying DRBG availability at startup and adding a verification script:
#!/bin/bash # Verify that DRBG algorithm selection logic is consistent across the codebase # and check for similar patterns that might need the same fix echo "Searching for other SecureRandom.getInstance usages in the codebase..." rg -n "SecureRandom\.getInstance" --type java echo -e "\nSearching for other getAlgorithm patterns..." rg -n "getAlgorithm\(\)" --type java echo -e "\nSearching for other references to SHA1PRNG that might need updating..." rg -n "SHA1PRNG|SHA_1_PRNG" --type javacomponents/org.wso2.micro.integrator.server/src/main/java/org/wso2/micro/integrator/server/Main.java (1)
402-415: Add logging for operational visibility.The method is missing logging statements that were suggested in previous reviews:
- Log when initializing a JCE provider (at the start when
jceProvideris not null)- Log an error before throwing the exception for unsupported providers
These logs are important for operational visibility and troubleshooting.
Apply this diff to add the logging:
private static void addBcProviders() { String jceProvider = System.getProperty(SECURITY_JCE_PROVIDER); if (jceProvider != null) { + logger.info("Initializing JCE provider: " + jceProvider); if (BOUNCY_CASTLE_FIPS_PROVIDER.equals(jceProvider)) { System.setProperty(FIPS_APPROVED_ONLY, "true"); System.setProperty("org.bouncycastle.rsa.allow_multi_use", "true"); setBcProviders(BC_FIPS_CLASS_NAME, jceProvider); } else if (BOUNCY_CASTLE_PROVIDER.equals(jceProvider)) { setBcProviders(BC_CLASS_NAME, jceProvider); } else { + logger.error("Unsupported JCE provider: " + jceProvider); throw new RuntimeException("Unsupported JCE provider: " + jceProvider); } } }components/mediation/inbound-endpoints/org.wso2.micro.integrator.inbound.endpoint/src/main/java/org/wso2/carbon/inbound/endpoint/protocol/rabbitmq/RabbitMQConnectionFactory.java (1)
259-275: Consolidate duplicate logic into a single method.Both
getKeyManagerType()andgetTrustManagerType()implement identical logic, violating the DRY principle. Additionally, the logging improvements suggested in a previous review comment have not been implemented.Consider refactoring to a single parameterized method:
- private static String getKeyManagerType() { - String provider = System.getProperty(JCE_PROVIDER); - if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { - return PKIX; - } else { - return KeyManagerFactory.getDefaultAlgorithm(); - } - } - - private static String getTrustManagerType() { - String provider = System.getProperty(JCE_PROVIDER); - if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { - return PKIX; - } else { - return TrustManagerFactory.getDefaultAlgorithm(); - } - } + private static String getAlgorithmType(String defaultAlgorithm, String algorithmTypeLabel) { + String provider = System.getProperty(JCE_PROVIDER); + if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { + log.info("Using PKIX " + algorithmTypeLabel + " type for JCE provider: " + provider); + return PKIX; + } else { + log.debug("Using default " + algorithmTypeLabel + " algorithm: " + defaultAlgorithm); + return defaultAlgorithm; + } + } + + private static String getKeyManagerType() { + return getAlgorithmType(KeyManagerFactory.getDefaultAlgorithm(), "KeyManager"); + } + + private static String getTrustManagerType() { + return getAlgorithmType(TrustManagerFactory.getDefaultAlgorithm(), "TrustManager"); + }components/mediation/inbound-endpoints/org.wso2.micro.integrator.inbound.endpoint/src/main/java/org/wso2/carbon/inbound/endpoint/protocol/mqtt/MqttConnectionFactory.java (1)
354-370: Acknowledge past review comment on logging.A previous review suggested adding debug logging to these methods to aid troubleshooting. This would help operators understand which algorithm is being selected at runtime.
components/mediation/mediators/cache-mediator/org.wso2.carbon.mediator.cache/src/main/java/org/wso2/carbon/mediator/cache/digest/DOMHASHGenerator.java (1)
97-102: Implement debug logging for algorithm selection in DOMHASHGenerator.The logger is properly initialized and the suggested debug statements are appropriate. The algorithm selection logic currently lacks logging, which makes troubleshooting FIPS/BC provider configuration difficult. Apply the diff to add clarity to digest algorithm selection:
byte[] digest; - if(StringUtils.isNotEmpty(System.getProperty(JCE_PROVIDER))) { + if (StringUtils.isNotEmpty(System.getProperty(JCE_PROVIDER))) { + log.debug("Using SHA-256 digest algorithm as JCE provider is configured"); digest = getDigest(request, SHA_256_DIGEST_ALGORITHM); } else { + log.debug("Using MD5 digest algorithm as JCE provider is not configured"); digest = getDigest(request, MD5_DIGEST_ALGORITHM); }Note: When the JCE provider configuration changes, cache keys will differ (SHA-256 vs MD5), potentially invalidating existing cache entries. Verify this behavior is documented in operational guides or migration procedures.
🧹 Nitpick comments (5)
components/org.wso2.micro.integrator.server/src/main/java/org/wso2/micro/integrator/server/Main.java (1)
417-440: LGTM! Well-implemented provider registration.The implementation correctly:
- Uses reflection to dynamically load BC providers
- Inserts JCE provider at position 1 (highest priority) as per PR objectives
- Inserts JSSE provider at position 2 with the JCE provider name parameter
- Provides comprehensive exception handling with descriptive error messages
- Logs successful provider registration
The approach is secure because class names are hardcoded constants, not user input.
Optional enhancement: Consider verifying that the providers were successfully registered by checking
Security.getProvider()after insertion and logging the verification result. This would provide additional operational confidence.Example:
Provider jceProvider = Security.getProvider(jceProviderName); if (jceProvider != null) { logger.info("Verified JCE provider registration: " + jceProvider.getName()); } else { logger.warn("JCE provider registration could not be verified"); }components/org.wso2.micro.integrator.security/src/main/java/org/wso2/micro/integrator/security/user/core/util/UserCoreUtil.java (1)
351-351: Consider enhancing the error message for better debuggability.The change appropriately delegates to
getAlgorithm()for dynamic algorithm selection. The existing exception handling will catch failures, but the error message could be more informative.Optionally, improve the error message to include which algorithm was attempted:
+ String algorithm = getAlgorithm(); try { // the secure random - SecureRandom prng = SecureRandom.getInstance(getAlgorithm()); + SecureRandom prng = SecureRandom.getInstance(algorithm); for (int i = 0; i < length; i++) { password[i] = passwordChars.charAt(prng.nextInt(passwordFeed.length())); } randomNum = new Integer(prng.nextInt()).toString(); } catch (NoSuchAlgorithmException e) { - String errorMessage = "Error while creating the random password for user : " + username; + String errorMessage = "Error while creating the random password for user : " + username + + ". Algorithm '" + algorithm + "' not available."; if (log.isDebugEnabled()) { log.debug(errorMessage, e); } throw new UserStoreException(errorMessage, e); }components/org.wso2.micro.integrator.security/src/main/java/org/wso2/micro/integrator/security/util/ServerCrypto.java (1)
80-80: Remove unused constant.The
DEFAULTconstant is defined but never referenced in the code.- private static final String DEFAULT = "DEFAULT";components/mediation/inbound-endpoints/org.wso2.micro.integrator.inbound.endpoint/src/main/java/org/wso2/carbon/inbound/endpoint/protocol/mqtt/MqttConnectionFactory.java (1)
354-370: Consider consolidating duplicate logic.The
getKeyManagerType()andgetTrustManagerType()methods contain nearly identical logic. Consider extracting the common pattern to reduce duplication.Example consolidation:
- private static String getKeyManagerType() { - String provider = System.getProperty(JCE_PROVIDER); - if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { - return PKIX; - } else { - return KeyManagerFactory.getDefaultAlgorithm(); - } - } - - private static String getTrustManagerType() { - String provider = System.getProperty(JCE_PROVIDER); - if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { - return PKIX; - } else { - return TrustManagerFactory.getDefaultAlgorithm(); - } - } + private static String getKeyManagerType() { + return getManagerType(KeyManagerFactory.getDefaultAlgorithm()); + } + + private static String getTrustManagerType() { + return getManagerType(TrustManagerFactory.getDefaultAlgorithm()); + } + + private static String getManagerType(String defaultAlgorithm) { + String provider = System.getProperty(JCE_PROVIDER); + if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { + return PKIX; + } + return defaultAlgorithm; + }components/mediation/transports/org.wso2.micro.integrator.websocket.transport/src/main/java/org/wso2/micro/integrator/websocket/transport/utils/SSLUtil.java (1)
99-115: Consider renaming methods for clarity.The methods
getTrustManagerType()andgetKeyManagerType()return algorithm names (e.g., "PKIX" or the default algorithm) forTrustManagerFactoryandKeyManagerFactory, not "types" in the same sense as "keyStoreType" or "trustStoreType". In the context of this PR which adds support for keystore/truststore types, these method names could be confusing.Consider renaming for clarity:
-private static String getTrustManagerType() { +private static String getTrustManagerAlgorithm() { String provider = System.getProperty(JCE_PROVIDER); if (StringUtils.isNotEmpty(provider)) { return PKIX; } else { return TrustManagerFactory.getDefaultAlgorithm(); } } -private static String getKeyManagerType() { +private static String getKeyManagerAlgorithm() { String provider = System.getProperty(JCE_PROVIDER); if (StringUtils.isNotEmpty(provider)) { return PKIX; } else { return KeyManagerFactory.getDefaultAlgorithm(); } }And update the call sites accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
components/data/data-services/org.wso2.micro.integrator.dataservices.core/src/main/java/org/wso2/micro/integrator/dataservices/core/auth/JWTAuthorizationProvider.java(3 hunks)components/mediation/inbound-endpoints/org.wso2.micro.integrator.inbound.endpoint/src/main/java/org/wso2/carbon/inbound/endpoint/protocol/mqtt/MqttConnectionFactory.java(3 hunks)components/mediation/inbound-endpoints/org.wso2.micro.integrator.inbound.endpoint/src/main/java/org/wso2/carbon/inbound/endpoint/protocol/rabbitmq/RabbitMQConnectionFactory.java(3 hunks)components/mediation/inbound-endpoints/org.wso2.micro.integrator.inbound.endpoint/src/main/java/org/wso2/carbon/inbound/endpoint/protocol/websocket/InboundWebsocketConstants.java(1 hunks)components/mediation/inbound-endpoints/org.wso2.micro.integrator.inbound.endpoint/src/main/java/org/wso2/carbon/inbound/endpoint/protocol/websocket/management/WebsocketEndpointManager.java(3 hunks)components/mediation/inbound-endpoints/org.wso2.micro.integrator.inbound.endpoint/src/main/java/org/wso2/carbon/inbound/endpoint/protocol/websocket/ssl/InboundWebsocketSSLConfiguration.java(5 hunks)components/mediation/inbound-endpoints/org.wso2.micro.integrator.inbound.endpoint/src/main/java/org/wso2/carbon/inbound/endpoint/protocol/websocket/ssl/SSLHandlerFactory.java(5 hunks)components/mediation/mediators/cache-mediator/org.wso2.carbon.mediator.cache/src/main/java/org/wso2/carbon/mediator/cache/digest/DOMHASHGenerator.java(3 hunks)components/mediation/mediators/cache-mediator/org.wso2.carbon.mediator.cache/src/main/java/org/wso2/carbon/mediator/cache/digest/HttpRequestHashGenerator.java(6 hunks)components/mediation/mediators/cache-mediator/org.wso2.carbon.mediator.cache/src/main/java/org/wso2/carbon/mediator/cache/digest/REQUESTHASHGenerator.java(3 hunks)components/mediation/transports/org.wso2.micro.integrator.websocket.transport/src/main/java/org/wso2/micro/integrator/websocket/transport/WebsocketConnectionFactory.java(4 hunks)components/mediation/transports/org.wso2.micro.integrator.websocket.transport/src/main/java/org/wso2/micro/integrator/websocket/transport/WebsocketConstants.java(1 hunks)components/mediation/transports/org.wso2.micro.integrator.websocket.transport/src/main/java/org/wso2/micro/integrator/websocket/transport/utils/SSLUtil.java(6 hunks)components/org.wso2.micro.integrator.core/src/main/java/org/wso2/micro/core/util/CryptoUtil.java(6 hunks)components/org.wso2.micro.integrator.core/src/main/java/org/wso2/micro/integrator/core/internal/Activator.java(0 hunks)components/org.wso2.micro.integrator.security/src/main/java/org/wso2/micro/integrator/security/user/core/jdbc/JDBCUserStoreManager.java(4 hunks)components/org.wso2.micro.integrator.security/src/main/java/org/wso2/micro/integrator/security/user/core/system/SystemUserRoleManager.java(3 hunks)components/org.wso2.micro.integrator.security/src/main/java/org/wso2/micro/integrator/security/user/core/util/UserCoreUtil.java(3 hunks)components/org.wso2.micro.integrator.security/src/main/java/org/wso2/micro/integrator/security/util/ServerCrypto.java(6 hunks)components/org.wso2.micro.integrator.security/src/main/java/org/wso2/micro/integrator/security/vault/utils/KeyStoreUtil.java(3 hunks)components/org.wso2.micro.integrator.server/src/main/java/org/wso2/micro/integrator/server/Main.java(4 hunks)distribution/pom.xml(1 hunks)distribution/src/assembly/bin.xml(3 hunks)distribution/src/scripts/fips.bat(1 hunks)distribution/src/scripts/fips.sh(1 hunks)pom.xml(2 hunks)
💤 Files with no reviewable changes (1)
- components/org.wso2.micro.integrator.core/src/main/java/org/wso2/micro/integrator/core/internal/Activator.java
🧰 Additional context used
🧬 Code graph analysis (4)
components/mediation/transports/org.wso2.micro.integrator.websocket.transport/src/main/java/org/wso2/micro/integrator/websocket/transport/WebsocketConnectionFactory.java (2)
components/mediation/transports/org.wso2.micro.integrator.websocket.transport/src/main/java/org/wso2/micro/integrator/websocket/transport/WebsocketConstants.java (1)
WebsocketConstants(21-63)components/mediation/transports/org.wso2.micro.integrator.websocket.transport/src/main/java/org/wso2/micro/integrator/websocket/transport/utils/SSLUtil.java (1)
SSLUtil(38-117)
components/mediation/inbound-endpoints/org.wso2.micro.integrator.inbound.endpoint/src/main/java/org/wso2/carbon/inbound/endpoint/protocol/websocket/ssl/InboundWebsocketSSLConfiguration.java (1)
components/mediation/inbound-endpoints/org.wso2.micro.integrator.inbound.endpoint/src/main/java/org/wso2/carbon/inbound/endpoint/protocol/websocket/management/WebsocketEndpointManager.java (1)
WebsocketEndpointManager(50-333)
components/mediation/transports/org.wso2.micro.integrator.websocket.transport/src/main/java/org/wso2/micro/integrator/websocket/transport/utils/SSLUtil.java (1)
components/mediation/transports/org.wso2.micro.integrator.websocket.transport/src/main/java/org/wso2/micro/integrator/websocket/transport/WebsocketConnectionFactory.java (1)
WebsocketConnectionFactory(57-340)
components/mediation/inbound-endpoints/org.wso2.micro.integrator.inbound.endpoint/src/main/java/org/wso2/carbon/inbound/endpoint/protocol/websocket/management/WebsocketEndpointManager.java (1)
components/mediation/inbound-endpoints/org.wso2.micro.integrator.inbound.endpoint/src/main/java/org/wso2/carbon/inbound/endpoint/protocol/websocket/InboundWebsocketConstants.java (1)
InboundWebsocketConstants(21-83)
🪛 Shellcheck (0.11.0)
distribution/src/scripts/fips.sh
[error] 271-271: Since you double quoted this, it will not word split, and the loop will only run once.
(SC2066)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: ubuntu
- GitHub Check: ubuntu (2, tests-service_tests-patches_service-samples)
- GitHub Check: ubuntu (3, tests-transport_tests-platform)
- GitHub Check: ubuntu (4, management-api_dss-tests)
- GitHub Check: ubuntu (1, tests-mediators_tests-other_tests-sample)
🔇 Additional comments (21)
components/mediation/inbound-endpoints/org.wso2.micro.integrator.inbound.endpoint/src/main/java/org/wso2/carbon/inbound/endpoint/protocol/websocket/InboundWebsocketConstants.java (1)
32-35: LGTM! Clean addition of SSL store type constants.The new constants for keystore and truststore types are well-placed and follow the existing naming conventions perfectly. They logically fit between the related file/pass configuration constants, enabling the FIPS functionality by allowing different keystore types (e.g., "BCFKS" for BCFIPS providers).
components/org.wso2.micro.integrator.security/src/main/java/org/wso2/micro/integrator/security/user/core/jdbc/JDBCUserStoreManager.java (1)
96-96: LGTM! Constants are properly defined.The new constants
DRBGandJCE_PROVIDERare well-defined and support the dynamic algorithm selection for FIPS compliance.Also applies to: 106-106
components/org.wso2.micro.integrator.security/src/main/java/org/wso2/micro/integrator/security/user/core/system/SystemUserRoleManager.java (2)
385-385: LGTM: Dynamic algorithm selection.The change correctly implements runtime algorithm selection based on the JCE provider configuration, aligning with the FIPS support objectives.
640-647: LGTM: Algorithm selection logic is correct.The implementation correctly returns DRBG when a JCE provider is configured and SHA_1_PRNG otherwise. This aligns with the PR objectives for FIPS support.
Note: DRBG requires Java 9+. Ensure the minimum supported JVM version for Micro Integrator is documented if not already specified.
Would you like me to verify the minimum Java version requirements for this project?
components/org.wso2.micro.integrator.server/src/main/java/org/wso2/micro/integrator/server/Main.java (4)
47-48: LGTM!The imports are necessary for the BC provider registration functionality.
73-73: LGTM!Making the logger field final is a good practice.
74-80: LGTM!The constants are well-defined and follow proper naming conventions.
128-128: LGTM!The placement of
addBcProviders()is appropriate - it's called after configuration handling and before extensions, which is correct for security provider initialization.components/org.wso2.micro.integrator.security/src/main/java/org/wso2/micro/integrator/security/user/core/util/UserCoreUtil.java (1)
67-67: LGTM!The constant declaration follows Java naming conventions and centralizes the system property key.
components/org.wso2.micro.integrator.security/src/main/java/org/wso2/micro/integrator/security/util/ServerCrypto.java (1)
564-570: LGTM!Simplifying to always use "X.509" certificate factory is correct, as X.509 is a standard format supported by all JCE providers.
components/mediation/inbound-endpoints/org.wso2.micro.integrator.inbound.endpoint/src/main/java/org/wso2/carbon/inbound/endpoint/protocol/rabbitmq/RabbitMQConnectionFactory.java (2)
57-58: LGTM!The constants are well-defined and properly scoped for supporting dynamic JCE provider selection.
188-188: PKIX is universally supported by all intended JCE providers—no validation issue exists.The web search confirms that BouncyCastle FIPS (bcpkix-fips/bcjsse-fips), BouncyCastle standard (BC), and the default SUN provider all provide full PKIX support for KeyManagerFactory and TrustManagerFactory. The code correctly uses PKIX when a custom JCE provider is configured, and falls back to the default algorithm otherwise. Error handling for
NoSuchAlgorithmExceptionis present in calling contexts (e.g., ServerCrypto.java:702-707 shows proper try-catch wrapping).The implementation is consistent across all connection factories and follows the intended design pattern for BC/BCFIPS integration.
components/mediation/inbound-endpoints/org.wso2.micro.integrator.inbound.endpoint/src/main/java/org/wso2/carbon/inbound/endpoint/protocol/mqtt/MqttConnectionFactory.java (3)
54-55: LGTM! Good use of constants.The constants improve code maintainability by avoiding magic strings throughout the implementation.
338-338: LGTM! Proper delegation to helper methods.The changes correctly delegate algorithm selection to the new helper methods, centralizing the FIPS-aware logic.
Also applies to: 345-345
354-370: Code follows correct pattern for PKIX with BouncyCastle FIPS provider — no changes needed.BouncyCastle FIPS provider explicitly supports PKIX algorithm for KeyManagerFactory and TrustManagerFactory, and the logic here correctly switches to PKIX when a custom JCE provider is explicitly configured. The defensive approach (checking for non-empty provider, then returning PKIX) aligns with vendor-recommended configuration for BCFIPS deployments.
components/mediation/mediators/cache-mediator/org.wso2.carbon.mediator.cache/src/main/java/org/wso2/carbon/mediator/cache/digest/DOMHASHGenerator.java (2)
28-28: LGTM: Import necessary for system property check.The StringUtils import is required for the non-empty check on the JCE provider system property.
59-60: LGTM: Constants appropriately defined.The SHA-256 algorithm constant follows the existing pattern of other algorithm constants, and the JCE provider key is properly scoped as private.
components/mediation/transports/org.wso2.micro.integrator.websocket.transport/src/main/java/org/wso2/micro/integrator/websocket/transport/WebsocketConstants.java (1)
50-51: LGTM!The new constants for keystore and truststore types follow the existing naming conventions and provide the necessary configuration keys for FIPS support.
components/mediation/inbound-endpoints/org.wso2.micro.integrator.inbound.endpoint/src/main/java/org/wso2/carbon/inbound/endpoint/protocol/websocket/ssl/InboundWebsocketSSLConfiguration.java (1)
28-29: Making fields final is good practice.Converting
keyStoreandkeyStorePassto final fields improves immutability and makes the constructor contract clearer—these values cannot be changed after construction.components/mediation/mediators/cache-mediator/org.wso2.carbon.mediator.cache/src/main/java/org/wso2/carbon/mediator/cache/digest/REQUESTHASHGenerator.java (1)
260-267: Helper ensures SHA-256 when provider is configuredSwitching through
getAlgorithm()preserves MD5 by default but automatically upgrades to SHA-256 oncesecurity.jce.provideris present, keeping cache digests compatible with FIPS deployments. Looks good.components/org.wso2.micro.integrator.core/src/main/java/org/wso2/micro/core/util/CryptoUtil.java (1)
458-464: Thumbprint digest upgrade aligns with provider policyUsing
getAlgorithm()here means thumbprints move to SHA-256 automatically wheneversecurity.jce.provideris set, while retaining SHA-1 for existing deployments. This matches the new provider ordering strategy; nice work.
| private static String getAlgorithm() { | ||
| if (StringUtils.isNotEmpty(System.getProperty(SECURITY_JCE_PROVIDER))) { | ||
| return "SHA-256"; | ||
| } else { | ||
| return "SHA-1"; | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix unresolved reference to StringUtils.
StringUtils isn't imported anywhere in this class, so the new getAlgorithm() method won’t compile. Please either add the proper import or, more simply, avoid the dependency by checking the property directly.
- if (StringUtils.isNotEmpty(System.getProperty(SECURITY_JCE_PROVIDER))) {
+ String provider = System.getProperty(SECURITY_JCE_PROVIDER);
+ if (provider != null && !provider.isEmpty()) {
return "SHA-256";
} else {
return "SHA-1";
}🤖 Prompt for AI Agents
In
components/data/data-services/org.wso2.micro.integrator.dataservices.core/src/main/java/org/wso2/micro/integrator/dataservices/core/auth/JWTAuthorizationProvider.java
around lines 308 to 314, the new getAlgorithm() uses StringUtils.isNotEmpty but
StringUtils is not imported causing a compile error; fix by removing the
dependency and directly checking the JVM property (e.g., retrieve
System.getProperty(SECURITY_JCE_PROVIDER) into a local var and test it for
null/empty) or alternatively add the proper import for StringUtils if you prefer
to keep it; update the method to use the direct System.getProperty check and
return "SHA-256" when the property is present/non-empty, otherwise "SHA-1".
| private String keyStoreType = "JKS"; | ||
| private String trustStoreType = "JKS"; |
There was a problem hiding this comment.
Critical: Thread safety issue with mutable singleton state.
The instance fields keyStoreType and trustStoreType are mutated in buildSSLConfiguration() (lines 278-279) without synchronization, but WebsocketEndpointManager is a singleton that can be accessed concurrently by multiple threads calling startSSLEndpoint(). This creates a race condition where:
- Thread A calls
buildSSLConfiguration()for endpoint X with keyStoreType="BCFKS" - Thread B calls
buildSSLConfiguration()for endpoint Y with keyStoreType="JKS" - Thread B's assignment overwrites Thread A's value before Thread A's consumer reads it via
getKeyStoreType()
Additionally, the values assigned on lines 278-279 are not used within buildSSLConfiguration() itself—the builder doesn't receive these values. This suggests the mutation is solely for external consumption via the getters, which is a problematic design.
Recommended fix: Pass keyStoreType and trustStoreType directly to the SSLConfigurationBuilder instead of storing them in instance fields:
public InboundWebsocketSSLConfiguration buildSSLConfiguration(InboundProcessorParams params) {
- keyStoreType = params.getProperties().getProperty(InboundWebsocketConstants.INBOUND_SSL_KEY_STORE_TYPE);
- trustStoreType = params.getProperties().getProperty(InboundWebsocketConstants.INBOUND_SSL_TRUST_STORE_TYPE);
+ String keyStoreType = params.getProperties().getProperty(InboundWebsocketConstants.INBOUND_SSL_KEY_STORE_TYPE);
+ String trustStoreType = params.getProperties().getProperty(InboundWebsocketConstants.INBOUND_SSL_TRUST_STORE_TYPE);
return new InboundWebsocketSSLConfiguration.SSLConfigurationBuilder(
params.getProperties().getProperty(InboundWebsocketConstants.INBOUND_SSL_KEY_STORE_FILE),
params.getProperties().getProperty(InboundWebsocketConstants.INBOUND_SSL_KEY_STORE_PASS),
params.getProperties().getProperty(InboundWebsocketConstants.INBOUND_SSL_TRUST_STORE_FILE),
params.getProperties().getProperty(InboundWebsocketConstants.INBOUND_SSL_TRUST_STORE_PASS),
params.getProperties().getProperty(InboundWebsocketConstants.INBOUND_SSL_CERT_PASS),
params.getProperties().getProperty(InboundWebsocketConstants.SSL_PROTOCOLS),
- params.getProperties().getProperty(InboundWebsocketConstants.CIPHER_SUITES)).build();
+ params.getProperties().getProperty(InboundWebsocketConstants.CIPHER_SUITES),
+ keyStoreType,
+ trustStoreType).build();
}Then update SSLConfigurationBuilder to accept these parameters. If external code needs access to the configured types, they should retrieve them from the InboundWebsocketSSLConfiguration object itself, not from the manager singleton.
Also applies to: 278-279, 326-332
| public InboundWebsocketSSLConfiguration(File keyStore, String keyStorePass) { | ||
| this.keyStore = keyStore; | ||
| this.keyStorePass = keyStorePass; | ||
| this.keyStoreType = WebsocketEndpointManager.getInstance().getKeyStoreType(); |
There was a problem hiding this comment.
Critical: Tight coupling to singleton with race condition.
Line 57 reads keyStoreType from WebsocketEndpointManager.getInstance().getKeyStoreType(), which has the race condition identified in the first file review. This creates a critical issue where the SSL configuration may be built with the wrong keystore type due to concurrent modifications to the singleton's mutable state.
Furthermore, this tight coupling between InboundWebsocketSSLConfiguration and WebsocketEndpointManager violates separation of concerns—the SSL configuration should not need to reach into a singleton to get its configuration values.
Recommended fix: Pass the keystore type as a constructor parameter instead:
-public InboundWebsocketSSLConfiguration(File keyStore, String keyStorePass) {
+public InboundWebsocketSSLConfiguration(File keyStore, String keyStorePass, String keyStoreType) {
this.keyStore = keyStore;
this.keyStorePass = keyStorePass;
- this.keyStoreType = WebsocketEndpointManager.getInstance().getKeyStoreType();
+ this.keyStoreType = keyStoreType != null ? keyStoreType : "JKS";
}Then update the builder to pass this value from buildSSLConfiguration() in WebsocketEndpointManager.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
components/mediation/inbound-endpoints/org.wso2.micro.integrator.inbound.endpoint/src/main/java/org/wso2/carbon/inbound/endpoint/protocol/websocket/ssl/InboundWebsocketSSLConfiguration.java
around line 57, the class currently reads the keystore type directly from
WebsocketEndpointManager.getInstance() causing a race condition and tight
coupling; change the class to accept keyStoreType as a constructor parameter
(remove the call to the singleton) and store that value on construction, then
update WebsocketEndpointManager.buildSSLConfiguration() to pass its keystore
type into the InboundWebsocketSSLConfiguration constructor when building
instances so the configuration is immutable and no longer depends on the
singleton at read time.
| private final String keyStoreFile; | ||
| private final String keyStorePass; | ||
| private String keyStoreType = "JKS"; | ||
| private final String trustStoreFile; | ||
| private final String trustStorePass; | ||
| private String trustStoreType = "JKS"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Design inconsistency: Builder fields are unused.
The SSLConfigurationBuilder declares keyStoreType and trustStoreType fields (lines 113, 116), but the build() method doesn't pass keyStoreType to the InboundWebsocketSSLConfiguration constructor. Instead, the configuration reads it from WebsocketEndpointManager (line 57 in the constructor). Only trustStoreType is passed via the setter (line 170).
This creates an inconsistent pattern where:
trustStoreTypeflows from builder → configuration via setterkeyStoreTypeis ignored by builder and read from singleton instead
Recommended fix: Make the builder fully responsible for passing both store types:
- Update the constructor to accept keyStoreType:
public InboundWebsocketSSLConfiguration(File keyStore, String keyStorePass, String keyStoreType) {
this.keyStore = keyStore;
this.keyStorePass = keyStorePass;
this.keyStoreType = keyStoreType != null ? keyStoreType : "JKS";
}- Update the builder to pass it:
InboundWebsocketSSLConfiguration sslConfig = new InboundWebsocketSSLConfiguration(
keyStore, keyStorePass, keyStoreType)
.setCertPass(certPass);- Add setters to the builder to allow setting these types:
public SSLConfigurationBuilder keyStoreType(String keyStoreType) {
this.keyStoreType = keyStoreType;
return this;
}
public SSLConfigurationBuilder trustStoreType(String trustStoreType) {
this.trustStoreType = trustStoreType;
return this;
}Also applies to: 143-171
🤖 Prompt for AI Agents
components/mediation/inbound-endpoints/org.wso2/micro.integrator.inbound.endpoint/src/main/java/org/wso2/carbon/inbound/endpoint/protocol/websocket/ssl/InboundWebsocketSSLConfiguration.java
lines 111-116 and related builder code (approx 143-171): the builder declares
keyStoreType and trustStoreType but keyStoreType is not passed into the
InboundWebsocketSSLConfiguration constructor (the constructor reads it from
WebsocketEndpointManager), creating inconsistent flow; fix by adding a
constructor overload or parameter to accept keyStoreType and set the instance
field (defaulting to "JKS" when null), update the
SSLConfigurationBuilder.build() to pass keyStoreType when constructing the
configuration, and add fluent setter methods on the builder for keyStoreType and
trustStoreType so callers can set them; also remove reliance on
WebsocketEndpointManager inside the configuration constructor for keyStoreType
so both types consistently come from the builder.
| private static String getKeyManagerType() { | ||
| if (StringUtils.isNotEmpty(System.getProperty(JCE_PROVIDER))) { | ||
| return PKIX; | ||
| } else { | ||
| return KeyManagerFactory.getDefaultAlgorithm(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Guard PKIX selection with an availability check.
When security.jce.provider is set, this method unconditionally returns PKIX. If the selected provider (e.g., the stock BC JCE provider without BCJSSE) does not advertise a KeyManagerFactory for PKIX, KeyManagerFactory.getInstance("PKIX") will throw NoSuchAlgorithmException, aborting server startup. Fall back to the default algorithm unless the provider actually exposes PKIX.
- private static String getKeyManagerType() {
- if (StringUtils.isNotEmpty(System.getProperty(JCE_PROVIDER))) {
- return PKIX;
- } else {
- return KeyManagerFactory.getDefaultAlgorithm();
- }
- }
+ private static String getKeyManagerType() {
+ if (StringUtils.isNotEmpty(System.getProperty(JCE_PROVIDER))
+ && Security.getAlgorithms("KeyManagerFactory").contains(PKIX)) {
+ return PKIX;
+ }
+ return KeyManagerFactory.getDefaultAlgorithm();
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private static String getKeyManagerType() { | |
| if (StringUtils.isNotEmpty(System.getProperty(JCE_PROVIDER))) { | |
| return PKIX; | |
| } else { | |
| return KeyManagerFactory.getDefaultAlgorithm(); | |
| } | |
| } | |
| private static String getKeyManagerType() { | |
| if (StringUtils.isNotEmpty(System.getProperty(JCE_PROVIDER)) | |
| && Security.getAlgorithms("KeyManagerFactory").contains(PKIX)) { | |
| return PKIX; | |
| } | |
| return KeyManagerFactory.getDefaultAlgorithm(); | |
| } |
🤖 Prompt for AI Agents
In
components/mediation/inbound-endpoints/org.wso2.micro.integrator.inbound.endpoint/src/main/java/org/wso2/carbon/inbound/endpoint/protocol/websocket/ssl/SSLHandlerFactory.java
around lines 104-110, currently PKIX is returned unconditionally when a JCE
provider is set; change this to check whether the configured provider actually
exposes a KeyManagerFactory service for "PKIX" and only return PKIX if that
service exists, otherwise fall back to KeyManagerFactory.getDefaultAlgorithm();
implement this by reading the provider name from
System.getProperty(JCE_PROVIDER), resolving Security.getProvider(providerName)
(handle null), checking provider.getService("KeyManagerFactory", "PKIX") !=
null, and using that result to decide between "PKIX" and the default algorithm.
| sha = MessageDigest.getInstance(getAlgorithm()); | ||
| } catch (NoSuchAlgorithmException e1) { | ||
| throw new WSSecurityException(0, "noSHA1availabe"); | ||
| } |
There was a problem hiding this comment.
Update error message to reflect dynamic algorithm.
The error message on line 516 references "noSHA1availabe", which is inaccurate since the algorithm could be SHA-256 when a JCE provider is configured.
Consider updating the error code or message to be algorithm-agnostic:
try {
sha = MessageDigest.getInstance(getAlgorithm());
} catch (NoSuchAlgorithmException e1) {
- throw new WSSecurityException(0, "noSHA1availabe");
+ throw new WSSecurityException(0, "noDigestAlgorithmAvailable",
+ new Object[]{getAlgorithm()});
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
components/org.wso2.micro.integrator.security/src/main/java/org/wso2/micro/integrator/security/util/ServerCrypto.java
around lines 514 to 517, the catch block throws WSSecurityException with a
hardcoded "noSHA1availabe" message which is inaccurate for algorithms other than
SHA-1; change the thrown exception to be algorithm-agnostic by either using a
generic error code/message (e.g., "noMessageDigestAlgorithmAvailable") or
include the actual algorithm name from getAlgorithm() in the message (e.g.,
format the message to contain getAlgorithm()), so the exception reflects the
algorithm attempted.
| private static String getAlgorithm() { | ||
| String provider = System.getProperty(JCE_PROVIDER); | ||
| if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { | ||
| return "SHA-256"; | ||
| } else { | ||
| return "SHA-1"; | ||
| } | ||
| } |
There was a problem hiding this comment.
Validate provider capabilities or document assumptions.
The method assumes that any configured JCE provider supports SHA-256. If an invalid or unsupported provider is specified, this will cause NoSuchAlgorithmException at runtime in methods like getSKIBytesFromCert (line 485) and getAliasForX509CertThumb (line 514).
Consider one of the following approaches:
- Document that the configured provider must support SHA-256
- Add provider validation at startup
- Gracefully fall back to SHA-1 if SHA-256 is not available
🤖 Prompt for AI Agents
In
components/org.wso2.micro.integrator.security/src/main/java/org/wso2/micro/integrator/security/util/ServerCrypto.java
around lines 712 to 719, update getAlgorithm() so it does not assume any
configured JCE provider supports SHA-256; instead read the JCE provider system
property and attempt to validate it by calling
MessageDigest.getInstance("SHA-256", provider) (or getInstance("SHA-256") if
provider is empty) and return "SHA-256" if that succeeds, otherwise catch the
relevant exceptions and either (a) log a clear warning and return "SHA-1" as a
safe fallback or (b) rethrow/abort startup with a descriptive error if you
prefer fail-fast; ensure the method logs which algorithm/provider is actually
selected so callers like getSKIBytesFromCert and getAliasForX509CertThumb will
not encounter unexpected NoSuchAlgorithmException at runtime.
| private static String getKeyType(Properties prop) { | ||
| String provider = System.getProperty(JCE_PROVIDER); | ||
| if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { | ||
| return prop.getProperty("org.wso2.carbon.security.crypto.type", "BCFKS"); | ||
| } else { | ||
| return prop.getProperty("org.wso2.carbon.security.crypto.type", KeyStore.getDefaultType()); | ||
| } | ||
| } |
There was a problem hiding this comment.
Critical: Hardcoded BCFKS type breaks BC (non-FIPS) support.
The method hardcodes "BCFKS" for any configured JCE provider, but BCFKS is specific to Bouncy Castle FIPS. According to the PR objectives, the implementation should support both BC (Bouncy Castle) and BCFIPS (Bouncy Castle FIPS):
- BC uses
BKSkeystore type - BCFIPS uses
BCFKSkeystore type
When a user configures -Dsecurity.jce.provider=BC, the keystore loading will fail because BC doesn't support the BCFKS format.
Differentiate between provider types:
private static String getKeyType(Properties prop) {
String provider = System.getProperty(JCE_PROVIDER);
if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) {
- return prop.getProperty("org.wso2.carbon.security.crypto.type", "BCFKS");
+ String defaultType = "BCFIPS".equals(provider) ? "BCFKS" : "BKS";
+ return prop.getProperty("org.wso2.carbon.security.crypto.type", defaultType);
} else {
return prop.getProperty("org.wso2.carbon.security.crypto.type", KeyStore.getDefaultType());
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private static String getKeyType(Properties prop) { | |
| String provider = System.getProperty(JCE_PROVIDER); | |
| if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { | |
| return prop.getProperty("org.wso2.carbon.security.crypto.type", "BCFKS"); | |
| } else { | |
| return prop.getProperty("org.wso2.carbon.security.crypto.type", KeyStore.getDefaultType()); | |
| } | |
| } | |
| private static String getKeyType(Properties prop) { | |
| String provider = System.getProperty(JCE_PROVIDER); | |
| if (org.apache.commons.lang.StringUtils.isNotEmpty(provider)) { | |
| String defaultType = "BCFIPS".equals(provider) ? "BCFKS" : "BKS"; | |
| return prop.getProperty("org.wso2.carbon.security.crypto.type", defaultType); | |
| } else { | |
| return prop.getProperty("org.wso2.carbon.security.crypto.type", KeyStore.getDefaultType()); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
components/org.wso2.micro.integrator.security/src/main/java/org/wso2/micro/integrator/security/util/ServerCrypto.java
around lines 721-728, the method currently hardcodes "BCFKS" whenever a JCE
provider is set which breaks non-FIPS BC support; update the logic to inspect
the provider string and return the keystore type accordingly: if provider
indicates BCFIPS (e.g., equalsIgnoreCase "BCFIPS" or contains "BCFIPS") use
prop.getProperty("org.wso2.carbon.security.crypto.type","BCFKS"); if provider
indicates BC (e.g., equalsIgnoreCase "BC" or contains "BC") use
prop.getProperty("org.wso2.carbon.security.crypto.type","BKS"); otherwise fall
back to prop.getProperty("org.wso2.carbon.security.crypto.type",
KeyStore.getDefaultType()). Ensure checks are case-insensitive and preserve the
existing property override behavior.
| :: ======= FIPS (to install in default mode) ======= | ||
| set "BC_FIPS_VERSION=2.0.1" | ||
| set "BCPKIX_FIPS_VERSION=2.0.10" | ||
| set "BCTLS_FIPS_VERSION=2.0.19" | ||
| set "BCUTIL_FIPS_VERSION=2.0.2" | ||
|
|
||
| :: Official SHA-1 checksums (Maven Central) | ||
| set "EXPECTED_BC_FIPS_CHECKSUM=67cf4d43d0e86b8a493cfdfe266c226ff7ffc410" | ||
| set "EXPECTED_BCPKIX_FIPS_CHECKSUM=4cc5a8607f3dd6cd3fb0ee5abc2e7a068adf2cf1" | ||
| set "EXPECTED_BCTLS_FIPS_CHECKSUM=9cc33650ede63bc1a8281ed5c8e1da314d50bc76" | ||
| set "EXPECTED_BCUTIL_FIPS_CHECKSUM=c11996822d9d0f831b340bf4ea4d9d3e87a8e9de" | ||
|
|
||
| :: ======= Legacy (non-FIPS) to restore on DISABLE if no backup exists ======= |
There was a problem hiding this comment.
Align Windows installer with the latest BC-FJA release.
The batch installer still stages the 2.0.1/2.0.x artifacts, so Windows users miss the BC-FJA 2.1.2 update (and its CVE remediation work). Please refresh the version and checksum constants to 2.1.2—or provide an overridable mechanism—so that both platforms consume the same up-to-date FIPS jars. (bouncycastle.org)
🤖 Prompt for AI Agents
In distribution/src/scripts/fips.bat around lines 20-32, the script hardcodes
BouncyCastle FIPS versions and SHA-1 checksums for 2.0.x; update the BC_FIPS_*
variables to use version 2.1.2 and replace the EXPECTED_*_CHECKSUM values with
the official 2.1.2 SHA-1 checksums (fetch them from Maven Central or the
BouncyCastle download page), and optionally make these values overridable via
environment variables (fall back to the updated constants) so Windows installer
consumers receive the same BC‑FJA 2.1.2 artifacts as other platforms.
| # ======= FIPS (to install in default mode) ======= | ||
| BC_FIPS_VERSION="2.0.1" | ||
| BCPKIX_FIPS_VERSION="2.0.10" | ||
| BCTLS_FIPS_VERSION="2.0.19" | ||
| BCUTIL_FIPS_VERSION="2.0.2" | ||
|
|
||
| # Official SHA-1 checksums (Maven Central) | ||
| EXPECTED_BC_FIPS_CHECKSUM="67cf4d43d0e86b8a493cfdfe266c226ff7ffc410" | ||
| EXPECTED_BCPKIX_FIPS_CHECKSUM="4cc5a8607f3dd6cd3fb0ee5abc2e7a068adf2cf1" | ||
| EXPECTED_BCTLS_FIPS_CHECKSUM="9cc33650ede63bc1a8281ed5c8e1da314d50bc76" | ||
| EXPECTED_BCUTIL_FIPS_CHECKSUM="c11996822d9d0f831b340bf4ea4d9d3e87a8e9de" | ||
|
|
||
| LEGACY_BCPROV_VERSION="1.78.1" |
There was a problem hiding this comment.
Update FIPS artifacts to latest patch release.
Hard-coding the 2.0.1/2.0.x line means the installer always pulls a build that predates the current BC-FJA 2.1.2 patch stream, which Bouncy Castle now ships with additional fixes (including CVE-2025-12194) and improved cleanup logic. For the November 10, 2025 release window we should either bump these constants to 2.1.2 (and matching checksum values) or make the version configurable so operators can stay on the supported update stream. (bouncycastle.org)
🤖 Prompt for AI Agents
In distribution/src/scripts/fips.sh around lines 20 to 32, the script pins
Bouncy Castle FIPS artifacts to the old 2.0.x line; update the script to use the
current patch stream (bump BC_FIPS_VERSION, BCPKIX_FIPS_VERSION,
BCTLS_FIPS_VERSION, BCUTIL_FIPS_VERSION to 2.1.2 and replace EXPECTED_*_CHECKSUM
values with the official SHA-1 sums for the 2.1.2 artifacts), or alternatively
make these four version variables and their checksums configurable via
environment variables (e.g., honor
BC_FIPS_VERSION_OVERRIDE/EXPECTED_BC_FIPS_CHECKSUM_OVERRIDE) so operators can
upgrade without editing the script; ensure any added env fallback preserves
current defaults and that checksum verification uses the new values.
1 participant
Bouncy Castle Overview
Bouncy Castle is an open-source cryptography library for Java. It provides a wide range of cryptographic algorithms and utilities, including:
Bouncy Castle FIPS
Bouncy Castle FIPS is a special version of the Bouncy Castle cryptography library that has been validated against the FIPS 140-2 / 140-3 standards.
This implementation enables BC and FIPS compliance. You can enable it by running the MI server script with the appropriate argument, as shown below:
For Bouncy Castle (BC):
For Bouncy Castle FIPS (BCFIPS):
Key Points for Implementation
Summary by CodeRabbit
New Features
Chores