Skip to content
Permalink
Browse files

Improved: Extract verification of certificates in ‘RequestHandler’

(OFBIZ-10450)

No functional change. Reducte the size of RequestHandler::doRequest method.
Add somes unit tests.
Thanks Mathieu

git-svn-id: https://svn.apache.org/repos/asf/ofbiz/ofbiz-framework/trunk@1850250 13f79535-47bb-0310-9956-ffa450edef68
  • Loading branch information
gilPts committed Jan 3, 2019
1 parent 61ebf7a commit 1b02bfa7aa0b2401aa4e3de7eaecd6c56828422f
@@ -31,7 +31,10 @@
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.function.Predicate;
import java.util.stream.Stream;

import javax.servlet.ServletContext;
import javax.servlet.http.HttpServletRequest;
@@ -372,33 +375,7 @@ public void doRequest(HttpServletRequest request, HttpServletResponse response,

// Check for HTTPS client (x.509) security
if (request.isSecure() && requestMap.securityCert) {
X509Certificate[] clientCerts = (X509Certificate[]) request.getAttribute("javax.servlet.request.X509Certificate"); // 2.2 spec
if (clientCerts == null) {
clientCerts = (X509Certificate[]) request.getAttribute("javax.net.ssl.peer_certificates"); // 2.1 spec
}
if (clientCerts == null) {
Debug.logWarning("Received no client certificates from browser", module);
}

// check if the client has a valid certificate (in our db store)
boolean foundTrustedCert = false;

if (clientCerts == null) {
throw new RequestHandlerException(requestMissingErrorMessage);
} else {
if (Debug.infoOn()) {
for (int i = 0; i < clientCerts.length; i++) {
Debug.logInfo(clientCerts[i].getSubjectX500Principal().getName(), module);
}
}

// check if this is a trusted cert
if (SSLUtil.isClientTrusted(clientCerts, null)) {
foundTrustedCert = true;
}
}

if (!foundTrustedCert) {
if (!checkCertificates(request, certs -> SSLUtil.isClientTrusted(certs, null))) {
Debug.logWarning(requestMissingErrorMessage, module);
throw new RequestHandlerException(requestMissingErrorMessage);
}
@@ -1322,4 +1299,31 @@ private String showSessionId(HttpServletRequest request) {
}
return " Hidden sessionId by default.";
}

/**
* Checks that the request contains some valid certificates.
*
* @param request the request to verify
* @param validator the predicate applied the certificates found
* @return true if the request contains some valid certificates, otherwise false.
*/
static boolean checkCertificates(HttpServletRequest request, Predicate<X509Certificate[]> validator) {
return Stream.of("javax.servlet.request.X509Certificate", // 2.2 spec
"javax.net.ssl.peer_certificates") // 2.1 spec
.map(request::getAttribute)
.filter(Objects::nonNull)
.map(X509Certificate[].class::cast)
.peek(certs -> {
if (Debug.infoOn()) {
for (X509Certificate cert : certs) {
Debug.logInfo(cert.getSubjectX500Principal().getName(), module);
}
}
})
.map(validator::test)
.findFirst().orElseGet(() -> {
Debug.logWarning("Received no client certificates from browser", module);
return false;
});
}
}
@@ -25,9 +25,12 @@
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import java.security.cert.CertificateEncodingException;
import java.security.cert.X509Certificate;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
@@ -197,4 +200,43 @@ public void resolveMethodCatchAll() throws RequestHandlerException {
assertTrue(RequestHandler.resolveMethod("delete", rmaps).isPresent());
}
}

public static class checkCertificatesTests {
private HttpServletRequest req;

@Before
public void setUp() {
req = mock(HttpServletRequest.class);
when(req.getAttribute(anyString())).thenReturn(null);
}

@Test
// Check that the verification fails when the request does not contain any certificate.
public void checkCertificatesFailure() {
assertFalse(RequestHandler.checkCertificates(req, x -> true));
}

@Test
// Check that certificates with 2.2 spec are handled correctly.
public void checkCertificates22() throws CertificateEncodingException {
when(req.getAttribute("javax.servlet.request.X509Certificate")).thenReturn(new X509Certificate[] {});
assertTrue(RequestHandler.checkCertificates(req, x -> true));
assertFalse(RequestHandler.checkCertificates(req, x -> false));
}

@Test
// Check that certificates with 2.1 spec are handled correctly.
public void checkCertificates21() {
when(req.getAttribute("javax.net.ssl.peer_certificates")).thenReturn(new X509Certificate[] {});
assertTrue(RequestHandler.checkCertificates(req, x -> true));
assertFalse(RequestHandler.checkCertificates(req, x -> false));
}

@Test
// Check that certificates in an invalid attribute are ignored.
public void checkCertificatesUnrecognized() {
when(req.getAttribute("NOT_RECOGNIZED")).thenReturn(new X509Certificate[] {});
assertFalse(RequestHandler.checkCertificates(req, x -> true));
}
}
}

0 comments on commit 1b02bfa

Please sign in to comment.
You can’t perform that action at this time.