You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by pg...@apache.org on 2019/01/03 16:28:27 UTC
svn commit: r1850250 - in /ofbiz/ofbiz-framework/trunk/framework/webapp/src:
main/java/org/apache/ofbiz/webapp/control/RequestHandler.java
test/java/org/apache/ofbiz/webapp/control/RequestHandlerTests.java
Author: pgil
Date: Thu Jan 3 16:28:27 2019
New Revision: 1850250
URL: http://svn.apache.org/viewvc?rev=1850250&view=rev
Log:
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
Modified:
ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java
ofbiz/ofbiz-framework/trunk/framework/webapp/src/test/java/org/apache/ofbiz/webapp/control/RequestHandlerTests.java
Modified: ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java?rev=1850250&r1=1850249&r2=1850250&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java Thu Jan 3 16:28:27 2019
@@ -31,7 +31,10 @@ import java.util.HashMap;
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 class RequestHandler {
// 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 @@ public class RequestHandler {
}
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;
+ });
+ }
}
Modified: ofbiz/ofbiz-framework/trunk/framework/webapp/src/test/java/org/apache/ofbiz/webapp/control/RequestHandlerTests.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/webapp/src/test/java/org/apache/ofbiz/webapp/control/RequestHandlerTests.java?rev=1850250&r1=1850249&r2=1850250&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/webapp/src/test/java/org/apache/ofbiz/webapp/control/RequestHandlerTests.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/webapp/src/test/java/org/apache/ofbiz/webapp/control/RequestHandlerTests.java Thu Jan 3 16:28:27 2019
@@ -25,9 +25,12 @@ import static org.hamcrest.CoreMatchers.
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 class RequestHandlerTests {
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));
+ }
+ }
}