YYTVicky commented on a change in pull request #1087:
URL: https://github.com/apache/brooklyn-server/pull/1087#discussion_r417712318
##########
File path:
utils/common/src/main/java/org/apache/brooklyn/util/crypto/SslTrustUtils.java
##########
@@ -39,7 +39,7 @@
return connection;
}
- /** trusts all SSL certificates */
+ /** trusts all SSL certificates, it is not secure*/
Review comment:
Hi,
Thanks for your kind feedback, our tool detected the checkClientTrusted
and checkServerTrusted couldn't be the empty body or simply return true. It
might cause Man-in-the-middle attacks. Here is a document to address these
issues
(https://www.guardrails.io/docs/en/vulnerabilities/java/insecure_network_communication).
our tool also suggested some code template for such case, simply replace the
current implementation may not works, but hope it gives some guideline for
implementation!
new X509TrustManager(){
@override
public void checkClientTrusted(X509Certificate[] chain, String authType)
throws CertificateException {
for (final X509TrustManager trustManager :
trustManagers) {
try {
trustManager.checkClientTrusted(chain,
authType);
return;
} catch (final CertificateException e) {
//LOGGER.debug(e.getMessage(), e);
}
}
throw new CertificateException("None of the
TrustManagers trust this certificate chain");
}
@Override
public X509Certificate[] getAcceptedIssuers() {
for (final X509TrustManager trustManager :
trustManagers) {
final List<X509Certificate> list =
Arrays.asList(trustManager.getAcceptedIssuers());
certificates.addAll(list);
}
return certificates.toArray(new X509Certificate[] {});
}
@Override
public void checkServerTrusted(X509Certificate[] chain, String
authType) throws CertificateException{
if (chain == null) {
throw new
IllegalArgumentException("checkServerTrusted:x509Certificate array isnull");
}
if (!(chain.length > 0)) {
throw new
IllegalArgumentException("checkServerTrusted: X509Certificate is empty");
}
if (!(null != authType &&
authType.equalsIgnoreCase("RSA"))) {
throw new
CertificateException("checkServerTrusted: AuthType is not RSA");
}
try {
TrustManagerFactory tmf =
TrustManagerFactory.getInstance("X509");
tmf.init((KeyStore) null);
for (TrustManager trustManager :
tmf.getTrustManagers()) {
((X509TrustManager)
trustManager).checkServerTrusted(chain, authType);
}
} catch (Exception e) {
throw new CertificateException(e);
}
RSAPublicKey pubkey = (RSAPublicKey)
chain[0].getPublicKey();
String encoded = new BigInteger(1 ,
pubkey.getEncoded()).toString(16);
final boolean expected =
PUB_KEY.equalsIgnoreCase(encoded);
if (!expected) {
throw new
CertificateException("checkServerTrusted: Expected public key: "
+ PUB_KEY + ", got public key:"
+ encoded);
}
}
};
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
