Skip to content

Commit

Permalink
Remove default cert approval callback in PKIClient
Browse files Browse the repository at this point in the history
Previously the PKIClient class had a default cert approval
callback which would only warn the user if it receives a cert
with a BAD_CERT_DOMAIN but still allow it, or ask the user
whether to trust an UNTRUSTED_ISSUER.

On the client side (e.g. CLI, console) this is fine since the
user is actively interacting with the application, but on the
server side (e.g. authenticators) there are no users constantly
monitoring the logs so the cert verification needs to be more
stringent.

To resolve the issue, the default cert approval callback in
PKIClient has been removed such that certs with BAD_CERT_DOMAIN
or UNTRUSTED_ISSUER will automatically be rejected. On the server
side PKIClient will be used without a cert approval callback. On
the client side it will be used with an interactive callback.

Previously some of ACME tests were using the default issuer URL
which contains localhost.localdomain hostname so it actually
generated BAD_CERT_DOMAIN errors. They have been updated to use
the proper CA hostname.
  • Loading branch information
edewata committed Jul 24, 2024
1 parent eb86172 commit f08c4bc
Show file tree
Hide file tree
Showing 6 changed files with 129 additions and 42 deletions.
50 changes: 41 additions & 9 deletions .github/workflows/acme-certbot-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,9 @@ jobs:
docker exec pki pki-server acme-database-mod \
--type ds \
-D url=ldap://ds.example.com:3389
docker exec pki pki-server acme-issuer-mod --type pki
docker exec pki pki-server acme-issuer-mod \
--type pki \
-D url=https://pki.example.com:8443
docker exec pki pki-server acme-realm-mod \
--type ds \
-D url=ldap://ds.example.com:3389
Expand Down Expand Up @@ -552,6 +554,44 @@ jobs:
sed -n 's/^acmeStatus: *\(.*\)$/\1/p' output > actual
diff expected actual
- name: Remove ACME from PKI container
run: |
docker exec pki pki-server acme-undeploy --wait
docker exec pki pki-server acme-remove
- name: Remove CA from PKI container
run: docker exec pki pkidestroy -i pki-tomcat -s CA -v

- name: Check DS server systemd journal
if: always()
run: |
docker exec ds journalctl -x --no-pager -u [email protected]
- name: Check DS container logs
if: always()
run: |
docker logs ds
- name: Check PKI server systemd journal
if: always()
run: |
docker exec pki journalctl -x --no-pager -u [email protected]
- name: Check CA debug log
if: always()
run: |
docker exec pki find /var/lib/pki/pki-tomcat/logs/ca -name "debug.*" -exec cat {} \;
- name: Check ACME debug log
if: always()
run: |
docker exec pki find /var/lib/pki/pki-tomcat/logs/acme -name "debug.*" -exec cat {} \;
- name: Check certbot log
if: always()
run: |
docker exec client cat /var/log/letsencrypt/letsencrypt.log
- name: Gather artifacts from server containers
if: always()
run: |
Expand All @@ -570,14 +610,6 @@ jobs:
docker cp client:/var/log/letsencrypt/letsencrypt.log /tmp/artifacts/client/var/log/letsencrypt
continue-on-error: true

- name: Remove ACME from PKI container
run: |
docker exec pki pki-server acme-undeploy --wait
docker exec pki pki-server acme-remove
- name: Remove CA from PKI container
run: docker exec pki pkidestroy -i pki-tomcat -s CA -v

- name: Upload artifacts from server containers
if: always()
uses: actions/upload-artifact@v4
Expand Down
50 changes: 41 additions & 9 deletions .github/workflows/acme-postgresql-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,9 @@ jobs:
--type postgresql \
-Dpassword=mysecretpassword \
-Durl='jdbc:postgresql://postgresql.example.com:5432/acme?ssl=true&sslmode=require'
docker exec pki pki-server acme-issuer-mod --type pki
docker exec pki pki-server acme-issuer-mod \
--type pki \
-D url=https://pki.example.com:8443
docker exec pki pki-server acme-realm-mod \
--type postgresql \
-Dpassword=mysecretpassword \
Expand Down Expand Up @@ -523,6 +525,44 @@ jobs:
cat output | awk -F '|' '{ print $3 }' > actual
diff expected actual
- name: Remove ACME from PKI container
run: |
docker exec pki pki-server acme-undeploy --wait
docker exec pki pki-server acme-remove
- name: Remove CA from PKI container
run: docker exec pki pkidestroy -i pki-tomcat -s CA -v

- name: Check DS server systemd journal
if: always()
run: |
docker exec ds journalctl -x --no-pager -u [email protected]
- name: Check DS container logs
if: always()
run: |
docker logs ds
- name: Check PKI server systemd journal
if: always()
run: |
docker exec pki journalctl -x --no-pager -u [email protected]
- name: Check CA debug log
if: always()
run: |
docker exec pki find /var/lib/pki/pki-tomcat/logs/ca -name "debug.*" -exec cat {} \;
- name: Check ACME debug log
if: always()
run: |
docker exec pki find /var/lib/pki/pki-tomcat/logs/acme -name "debug.*" -exec cat {} \;
- name: Check certbot log
if: always()
run: |
docker exec client cat /var/log/letsencrypt/letsencrypt.log
- name: Gather artifacts from server containers
if: always()
run: |
Expand All @@ -541,14 +581,6 @@ jobs:
docker cp client:/var/log/letsencrypt/letsencrypt.log /tmp/artifacts/client/var/log/letsencrypt
continue-on-error: true

- name: Remove ACME from PKI container
run: |
docker exec pki pki-server acme-undeploy --wait
docker exec pki pki-server acme-remove
- name: Remove CA from PKI container
run: docker exec pki pkidestroy -i pki-tomcat -s CA -v

- name: Upload artifacts from server containers
if: always()
uses: actions/upload-artifact@v4
Expand Down
50 changes: 41 additions & 9 deletions .github/workflows/acme-switchover-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,9 @@ jobs:
docker exec pki pki-server acme-database-mod \
--type ds \
-D url=ldap://ds.example.com:3389
docker exec pki pki-server acme-issuer-mod --type pki
docker exec pki pki-server acme-issuer-mod \
--type pki \
-D url=https://pki.example.com:8443
docker exec pki pki-server acme-realm-mod \
--type ds \
-D url=ldap://ds.example.com:3389
Expand Down Expand Up @@ -163,6 +165,44 @@ jobs:
--server http://pki.example.com:8080/acme/directory \
--non-interactive
- name: Remove ACME from PKI container
run: |
docker exec pki pki-server acme-undeploy --wait
docker exec pki pki-server acme-remove
- name: Remove CA from PKI container
run: docker exec pki pkidestroy -i pki-tomcat -s CA -v

- name: Check DS server systemd journal
if: always()
run: |
docker exec ds journalctl -x --no-pager -u [email protected]
- name: Check DS container logs
if: always()
run: |
docker logs ds
- name: Check PKI server systemd journal
if: always()
run: |
docker exec pki journalctl -x --no-pager -u [email protected]
- name: Check CA debug log
if: always()
run: |
docker exec pki find /var/lib/pki/pki-tomcat/logs/ca -name "debug.*" -exec cat {} \;
- name: Check ACME debug log
if: always()
run: |
docker exec pki find /var/lib/pki/pki-tomcat/logs/acme -name "debug.*" -exec cat {} \;
- name: Check certbot log
if: always()
run: |
docker exec client cat /var/log/letsencrypt/letsencrypt.log
- name: Gather artifacts from server containers
if: always()
run: |
Expand All @@ -181,14 +221,6 @@ jobs:
docker cp client:/var/log/letsencrypt/letsencrypt.log /tmp/artifacts/client/var/log/letsencrypt
continue-on-error: true

- name: Remove ACME from PKI container
run: |
docker exec pki pki-server acme-undeploy --wait
docker exec pki pki-server acme-remove
- name: Remove CA from PKI container
run: docker exec pki pkidestroy -i pki-tomcat -s CA -v

- name: Upload artifacts from server containers
if: always()
uses: actions/upload-artifact@v4
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,6 @@ public PKIClient(ClientConfig config, SSLCertificateApprovalCallback callback) t
this.config = config;

connection = new PKIConnection(config);

if (callback == null) {
callback = new PKICertificateApprovalCallback();
}

connection.setCallback(callback);

String messageFormat = config.getMessageFormat();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,11 @@
import org.dogtagpki.common.InfoClient;
import org.dogtagpki.util.logging.PKILogger;
import org.dogtagpki.util.logging.PKILogger.LogLevel;

import org.mozilla.jss.CryptoManager;
import org.mozilla.jss.crypto.AlreadyInitializedException;

import com.netscape.certsrv.client.ClientConfig;
import com.netscape.certsrv.client.PKICertificateApprovalCallback;
import com.netscape.certsrv.client.PKIClient;
import com.netscape.management.client.Framework;
import com.netscape.management.client.IPage;
Expand All @@ -74,8 +74,8 @@
import com.netscape.management.client.console.ConsoleInfo;
import com.netscape.management.client.console.LoginDialog;
import com.netscape.management.client.console.VersionInfo;
import com.netscape.management.client.preferences.FilePreferences;
import com.netscape.management.client.preferences.FilePreferenceManager;
import com.netscape.management.client.preferences.FilePreferences;
import com.netscape.management.client.preferences.PreferenceManager;
import com.netscape.management.client.preferences.Preferences;
import com.netscape.management.client.topology.IServerObject;
Expand Down Expand Up @@ -1804,7 +1804,9 @@ public static void mainImpl(String argv[]) throws Exception {
config.setCertNickname(client_cert_nick);
}

PKIClient client = new PKIClient(config);
PKICertificateApprovalCallback callback = new PKICertificateApprovalCallback();

PKIClient client = new PKIClient(config, callback);

InfoClient infoClient = new InfoClient(client);
Info info = infoClient.getInfo();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,13 @@
import org.dogtagpki.server.authentication.AuthManagerConfig;
import org.dogtagpki.server.authentication.AuthToken;
import org.dogtagpki.server.authentication.AuthenticationConfig;
import org.mozilla.jss.ssl.SSLCertificateApprovalCallback.ValidityStatus;

import com.netscape.certsrv.authentication.AuthCredentials;
import com.netscape.certsrv.authentication.EInvalidCredentials;
import com.netscape.certsrv.authentication.EMissingCredential;
import com.netscape.certsrv.base.EBaseException;
import com.netscape.certsrv.base.SessionContext;
import com.netscape.certsrv.client.ClientConfig;
import com.netscape.certsrv.client.PKICertificateApprovalCallback;
import com.netscape.certsrv.client.PKIClient;
import com.netscape.certsrv.profile.EProfileException;
import com.netscape.certsrv.property.IDescriptor;
Expand Down Expand Up @@ -202,11 +200,7 @@ private String sendAuthRequest(String authHost, int authPort, String authUrl, Mu
ClientConfig config = new ClientConfig();
config.setServerURL(serverURL);

PKICertificateApprovalCallback callback = new PKICertificateApprovalCallback();
callback.reject(ValidityStatus.UNTRUSTED_ISSUER);
callback.reject(ValidityStatus.BAD_CERT_DOMAIN);

try (PKIClient client = new PKIClient(config, callback)) {
try (PKIClient client = new PKIClient(config)) {
return client.post(authUrl, content, String.class);
}
}
Expand Down

0 comments on commit f08c4bc

Please sign in to comment.