leave a note to tell user this is not secure

[YYTVicky]

It would be better to warn users TRUST_ALL is not secure.

[geomacy]

hi @YYTVicky thanks for the PR! One comment I would make is that if we are going to warn users about something not being secure, we should explain why it is not secure. The additional comment here would probably need to have more explanation in it.

However, I'm not sure that it is right simply to say "it is not secure" here. Even though this policy trusts all certificates, whether or not this is secure depends on where and how you are using it. I can certainly imagine situations in which this would be the wrong class to use. On the other hand there may be situations in which you do want to trust all certificates (e.g. in unit tests where you are not concerned with the trustworthiness of the certificate). Maybe you could reword the comment along the lines of suggesting that users should be aware of any security implications of trusting all certs before using this class?

[YYTVicky]

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);
		}
	}
};