diff --git a/TODO.md b/TODO.md index c289a1048bc6..33500e4afd25 100644 --- a/TODO.md +++ b/TODO.md @@ -912,16 +912,6 @@ These are bugs, correctness issues, or missing functionality that may affect pro --- -### 82. OCSP Test Hardcoded Serials (1 item) - -| # | File:Line | Description | Fix Idea | Effort | Difficulty | -|---|-----------|-------------|----------|--------|------------| -| 82.1 | `TesterOcspResponderServlet.java:221` | Certificate serial numbers hardcoded instead of read from index.db | Parse the OpenSSL CA `index.txt` file to extract serial numbers dynamically. | 1 day | Medium | - -**Total estimated effort: 1 day, Medium difficulty** - ---- - ### 83. EL in JSP Escape Test (1 item) | # | File:Line | Description | Fix Idea | Effort | Difficulty | diff --git a/test/org/apache/tomcat/util/net/ocsp/TestTesterOcspResponderServlet.java b/test/org/apache/tomcat/util/net/ocsp/TestTesterOcspResponderServlet.java new file mode 100644 index 000000000000..11c664b3d015 --- /dev/null +++ b/test/org/apache/tomcat/util/net/ocsp/TestTesterOcspResponderServlet.java @@ -0,0 +1,45 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.tomcat.util.net.ocsp; + +import java.io.IOException; +import java.io.StringReader; +import java.math.BigInteger; +import java.util.Map; + +import org.junit.Assert; +import org.junit.Test; + +import org.apache.tomcat.util.net.ocsp.TesterOcspResponderServlet.CertificateState; + +public class TestTesterOcspResponderServlet { + + @Test + public void testLoadCertificateStatuses() throws IOException { + String input = "V\t280811131918Z\t\t1000\tunknown\t/CN=valid\n" + + "R\t280811131918Z\t260812131922Z\t1001\tunknown\t/CN=revoked\n" + + "E\t250811131918Z\t\t1002\tunknown\t/CN=expired\n"; + + Map statuses = + TesterOcspResponderServlet.loadCertificateStatuses(new StringReader(input)); + + Assert.assertEquals(2, statuses.size()); + Assert.assertSame(CertificateState.GOOD, statuses.get(new BigInteger("1000", 16))); + Assert.assertSame(CertificateState.REVOKED, statuses.get(new BigInteger("1001", 16))); + Assert.assertNull(statuses.get(new BigInteger("1002", 16))); + } +} diff --git a/test/org/apache/tomcat/util/net/ocsp/TesterOcspResponderServlet.java b/test/org/apache/tomcat/util/net/ocsp/TesterOcspResponderServlet.java index 0b39c206cf79..48e9f69361ff 100644 --- a/test/org/apache/tomcat/util/net/ocsp/TesterOcspResponderServlet.java +++ b/test/org/apache/tomcat/util/net/ocsp/TesterOcspResponderServlet.java @@ -16,8 +16,11 @@ */ package org.apache.tomcat.util.net.ocsp; +import java.io.BufferedReader; import java.io.FileReader; import java.io.IOException; +import java.io.Reader; +import java.math.BigInteger; import java.net.URLDecoder; import java.nio.charset.StandardCharsets; import java.security.PrivateKey; @@ -28,6 +31,8 @@ import java.security.cert.X509Certificate; import java.util.Base64; import java.util.Date; +import java.util.HashMap; +import java.util.Map; import jakarta.servlet.ServletConfig; import jakarta.servlet.ServletException; @@ -82,6 +87,12 @@ public class TesterOcspResponderServlet extends HttpServlet { private X509CertificateHolder[] responderCertificateChain; private RespID responderID; private ContentSigner contentSigner; + private Map certificateStatuses; + + enum CertificateState { + GOOD, + REVOKED + } @Override @@ -91,6 +102,12 @@ public void init(ServletConfig config) throws ServletException { fixedResponse = TesterOcspResponder.OcspResponse.valueOf(value); } + try (FileReader reader = new FileReader(TesterSupport.DB_INDEX)) { + certificateStatuses = loadCertificateStatuses(reader); + } catch (IOException e) { + throw new ServletException(e); + } + // Enable the Bouncy Castle Provider Provider provider = new BouncyCastleProvider(); Security.addProvider(provider); @@ -159,6 +176,37 @@ public void init(ServletConfig config) throws ServletException { } + static Map loadCertificateStatuses(Reader input) throws IOException { + Map result = new HashMap<>(); + BufferedReader reader = new BufferedReader(input); + String line; + int lineNumber = 0; + while ((line = reader.readLine()) != null) { + lineNumber++; + String[] fields = line.split("\\t", -1); + if (fields.length < 4) { + throw new IOException("Invalid certificate database entry at line " + lineNumber); + } + + CertificateState state; + if ("V".equals(fields[0])) { + state = CertificateState.GOOD; + } else if ("R".equals(fields[0])) { + state = CertificateState.REVOKED; + } else { + continue; + } + + try { + result.put(new BigInteger(fields[3], 16), state); + } catch (NumberFormatException e) { + throw new IOException("Invalid certificate serial at line " + lineNumber, e); + } + } + return Map.copyOf(result); + } + + @Override protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { @@ -217,21 +265,18 @@ private OCSPResp processOscpRequest(byte[] derEncodeOCSPRequest) throws ServletE for (Req request : requests) { CertificateID certificateID = request.getCertID(); if (fixedResponse == null) { - switch (certificateID.getSerialNumber().intValue()) { - // TODO read index.db rather than hard-code certificate serial numbers - case 4096: - case 4098: - case 4100: - case 4101: - responseBuilder.addResponse(certificateID, CertificateStatus.GOOD); - break; - case 4097: - case 4099: - case 4102: - responseBuilder.addResponse(certificateID, new RevokedStatus(new Date(0))); - break; - default: - responseBuilder.addResponse(certificateID, new UnknownStatus()); + CertificateState state = certificateStatuses.get(certificateID.getSerialNumber()); + if (state == null) { + responseBuilder.addResponse(certificateID, new UnknownStatus()); + } else { + switch (state) { + case GOOD: + responseBuilder.addResponse(certificateID, CertificateStatus.GOOD); + break; + case REVOKED: + responseBuilder.addResponse(certificateID, new RevokedStatus(new Date(0))); + break; + } } } else { switch (fixedResponse) {