[
https://issues.apache.org/jira/browse/KNOX-3118?focusedWorklogId=964955&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-964955
]
ASF GitHub Bot logged work on KNOX-3118:
----------------------------------------
Author: ASF GitHub Bot
Created on: 07/Apr/25 08:02
Start Date: 07/Apr/25 08:02
Worklog Time Spent: 10m
Work Description: smolnar82 commented on code in PR #1015:
URL: https://github.com/apache/knox/pull/1015#discussion_r2030651750
##########
gateway-spi/src/main/java/org/apache/knox/gateway/config/GatewayConfig.java:
##########
@@ -92,6 +92,8 @@ public interface GatewayConfig {
String CREDENTIAL_STORE_ALG = "gateway.credential.store.alg";
String DEFAULT_CREDENTIAL_STORE_ALG = "AES";
+ String CREDENTIAL_SELF_SIGNING_ALG = "gateway.credential.self-signing.alg";
Review Comment:
Two notes:
- please use `.` as the separator character
- I'm not sure the property name is correct. I think
`gateway.self.signing.cert.alg` is more accurate
##########
gateway-server/src/test/java/org/apache/knox/gateway/services/security/impl/DefaultKeystoreServiceTest.java:
##########
@@ -814,7 +815,7 @@ private GatewayConfigImpl createGatewayConfig(Path baseDir)
{
}
- private void createKeystore(DefaultKeystoreService keystoreService, Path
keystoreFilePath, String alias, char[] password)
+ private void createKeystore(DefaultKeystoreService keystoreService, Path
keystoreFilePath, String alias, char[] password, String selfSigningAlgorithm)
Review Comment:
If I were you, I'd pass the `GatewayConfig` object here like this:
```
private void createKeystore(DefaultKeystoreService keystoreService, char[]
password, final GatewayConfig config)
```
and inline the `keystoreFilePath`, `alias`, and
`selfSigningCertificateAlgorithm` when they are used within this private method.
This way it will be more self-contained and resilient for future changes to
the gateway config (i.e. if more properties are added, no need to update the
method signature).
##########
gateway-spi/src/main/java/org/apache/knox/gateway/config/GatewayConfig.java:
##########
@@ -266,6 +268,11 @@ public interface GatewayConfig {
*/
String getCredentialStoreAlgorithm();
+ /**
+ * @return the algorithm that is used when generating a self-signing
certificate.
+ */
+ String getCredentialSelfSigningAlgorithm();
Review Comment:
According to the above, I think `getSelfSigningCertificateAlgorithm()` is a
better method name choice.
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/security/impl/DefaultKeystoreService.java:
##########
@@ -117,6 +118,7 @@ public void init(GatewayConfig config, Map<String, String>
options)
}
this.credentialStoreAlgorithm = config.getCredentialStoreAlgorithm();
+ this.credentialSelfSigningAlgorithm =
config.getCredentialSelfSigningAlgorithm();
Review Comment:
nit: `selfSignedCertificateAlgorithm`
Issue Time Tracking
-------------------
Worklog Id: (was: 964955)
Time Spent: 40m (was: 0.5h)
> Upgrade Knox SSL Self-Signed Certificate from SHA-1 to SHA-256
> --------------------------------------------------------------
>
> Key: KNOX-3118
> URL: https://issues.apache.org/jira/browse/KNOX-3118
> Project: Apache Knox
> Issue Type: Improvement
> Components: Server
> Affects Versions: 1.6.0
> Reporter: ChenXi
> Priority: Major
> Labels: security
> Time Spent: 40m
> Remaining Estimate: 0h
>
> SHA-1, currently used in Knox's current SSL certificates, is
> cryptographically broken. Proven collision attacks (e.g., SHAttered attack in
> 2017) allow malicious actors to forge certificates, exposing Knox to
> man-in-the-middle (MITM) attacks.
> Major browsers (Chrome, Firefox) and operating systems deprecated SHA-1
> support by 2017, leading to trust warnings for SHA-1-based certificates.
> Therefore, it is necessary to upgrade the default self-signing algorithm of
> knox from SHA1 to the more secure SHA2(e.g. SHA256).
> *Reference:*
> * SHA-1 : [https://en.wikipedia.org/wiki/SHA-1]
> * SHA-2: [https://en.wikipedia.org/wiki/SHA-2]
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
