From 7b6582ad78e426821ae5a9769bbb22b0737ac3c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20K=C3=B6rber?= Date: Wed, 3 May 2017 10:59:30 +0200 Subject: [PATCH] revoke-cert sends JWK header --- .../main/java/org/shredzone/acme4j/Certificate.java | 4 ++-- .../org/shredzone/acme4j/RegistrationBuilder.java | 2 +- .../org/shredzone/acme4j/connector/Connection.java | 13 +++++++++---- .../acme4j/connector/DefaultConnection.java | 11 ++++++----- .../java/org/shredzone/acme4j/CertificateTest.java | 7 +++++-- .../shredzone/acme4j/RegistrationBuilderTest.java | 6 ++++-- .../acme4j/connector/DefaultConnectionTest.java | 4 ++-- .../shredzone/acme4j/connector/DummyConnection.java | 3 ++- 8 files changed, 31 insertions(+), 19 deletions(-) diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/Certificate.java b/acme4j-client/src/main/java/org/shredzone/acme4j/Certificate.java index ec31923d..bf8e7fc3 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/Certificate.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/Certificate.java @@ -166,7 +166,7 @@ public class Certificate extends AcmeResource { LOG.debug("revoke"); URL resUrl = getSession().resourceUrl(Resource.REVOKE_CERT); if (resUrl == null) { - throw new AcmeProtocolException("CA does not support certificate revocation"); + throw new AcmeException("Server does not allow certificate revocation"); } try (Connection conn = getSession().provider().connect()) { @@ -176,7 +176,7 @@ public class Certificate extends AcmeResource { claims.put("reason", reason.getReasonCode()); } - conn.sendSignedRequest(resUrl, claims, getSession()); + conn.sendSignedRequest(resUrl, claims, getSession(), true); conn.accept(HttpURLConnection.HTTP_OK); } catch (CertificateEncodingException ex) { throw new AcmeProtocolException("Invalid certificate", ex); diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/RegistrationBuilder.java b/acme4j-client/src/main/java/org/shredzone/acme4j/RegistrationBuilder.java index 764f1920..9077369a 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/RegistrationBuilder.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/RegistrationBuilder.java @@ -126,7 +126,7 @@ public class RegistrationBuilder { createExternalAccountBinding(keyIdentifier, session.getKeyPair(), resourceUrl)); } - conn.sendJwkSignedRequest(resourceUrl, claims, session); + conn.sendSignedRequest(resourceUrl, claims, session, true); conn.accept(HttpURLConnection.HTTP_OK, HttpURLConnection.HTTP_CREATED); URL location = conn.getLocation(); diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/connector/Connection.java b/acme4j-client/src/main/java/org/shredzone/acme4j/connector/Connection.java index 89a38772..a4c37291 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/connector/Connection.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/connector/Connection.java @@ -49,8 +49,8 @@ public interface Connection extends AutoCloseable { void sendRequest(URL url, Session session) throws AcmeException; /** - * Sends a signed POST request. Ensures that the session has a KeyIdentifier set that - * is used in the "kid" protected header. + * Sends a signed POST request. Ensures that the session has a KeyIdentifier set, and + * that the "kid" protected header field is used. * * @param url * {@link URL} to send the request to. @@ -63,7 +63,7 @@ public interface Connection extends AutoCloseable { /** * Sends a signed POST request. If the session's KeyIdentifier is set, a "kid" - * protected header is sent. If not, a "jwk" protected header is sent. + * protected header field is sent. If not, a "jwk" protected header field is sent. * * @param url * {@link URL} to send the request to. @@ -71,8 +71,13 @@ public interface Connection extends AutoCloseable { * {@link JSONBuilder} containing claims. Must not be {@code null}. * @param session * {@link Session} instance to be used for signing and tracking + * @param enforceJwk + * {@code true} to enforce a "jwk" header field even if a KeyIdentifier is + * set, {@code false} to choose between "kid" and "jwk" header field + * automatically */ - void sendJwkSignedRequest(URL url, JSONBuilder claims, Session session) throws AcmeException; + void sendSignedRequest(URL url, JSONBuilder claims, Session session, boolean enforceJwk) + throws AcmeException; /** * Checks if the HTTP response status is in the given list of acceptable HTTP states, diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/connector/DefaultConnection.java b/acme4j-client/src/main/java/org/shredzone/acme4j/connector/DefaultConnection.java index 66e536ff..fae75bdb 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/connector/DefaultConnection.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/connector/DefaultConnection.java @@ -152,11 +152,12 @@ public class DefaultConnection implements Connection { throw new IllegalStateException("session has no KeyIdentifier set"); } - sendJwkSignedRequest(url, claims, session); + sendSignedRequest(url, claims, session, false); } @Override - public void sendJwkSignedRequest(URL url, JSONBuilder claims, Session session) throws AcmeException { + public void sendSignedRequest(URL url, JSONBuilder claims, Session session, boolean enforceJwk) + throws AcmeException { Objects.requireNonNull(url, "url"); Objects.requireNonNull(claims, "claims"); Objects.requireNonNull(session, "session"); @@ -182,7 +183,9 @@ public class DefaultConnection implements Connection { jws.setPayload(claims.toString()); jws.getHeaders().setObjectHeaderValue("nonce", Base64Url.encode(session.getNonce())); jws.getHeaders().setObjectHeaderValue("url", url); - if (session.getKeyIdentifier() != null) { + if (enforceJwk || session.getKeyIdentifier() == null) { + jws.getHeaders().setJwkHeaderValue("jwk", jwk); + } else { // TODO PEBBLE: cannot process "kid" yet, send "jwk" instead // https://github.com/letsencrypt/pebble/issues/23 if (Pebble.workaround()) { @@ -190,8 +193,6 @@ public class DefaultConnection implements Connection { } else { jws.getHeaders().setObjectHeaderValue("kid", session.getKeyIdentifier()); } - } else { - jws.getHeaders().setJwkHeaderValue("jwk", jwk); } jws.setAlgorithmHeaderValue(keyAlgorithm(jwk)); diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/CertificateTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/CertificateTest.java index ac17390a..76859240 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/CertificateTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/CertificateTest.java @@ -136,10 +136,12 @@ public class CertificateTest { } @Override - public void sendSignedRequest(URL url, JSONBuilder claims, Session session) { + public void sendSignedRequest(URL url, JSONBuilder claims, Session session, boolean enforceJwk) { assertThat(url, is(resourceUrl)); assertThat(claims.toString(), sameJSONAs(getJSON("revokeCertificateRequest").toString())); assertThat(session, is(notNullValue())); + assertThat(session.getKeyIdentifier(), is(nullValue())); + assertThat(enforceJwk, is(true)); certRequested = false; } @@ -188,10 +190,11 @@ public class CertificateTest { } @Override - public void sendSignedRequest(URL url, JSONBuilder claims, Session session) { + public void sendSignedRequest(URL url, JSONBuilder claims, Session session, boolean enforceJwk) { assertThat(url, is(resourceUrl)); assertThat(claims.toString(), sameJSONAs(getJSON("revokeCertificateWithReasonRequest").toString())); assertThat(session, is(notNullValue())); + assertThat(enforceJwk, is(true)); certRequested = false; } diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/RegistrationBuilderTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/RegistrationBuilderTest.java index b65c651d..8c08ba58 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/RegistrationBuilderTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/RegistrationBuilderTest.java @@ -57,10 +57,11 @@ public class RegistrationBuilderTest { } @Override - public void sendJwkSignedRequest(URL url, JSONBuilder claims, Session session) { + public void sendSignedRequest(URL url, JSONBuilder claims, Session session, boolean enforceJwk) { assertThat(session, is(notNullValue())); assertThat(url, is(resourceUrl)); assertThat(claims.toString(), sameJSONAs(getJSON("newRegistration").toString())); + assertThat(enforceJwk, is(true)); isUpdate = false; } @@ -120,10 +121,11 @@ public class RegistrationBuilderTest { TestableConnectionProvider provider = new TestableConnectionProvider() { @Override - public void sendJwkSignedRequest(URL url, JSONBuilder claims, Session session) { + public void sendSignedRequest(URL url, JSONBuilder claims, Session session, boolean enforceJwk) { try { assertThat(session, is(notNullValue())); assertThat(url, is(resourceUrl)); + assertThat(enforceJwk, is(true)); JSON binding = claims.toJSON() .get("external-account-binding") diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/connector/DefaultConnectionTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/connector/DefaultConnectionTest.java index a4b417f5..b8a604a7 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/connector/DefaultConnectionTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/connector/DefaultConnectionTest.java @@ -747,7 +747,7 @@ public class DefaultConnectionTest { }) { JSONBuilder cb = new JSONBuilder(); cb.put("foo", 123).put("bar", "a-string"); - conn.sendJwkSignedRequest(requestUrl, cb, session); + conn.sendSignedRequest(requestUrl, cb, session, true); } verify(mockUrlConnection).setRequestMethod("POST"); @@ -812,7 +812,7 @@ public class DefaultConnectionTest { try (DefaultConnection conn = new DefaultConnection(mockHttpConnection)) { JSONBuilder cb = new JSONBuilder(); - conn.sendJwkSignedRequest(requestUrl, cb, DefaultConnectionTest.this.session); + conn.sendSignedRequest(requestUrl, cb, DefaultConnectionTest.this.session, true); } } diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/connector/DummyConnection.java b/acme4j-client/src/test/java/org/shredzone/acme4j/connector/DummyConnection.java index bae6b7c1..f9383a47 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/connector/DummyConnection.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/connector/DummyConnection.java @@ -46,7 +46,8 @@ public class DummyConnection implements Connection { } @Override - public void sendJwkSignedRequest(URL url, JSONBuilder claims, Session session) { + public void sendSignedRequest(URL url, JSONBuilder claims, Session session, boolean enforceJwk) + throws AcmeException { throw new UnsupportedOperationException(); }