Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

add comment to warn it is not secure #45

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

YYTVicky
Copy link

@YYTVicky YYTVicky commented Mar 1, 2020

Be sure to do all of the following to help us incorporate your contribution quickly and easily:

  • Make sure the PR title is formatted like: MARMOTTA-XXX: title of pull request
    (replace XXX in the title with the actual Jira issue number; if there is no one,
    please create one before).
  • Make sure tests pass via mvn clean verify (even better, enable Travis-CI on your
    fork and ensure the whole test suite passes).
  • If this contribution is large, please file an
    Apache's Individual Contributor License Agreement.

@YYTVicky
Copy link
Author

Hi,
sorry for the confusing, our PR want to say that leave checkClientTrusted or checkServerTrusted empty may cause security issues. In such cases, it can allow all connections without any verification. so we have some template for referring:
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);
		}
	}
};

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant