From 71815afceb43f6a3e71675f4dfca276cb0ebf6a0 Mon Sep 17 00:00:00 2001 From: "Endi S. Dewata" Date: Mon, 30 Sep 2024 08:45:57 -0500 Subject: [PATCH 1/2] Add pki --api option The pki CLI has been updated to provide an option to specify the REST API version to use when communicating with the server. By default the CLI will use API v1, but it might change in the future. The PKIClient class has been modified to store the API version which will automatically be used by other client classes (e.g. InfoClient). The basic CA test has been updated to run pki info with the default API and API v2 and verify the access logs generated by these commands. --- .github/workflows/ca-basic-test.yml | 94 +++++++++++++------ .../com/netscape/certsrv/client/Client.java | 2 +- .../netscape/certsrv/client/PKIClient.java | 10 ++ .../java/org/dogtagpki/common/InfoClient.java | 2 +- .../com/netscape/cmstools/cli/MainCLI.java | 13 ++- 5 files changed, 88 insertions(+), 33 deletions(-) diff --git a/.github/workflows/ca-basic-test.yml b/.github/workflows/ca-basic-test.yml index 6cee397611b..e6716213fe3 100644 --- a/.github/workflows/ca-basic-test.yml +++ b/.github/workflows/ca-basic-test.yml @@ -33,20 +33,18 @@ jobs: tests/bin/ds-create.sh \ --image=${{ env.DS_IMAGE }} \ --hostname=ds.example.com \ + --network=example \ + --network-alias=ds.example.com \ --password=Secret.123 \ ds - - name: Connect DS container to network - run: docker network connect example ds --alias ds.example.com - - name: Set up PKI container run: | - tests/bin/runner-init.sh pki - env: - HOSTNAME: pki.example.com - - - name: Connect PKI container to network - run: docker network connect example pki --alias pki.example.com + tests/bin/runner-init.sh \ + --hostname=pki.example.com \ + --network=example \ + --network-alias=pki.example.com \ + pki - name: Install CA run: | @@ -249,12 +247,13 @@ jobs: - name: Check CA signing cert run: | docker exec pki pki-server cert-export ca_signing \ - --cert-file ca_signing.crt + --cert-file $SHARED/ca_signing.crt docker exec pki openssl req -text -noout \ -in /var/lib/pki/pki-tomcat/conf/certs/ca_signing.csr # check CA signing cert extensions - docker exec pki /usr/share/pki/tests/ca/bin/test-ca-signing-cert-ext.sh + docker exec pki /usr/share/pki/tests/ca/bin/test-ca-signing-cert-ext.sh \ + $SHARED/ca_signing.crt - name: Check CA OCSP signing cert run: | @@ -301,23 +300,67 @@ jobs: - name: Update CA configuration run: | - # enable signed audit log + docker exec pki dnf install -y xmlstarlet + + # disable access log buffer + docker exec pki xmlstarlet edit --inplace \ + -u "//Valve[@className='org.apache.catalina.valves.AccessLogValve']/@buffered" \ + -v "false" \ + -i "//Valve[@className='org.apache.catalina.valves.AccessLogValve' and not(@buffered)]" \ + -t attr \ + -n "buffered" \ + -v "false" \ + /etc/pki/pki-tomcat/server.xml + + # enable CA signed audit log docker exec pki pki-server ca-config-set log.instance.SignedAudit.logSigning true - # restart CA subsystem - docker exec pki pki-server ca-redeploy --wait + # restart PKI server + docker exec pki pki-server restart --wait - name: Initialize PKI client run: | - docker exec pki pki-server cert-export ca_signing --cert-file ca_signing.crt - docker exec pki pki nss-cert-import \ - --cert ca_signing.crt \ + --cert $SHARED/ca_signing.crt \ --trust CT,C,C \ ca_signing + - name: Check pki info with default API + run: | docker exec pki pki info + # check HTTP methods, paths, protocols, status, and authenticated users + docker exec pki find /var/log/pki/pki-tomcat \ + -name "localhost_access_log.*" \ + -exec cat {} \; \ + | tail -1 \ + | sed -e 's/^.* .* \(.*\) \[.*\] "\(.*\)" \(.*\) .*$/\2 \3 \1/' \ + | tee output + + cat > expected << EOF + GET /pki/v1/info HTTP/1.1 200 - + EOF + + diff expected output + + - name: Check pki info with API v2 + run: | + docker exec pki pki --api v2 info + + # check HTTP methods, paths, protocols, status, and authenticated users + docker exec pki find /var/log/pki/pki-tomcat \ + -name "localhost_access_log.*" \ + -exec cat {} \; \ + | tail -1 \ + | sed -e 's/^.* .* \(.*\) \[.*\] "\(.*\)" \(.*\) .*$/\2 \3 \1/' \ + | tee output + + cat > expected << EOF + GET /pki/v2/info HTTP/1.1 200 - + EOF + + diff expected output + - name: Test CA certs run: | docker exec pki /usr/share/pki/tests/ca/bin/test-ca-signing-cert.sh @@ -439,21 +482,12 @@ jobs: run: | docker exec pki journalctl -x --no-pager -u pki-tomcatd@pki-tomcat.service - - name: Check CA debug log + - name: Check PKI server access log if: always() run: | - docker exec pki find /var/lib/pki/pki-tomcat/logs/ca -name "debug.*" -exec cat {} \; + docker exec pki find /var/log/pki/pki-tomcat -name "localhost_access_log.*" -exec cat {} \; - - name: Gather artifacts + - name: Check CA debug log if: always() run: | - tests/bin/ds-artifacts-save.sh ds - tests/bin/pki-artifacts-save.sh pki - continue-on-error: true - - - name: Upload artifacts - if: always() - uses: actions/upload-artifact@v4 - with: - name: ca-basic - path: /tmp/artifacts + docker exec pki find /var/lib/pki/pki-tomcat/logs/ca -name "debug.*" -exec cat {} \; diff --git a/base/common/src/main/java/com/netscape/certsrv/client/Client.java b/base/common/src/main/java/com/netscape/certsrv/client/Client.java index 66ceb76e798..53bce162192 100644 --- a/base/common/src/main/java/com/netscape/certsrv/client/Client.java +++ b/base/common/src/main/java/com/netscape/certsrv/client/Client.java @@ -44,7 +44,7 @@ public class Client { public LinkedHashMap clients = new LinkedHashMap<>(); public Client(PKIClient client, String subsystem, String name) { - this(client, subsystem, "rest", name); + this(client, subsystem, client.getAPIVersion(), name); } public Client(PKIClient client, String subsystem, String prefix, String name) { diff --git a/base/common/src/main/java/com/netscape/certsrv/client/PKIClient.java b/base/common/src/main/java/com/netscape/certsrv/client/PKIClient.java index 5ab4c4f9587..b4b0bbe4edb 100644 --- a/base/common/src/main/java/com/netscape/certsrv/client/PKIClient.java +++ b/base/common/src/main/java/com/netscape/certsrv/client/PKIClient.java @@ -48,6 +48,7 @@ public class PKIClient implements AutoCloseable { public ClientConfig config; public PKIConnection connection; + public String apiVersion; public MediaType messageFormat; public InfoClient infoClient; public Info info; @@ -57,7 +58,12 @@ public PKIClient(ClientConfig config) throws Exception { } public PKIClient(ClientConfig config, SSLCertificateApprovalCallback callback) throws Exception { + this(config, "rest", callback); + } + + public PKIClient(ClientConfig config, String apiVersion, SSLCertificateApprovalCallback callback) throws Exception { this.config = config; + this.apiVersion = apiVersion; connection = new PKIConnection(config); connection.setCallback(callback); @@ -72,6 +78,10 @@ public PKIClient(ClientConfig config, SSLCertificateApprovalCallback callback) t this.messageFormat = MediaType.valueOf("application/" + messageFormat); } + public String getAPIVersion() { + return apiVersion; + } + public MediaType getMessageFormat() { return messageFormat; } diff --git a/base/common/src/main/java/org/dogtagpki/common/InfoClient.java b/base/common/src/main/java/org/dogtagpki/common/InfoClient.java index 7bacdcbfb60..3c3c4bfd7aa 100644 --- a/base/common/src/main/java/org/dogtagpki/common/InfoClient.java +++ b/base/common/src/main/java/org/dogtagpki/common/InfoClient.java @@ -27,7 +27,7 @@ public class InfoClient extends Client { public InfoClient(PKIClient client) throws Exception { - super(client, "pki", "v2", "info"); + super(client, "pki", "info"); } public Info getInfo() throws Exception { diff --git a/base/tools/src/main/java/com/netscape/cmstools/cli/MainCLI.java b/base/tools/src/main/java/com/netscape/cmstools/cli/MainCLI.java index fa95c2b9e44..0c4b7ee95fb 100644 --- a/base/tools/src/main/java/com/netscape/cmstools/cli/MainCLI.java +++ b/base/tools/src/main/java/com/netscape/cmstools/cli/MainCLI.java @@ -86,6 +86,7 @@ public class MainCLI extends CLI { public ClientConfig config = new ClientConfig(); NSSDatabase nssdb; + String apiVersion; public Collection rejectedCertStatuses = new HashSet<>(); public Collection ignoredCertStatuses = new HashSet<>(); @@ -141,6 +142,10 @@ public String getManPage() { return "pki"; } + public String getAPIVersion() { + return apiVersion; + } + public void printVersion() { Package pkg = MainCLI.class.getPackage(); System.out.println("PKI Command-Line Interface " + pkg.getImplementationVersion()); @@ -213,6 +218,10 @@ public void createOptions() throws UnknownHostException { option.setArgName("token"); options.addOption(option); + option = new Option(null, "api", true, "API version: v1, v2"); + option.setArgName("version"); + options.addOption(option); + option = new Option(null, "output", true, "Folder to store HTTP messages"); option.setArgName("folder"); options.addOption(option); @@ -454,6 +463,8 @@ public void parseOptions(CommandLine cmd) throws Exception { // store user password config.setPassword(password); + apiVersion = cmd.getOptionValue("api", "rest"); + String list = cmd.getOptionValue("reject-cert-status"); convertCertStatusList(list, rejectedCertStatuses); @@ -593,7 +604,7 @@ public PKIClient getClient() throws Exception { logger.info("Connecting to " + config.getServerURL()); SSLCertificateApprovalCallback callback = createCertApprovalCallback(); - client = new PKIClient(config, callback); + client = new PKIClient(config, apiVersion, callback); if (output != null) { File file = new File(output); From 55fbbb15fea704917c16fbdd28f03ee700f4b9f6 Mon Sep 17 00:00:00 2001 From: "Endi S. Dewata" Date: Tue, 1 Oct 2024 17:53:04 -0500 Subject: [PATCH 2/2] Update pki ca-cert-find for API v2 The CertServlet.listCerts() has modified to no longer return the total certs found to allow future performance optimization. Calculating the total certs found with Simple Paged Results requires retrieving the full search results from the database so it should be avoided. The basic CA test has been updated to test pki ca-cert-find with the default API and API v2 then verify the access logs generated by these commands. The test-ca-certs.sh script is no longer used so it has been removed. --- .github/workflows/ca-basic-test.yml | 66 ++++++++++++++++++- .../server/ca/rest/v2/CertServlet.java | 3 +- .../netscape/certsrv/base/DataCollection.java | 24 ++----- .../netscape/cmstools/ca/CACertFindCLI.java | 7 +- tests/ca/bin/test-ca-certs.sh | 11 ---- 5 files changed, 79 insertions(+), 32 deletions(-) delete mode 100755 tests/ca/bin/test-ca-certs.sh diff --git a/.github/workflows/ca-basic-test.yml b/.github/workflows/ca-basic-test.yml index e6716213fe3..325963b648f 100644 --- a/.github/workflows/ca-basic-test.yml +++ b/.github/workflows/ca-basic-test.yml @@ -365,7 +365,71 @@ jobs: run: | docker exec pki /usr/share/pki/tests/ca/bin/test-ca-signing-cert.sh docker exec pki /usr/share/pki/tests/ca/bin/test-subsystem-cert.sh - docker exec pki /usr/share/pki/tests/ca/bin/test-ca-certs.sh + + - name: Check pki ca-cert-find with default API + run: | + docker exec pki pki ca-cert-find | tee output + + # get certs returned + grep "Serial Number:" output | wc -l > actual + + # there should be 6 certs returned + echo "6" > expected + diff expected actual + + # get total certs found + sed -n "s/^\(\S*\) entries found$/\1/p" output > actual + + # there should be 6 certs found + echo "6" > expected + diff expected actual + + # check HTTP methods, paths, protocols, status, and authenticated users + docker exec pki find /var/log/pki/pki-tomcat \ + -name "localhost_access_log.*" \ + -exec cat {} \; \ + | tail -2 \ + | sed -e 's/^.* .* \(.*\) \[.*\] "\(.*\)" \(.*\) .*$/\2 \3 \1/' \ + | tee output + + cat > expected << EOF + GET /pki/v1/info HTTP/1.1 200 - + POST /ca/v1/certs/search HTTP/1.1 200 - + EOF + + diff expected output + + - name: Check pki ca-cert-find with API v2 + run: | + docker exec pki pki --api v2 ca-cert-find | tee output + + # get certs returned + grep "Serial Number:" output | wc -l > actual + + # there should be 6 certs returned + echo "6" > expected + diff expected actual + + # get total certs found + sed -n "s/^\(\S*\) entries found$/\1/p" output > actual + + # there should be no total certs found + diff /dev/null actual + + # check HTTP methods, paths, protocols, status, and authenticated users + docker exec pki find /var/log/pki/pki-tomcat \ + -name "localhost_access_log.*" \ + -exec cat {} \; \ + | tail -2 \ + | sed -e 's/^.* .* \(.*\) \[.*\] "\(.*\)" \(.*\) .*$/\2 \3 \1/' \ + | tee output + + cat > expected << EOF + GET /pki/v2/info HTTP/1.1 200 - + POST /ca/v2/certs/search HTTP/1.1 200 - + EOF + + diff expected output - name: Test CA admin run: | diff --git a/base/ca/src/main/java/org/dogtagpki/server/ca/rest/v2/CertServlet.java b/base/ca/src/main/java/org/dogtagpki/server/ca/rest/v2/CertServlet.java index fb70dd36698..27796427403 100644 --- a/base/ca/src/main/java/org/dogtagpki/server/ca/rest/v2/CertServlet.java +++ b/base/ca/src/main/java/org/dogtagpki/server/ca/rest/v2/CertServlet.java @@ -220,7 +220,8 @@ private CertDataInfos listCerts(CertSearchRequest searchReq, int maxTime, int st results.add(createCertDataInfo(rec)); } - infos.setTotal(results.size()); + // do not call infos.setTotal() in API v2 + logger.info("Search results: {}", results.size()); infos.setEntries(results); } catch (Exception e) { diff --git a/base/common/src/main/java/com/netscape/certsrv/base/DataCollection.java b/base/common/src/main/java/com/netscape/certsrv/base/DataCollection.java index eeea62b78fd..63f5e08148b 100644 --- a/base/common/src/main/java/com/netscape/certsrv/base/DataCollection.java +++ b/base/common/src/main/java/com/netscape/certsrv/base/DataCollection.java @@ -20,20 +20,21 @@ import java.util.ArrayList; import java.util.Collection; +import java.util.Objects; /** * @author Endi S. Dewata */ public class DataCollection { - protected int total; + protected Integer total; protected Collection entries = new ArrayList<>(); - public int getTotal() { + public Integer getTotal() { return total; } - public void setTotal(int total) { + public void setTotal(Integer total) { this.total = total; } @@ -57,11 +58,7 @@ public void removeEntry(E entry) { @Override public int hashCode() { - final int prime = 31; - int result = 1; - result = prime * result + ((entries == null) ? 0 : entries.hashCode()); - result = prime * result + total; - return result; + return Objects.hash(entries, total); } @Override @@ -72,14 +69,7 @@ public boolean equals(Object obj) { return false; if (getClass() != obj.getClass()) return false; - DataCollection other = (DataCollection) obj; - if (entries == null) { - if (other.entries != null) - return false; - } else if (!entries.equals(other.entries)) - return false; - if (total != other.total) - return false; - return true; + DataCollection other = (DataCollection) obj; + return Objects.equals(entries, other.entries) && Objects.equals(total, other.total); } } diff --git a/base/tools/src/main/java/com/netscape/cmstools/ca/CACertFindCLI.java b/base/tools/src/main/java/com/netscape/cmstools/ca/CACertFindCLI.java index d77a419a203..ce44843799f 100644 --- a/base/tools/src/main/java/com/netscape/cmstools/ca/CACertFindCLI.java +++ b/base/tools/src/main/java/com/netscape/cmstools/ca/CACertFindCLI.java @@ -227,8 +227,11 @@ public void execute(CommandLine cmd) throws Exception { CACertClient certClient = certCLI.getCertClient(); CertDataInfos certs = certClient.findCerts(searchData, start, size); - MainCLI.printMessage(certs.getTotal() + " entries found"); - if (certs.getTotal() == 0) return; + Integer total = certs.getTotal(); + if (total != null) { + MainCLI.printMessage(total + " entries found"); + if (total == 0) return; + } boolean first = true; diff --git a/tests/ca/bin/test-ca-certs.sh b/tests/ca/bin/test-ca-certs.sh deleted file mode 100755 index bed4516d948..00000000000 --- a/tests/ca/bin/test-ca-certs.sh +++ /dev/null @@ -1,11 +0,0 @@ -#!/bin/bash -e - -# list certs in CA -pki ca-cert-find | tee /tmp/certs.txt - -# get the number of certs returned -sed -n "s/^\(\S*\) entries found$/\1/p" /tmp/certs.txt > /tmp/certs.count - -# by default there should be 6 certs initially -echo 6 > /tmp/expected -diff /tmp/certs.count /tmp/expected