Slair1 commented on a change in pull request #4852:
URL: https://github.com/apache/cloudstack/pull/4852#discussion_r600521628
##########
File path:
plugins/ca/root-ca/src/main/java/org/apache/cloudstack/ca/provider/RootCACustomTrustManager.java
##########
@@ -79,57 +79,58 @@ public void checkClientTrusted(final X509Certificate[]
certificates, final Strin
if (LOG.isDebugEnabled()) {
printCertificateChain(certificates, s);
}
- if (!authStrictness) {
- return;
- }
- if (certificates == null || certificates.length < 1 || certificates[0]
== null) {
- throw new CertificateException("In strict auth mode,
certificate(s) are expected from client:" + clientAddress);
- }
- final X509Certificate primaryClientCertificate = certificates[0];
-
- // Revocation check
- final BigInteger serialNumber =
primaryClientCertificate.getSerialNumber();
- if (serialNumber == null || crlDao.findBySerial(serialNumber) != null)
{
- final String errorMsg = String.format("Client is using revoked
certificate of serial=%x, subject=%s from address=%s",
- primaryClientCertificate.getSerialNumber(),
primaryClientCertificate.getSubjectDN(), clientAddress);
- LOG.error(errorMsg);
- throw new CertificateException(errorMsg);
- }
- // Validity check
- if (!allowExpiredCertificate) {
- try {
- primaryClientCertificate.checkValidity();
- } catch (final CertificateExpiredException |
CertificateNotYetValidException e) {
- final String errorMsg = String.format("Client certificate has
expired with serial=%x, subject=%s from address=%s",
+ final X509Certificate primaryClientCertificate = (certificates != null
&& certificates.length > 0 && certificates[0] != null) ? certificates[0] : null;
+
+ if (authStrictness) {
+ if (primaryClientCertificate == null) {
+ throw new CertificateException("In strict auth mode,
certificate(s) are expected from client:" + clientAddress);
+ }
+
+ // Revocation check
+ final BigInteger serialNumber =
primaryClientCertificate.getSerialNumber();
+ if (serialNumber == null || crlDao.findBySerial(serialNumber) !=
null) {
+ final String errorMsg = String.format("Client is using revoked
certificate of serial=%x, subject=%s from address=%s",
primaryClientCertificate.getSerialNumber(),
primaryClientCertificate.getSubjectDN(), clientAddress);
LOG.error(errorMsg);
- throw new CertificateException(errorMsg); }
- }
+ throw new CertificateException(errorMsg);
+ }
+
+ // Validity check
+ if (!allowExpiredCertificate) {
+ try {
+ primaryClientCertificate.checkValidity();
+ } catch (final CertificateExpiredException |
CertificateNotYetValidException e) {
+ final String errorMsg = String.format("Client certificate
has expired with serial=%x, subject=%s from address=%s",
+ primaryClientCertificate.getSerialNumber(),
primaryClientCertificate.getSubjectDN(), clientAddress);
+ LOG.error(errorMsg);
+ throw new CertificateException(errorMsg); }
+ }
- // Ownership check
- boolean certMatchesOwnership = false;
- if (primaryClientCertificate.getSubjectAlternativeNames() != null) {
- for (final List<?> list :
primaryClientCertificate.getSubjectAlternativeNames()) {
- if (list != null && list.size() == 2 && list.get(1) instanceof
String) {
- final String alternativeName = (String) list.get(1);
- if (clientAddress.equals(alternativeName)) {
- certMatchesOwnership = true;
+ // Ownership check
+ boolean certMatchesOwnership = false;
+ if (primaryClientCertificate.getSubjectAlternativeNames() != null)
{
+ for (final List<?> list :
primaryClientCertificate.getSubjectAlternativeNames()) {
+ if (list != null && list.size() == 2 && list.get(1)
instanceof String) {
+ final String alternativeName = (String) list.get(1);
+ if (clientAddress.equals(alternativeName)) {
+ certMatchesOwnership = true;
+ }
}
}
}
+ if (!certMatchesOwnership) {
+ final String errorMsg = "Certificate ownership verification
failed for client: " + clientAddress;
+ LOG.error(errorMsg);
+ throw new CertificateException(errorMsg);
+ }
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Client/agent connection from ip=" + clientAddress +
" has been validated and trusted.");
+ }
}
- if (!certMatchesOwnership) {
- final String errorMsg = "Certificate ownership verification failed
for client: " + clientAddress;
- LOG.error(errorMsg);
- throw new CertificateException(errorMsg);
- }
- if (activeCertMap != null && !Strings.isNullOrEmpty(clientAddress)) {
+ if (primaryClientCertificate != null && activeCertMap != null &&
!Strings.isNullOrEmpty(clientAddress)) {
Review comment:
@sureshanaparti the agent handler code on the mgmt server already takes
care of removing a cert from the memory map if it existed when the agent
disconnects (which would have happened if this code ran at an agent's
reconnect). Also for what it's worth, this part of the functionality is
unchanged from the current version of the code.
--
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]
