From 1b058f27537c35e83e3da32a250c2ada152978e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20K=C3=B6rber?= Date: Sat, 15 Apr 2017 17:20:31 +0200 Subject: [PATCH] Key-Identifier is part of the session --- .../org/shredzone/acme4j/AcmeResource.java | 1 + .../shredzone/acme4j/RegistrationBuilder.java | 6 +- .../java/org/shredzone/acme4j/Session.java | 15 +++ .../acme4j/connector/Connection.java | 16 +++- .../acme4j/connector/DefaultConnection.java | 16 +++- .../acme4j/RegistrationBuilderTest.java | 32 +++++-- .../shredzone/acme4j/RegistrationTest.java | 4 +- .../org/shredzone/acme4j/SessionTest.java | 7 ++ .../connector/DefaultConnectionTest.java | 94 ++++++++++++++++++- .../acme4j/connector/DummyConnection.java | 5 + .../shredzone/acme4j/it/RegistrationIT.java | 22 +---- 11 files changed, 182 insertions(+), 36 deletions(-) diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/AcmeResource.java b/acme4j-client/src/main/java/org/shredzone/acme4j/AcmeResource.java index 4e01fae8..8ec67d2f 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/AcmeResource.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/AcmeResource.java @@ -59,6 +59,7 @@ public abstract class AcmeResource implements Serializable { */ protected void setLocation(URI location) { this.location = Objects.requireNonNull(location, "location"); + session.setKeyIdentifier(this.location); } /** 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 4edaed6c..5b562723 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/RegistrationBuilder.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/RegistrationBuilder.java @@ -87,6 +87,10 @@ public class RegistrationBuilder { public Registration create(Session session) throws AcmeException { LOG.debug("create"); + if (session.getKeyIdentifier() != null) { + throw new IllegalArgumentException("session already seems to have a Registration"); + } + try (Connection conn = session.provider().connect()) { JSONBuilder claims = new JSONBuilder(); claims.putResource(Resource.NEW_REG); @@ -97,7 +101,7 @@ public class RegistrationBuilder { claims.put("terms-of-service-agreed", termsOfServiceAgreed); } - conn.sendSignedRequest(session.resourceUri(Resource.NEW_REG), claims, session); + conn.sendJwkSignedRequest(session.resourceUri(Resource.NEW_REG), claims, session); conn.accept(HttpURLConnection.HTTP_CREATED); URI location = conn.getLocation(); diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/Session.java b/acme4j-client/src/main/java/org/shredzone/acme4j/Session.java index f1424e8d..9a177a06 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/Session.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/Session.java @@ -44,6 +44,7 @@ public class Session { private final URI serverUri; private KeyPair keyPair; + private URI keyIdentifier; private AcmeProvider provider; private byte[] nonce; private JSON directoryJson; @@ -97,6 +98,20 @@ public class Session { this.keyPair = keyPair; } + /** + * Gets the key identifier of the ACME account. + */ + public URI getKeyIdentifier() { + return keyIdentifier; + } + + /** + * Sets the key identifier of the ACME account. + */ + public void setKeyIdentifier(URI keyIdentifier) { + this.keyIdentifier = keyIdentifier; + } + /** * Gets the last nonce, or {@code null} if the session is new. */ 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 68554181..2b112bbb 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 @@ -47,7 +47,8 @@ public interface Connection extends AutoCloseable { void sendRequest(URI uri, Session session) throws AcmeException; /** - * Sends a signed POST request. + * Sends a signed POST request. Ensures that the session has a KeyIdentifier set that + * is used in the "kid" protected header. * * @param uri * {@link URI} to send the request to. @@ -58,6 +59,19 @@ public interface Connection extends AutoCloseable { */ void sendSignedRequest(URI uri, JSONBuilder claims, Session session) throws AcmeException; + /** + * 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. + * + * @param uri + * {@link URI} to send the request to. + * @param claims + * {@link JSONBuilder} containing claims. Must not be {@code null}. + * @param session + * {@link Session} instance to be used for signing and tracking + */ + void sendJwkSignedRequest(URI uri, JSONBuilder claims, Session session) throws AcmeException; + /** * Checks if the HTTP response status is in the given list of acceptable HTTP states, * otherwise raises an {@link AcmeException} matching the error. 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 29d714f9..81b3c7b1 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 @@ -147,6 +147,15 @@ public class DefaultConnection implements Connection { @Override public void sendSignedRequest(URI uri, JSONBuilder claims, Session session) throws AcmeException { + if (session.getKeyIdentifier() == null) { + throw new IllegalStateException("session has no KeyIdentifier set"); + } + + sendJwkSignedRequest(uri, claims, session); + } + + @Override + public void sendJwkSignedRequest(URI uri, JSONBuilder claims, Session session) throws AcmeException { Objects.requireNonNull(uri, "uri"); Objects.requireNonNull(claims, "claims"); Objects.requireNonNull(session, "session"); @@ -170,12 +179,15 @@ public class DefaultConnection implements Connection { conn.setDoOutput(true); final PublicJsonWebKey jwk = PublicJsonWebKey.Factory.newPublicJwk(keypair.getPublic()); - JsonWebSignature jws = new JsonWebSignature(); jws.setPayload(claims.toString()); jws.getHeaders().setObjectHeaderValue("nonce", Base64Url.encode(session.getNonce())); jws.getHeaders().setObjectHeaderValue("url", uri); - jws.getHeaders().setJwkHeaderValue("jwk", jwk); + if (session.getKeyIdentifier() != null) { + jws.getHeaders().setObjectHeaderValue("kid", session.getKeyIdentifier()); + } else { + jws.getHeaders().setJwkHeaderValue("jwk", jwk); + } jws.setAlgorithmHeaderValue(keyAlgorithm(jwk)); jws.setKey(keypair.getPrivate()); byte[] outputData = jws.getCompactSerialization().getBytes(DEFAULT_CHARSET); 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 5456c985..14e101d3 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/RegistrationBuilderTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/RegistrationBuilderTest.java @@ -47,14 +47,17 @@ public class RegistrationBuilderTest { @Override public void sendSignedRequest(URI uri, JSONBuilder claims, Session session) { assertThat(session, is(notNullValue())); - if (resourceUri.equals(uri)) { - isUpdate = false; - assertThat(claims.toString(), sameJSONAs(getJson("newRegistration"))); - } else if (locationUri.equals(uri)) { - isUpdate = true; - } else { - fail("bad URI"); - } + assertThat(uri, is(locationUri)); + assertThat(isUpdate, is(false)); + isUpdate = true; + } + + @Override + public void sendJwkSignedRequest(URI uri, JSONBuilder claims, Session session) { + assertThat(session, is(notNullValue())); + assertThat(uri, is(resourceUri)); + assertThat(claims.toString(), sameJSONAs(getJson("newRegistration"))); + isUpdate = false; } @Override @@ -86,10 +89,21 @@ public class RegistrationBuilderTest { builder.addContact("mailto:foo@example.com"); builder.agreeToTermsOfService(); - Registration registration = builder.create(provider.createSession()); + Session session = provider.createSession(); + Registration registration = builder.create(session); assertThat(registration.getLocation(), is(locationUri)); assertThat(registration.getTermsOfServiceAgreed(), is(true)); + assertThat(session.getKeyIdentifier(), is(locationUri)); + + try { + RegistrationBuilder builder2 = new RegistrationBuilder(); + builder2.agreeToTermsOfService(); + builder2.create(session); + fail("registered twice on same session"); + } catch (IllegalArgumentException ex) { + // expected + } provider.close(); } diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/RegistrationTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/RegistrationTest.java index e8545acf..1c4c689f 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/RegistrationTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/RegistrationTest.java @@ -120,9 +120,11 @@ public class RegistrationTest { } }; - Registration registration = new Registration(provider.createSession(), locationUri); + Session session = provider.createSession(); + Registration registration = new Registration(session, locationUri); registration.update(); + assertThat(session.getKeyIdentifier(), is(locationUri)); assertThat(registration.getLocation(), is(locationUri)); assertThat(registration.getTermsOfServiceAgreed(), is(true)); assertThat(registration.getContacts(), hasSize(1)); diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/SessionTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/SessionTest.java index 46311ad6..8974e40f 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/SessionTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/SessionTest.java @@ -72,11 +72,13 @@ public class SessionTest { assertThat(session, not(nullValue())); assertThat(session.getServerUri(), is(serverUri)); assertThat(session.getKeyPair(), is(keyPair)); + assertThat(session.getKeyIdentifier(), is(nullValue())); Session session2 = new Session("https://example.com/acme", keyPair); assertThat(session2, not(nullValue())); assertThat(session2.getServerUri(), is(serverUri)); assertThat(session2.getKeyPair(), is(keyPair)); + assertThat(session2.getKeyIdentifier(), is(nullValue())); try { new Session("#*aBaDuRi*#", keyPair); @@ -94,6 +96,7 @@ public class SessionTest { KeyPair kp1 = TestUtils.createKeyPair(); KeyPair kp2 = TestUtils.createDomainKeyPair(); URI serverUri = URI.create(TestUtils.ACME_SERVER_URI); + URI keyIdentifierUri = URI.create(TestUtils.ACME_SERVER_URI + "/acct/1"); Session session = new Session(serverUri, kp1); @@ -106,6 +109,10 @@ public class SessionTest { session.setKeyPair(kp2); assertThat(session.getKeyPair(), is(kp2)); + assertThat(session.getKeyIdentifier(), is(nullValue())); + session.setKeyIdentifier(keyIdentifierUri); + assertThat(session.getKeyIdentifier(), is(keyIdentifierUri)); + assertThat(session.getServerUri(), is(serverUri)); } 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 6ca7605c..cc9d892d 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 @@ -56,7 +56,8 @@ import org.shredzone.acme4j.util.TestUtils; */ public class DefaultConnectionTest { - private URI requestUri = URI.create("http://example.com/acme/");; + private URI requestUri = URI.create("http://example.com/acme/"); + private URI keyIdentifierUri = URI.create(TestUtils.ACME_SERVER_URI + "/acct/1"); private HttpURLConnection mockUrlConnection; private HttpConnector mockHttpConnection; private Session session; @@ -581,7 +582,81 @@ public class DefaultConnectionTest { }) { JSONBuilder cb = new JSONBuilder(); cb.put("foo", 123).put("bar", "a-string"); - conn.sendSignedRequest(requestUri, cb, DefaultConnectionTest.this.session); + session.setKeyIdentifier(keyIdentifierUri); + conn.sendSignedRequest(requestUri, cb, session); + } + + verify(mockUrlConnection).setRequestMethod("POST"); + verify(mockUrlConnection).setRequestProperty("Accept", "application/json"); + verify(mockUrlConnection).setRequestProperty("Accept-Charset", "utf-8"); + verify(mockUrlConnection).setRequestProperty("Accept-Language", "ja-JP"); + verify(mockUrlConnection).setRequestProperty("Content-Type", "application/jose+json"); + verify(mockUrlConnection).connect(); + verify(mockUrlConnection).setDoOutput(true); + verify(mockUrlConnection).setFixedLengthStreamingMode(outputStream.toByteArray().length); + verify(mockUrlConnection).getOutputStream(); + verify(mockUrlConnection, atLeast(0)).getHeaderFields(); + verifyNoMoreInteractions(mockUrlConnection); + + String serialized = new String(outputStream.toByteArray(), "utf-8"); + String[] written = CompactSerializer.deserialize(serialized); + String header = Base64Url.decodeToUtf8String(written[0]); + String claims = Base64Url.decodeToUtf8String(written[1]); + String signature = written[2]; + + StringBuilder expectedHeader = new StringBuilder(); + expectedHeader.append('{'); + expectedHeader.append("\"nonce\":\"").append(Base64Url.encode(nonce1)).append("\","); + expectedHeader.append("\"url\":\"").append(requestUri).append("\","); + expectedHeader.append("\"alg\":\"RS256\","); + expectedHeader.append("\"kid\":\"").append(keyIdentifierUri).append('"'); + expectedHeader.append('}'); + + assertThat(header, sameJSONAs(expectedHeader.toString())); + assertThat(claims, sameJSONAs("{\"foo\":123,\"bar\":\"a-string\"}")); + assertThat(signature, not(isEmptyOrNullString())); + + JsonWebSignature jws = new JsonWebSignature(); + jws.setCompactSerialization(serialized); + jws.setKey(session.getKeyPair().getPublic()); + assertThat(jws.verifySignature(), is(true)); + } + + /** + * Test signed POST requests without KeyIdentifier. + */ + @Test + public void testSendSignedRequestNoKid() throws Exception { + final byte[] nonce1 = "foo-nonce-1-foo".getBytes(); + final byte[] nonce2 = "foo-nonce-2-foo".getBytes(); + final ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + + when(mockUrlConnection.getOutputStream()).thenReturn(outputStream); + + try (DefaultConnection conn = new DefaultConnection(mockHttpConnection) { + @Override + public void resetNonce(Session session) throws AcmeException { + assertThat(session, is(sameInstance(DefaultConnectionTest.this.session))); + if (session.getNonce() == null) { + session.setNonce(nonce1); + } else { + fail("unknown nonce"); + } + }; + + @Override + public void updateSession(Session session) { + assertThat(session, is(sameInstance(DefaultConnectionTest.this.session))); + if (session.getNonce() == nonce1) { + session.setNonce(nonce2); + } else { + fail("unknown nonce"); + } + }; + }) { + JSONBuilder cb = new JSONBuilder(); + cb.put("foo", 123).put("bar", "a-string"); + conn.sendJwkSignedRequest(requestUri, cb, session); } verify(mockUrlConnection).setRequestMethod("POST"); @@ -619,10 +694,21 @@ public class DefaultConnectionTest { JsonWebSignature jws = new JsonWebSignature(); jws.setCompactSerialization(serialized); - jws.setKey(DefaultConnectionTest.this.session.getKeyPair().getPublic()); + jws.setKey(session.getKeyPair().getPublic()); assertThat(jws.verifySignature(), is(true)); } + /** + * Test signed POST requests without a required KeyIdentifier. + */ + @Test(expected = IllegalStateException.class) + public void testSendSignedRequestNoKidFailed() throws Exception { + try (DefaultConnection conn = new DefaultConnection(mockHttpConnection)) { + JSONBuilder cb = new JSONBuilder(); + conn.sendSignedRequest(requestUri, cb, session); + } + } + /** * Test signed POST requests if there is no nonce. */ @@ -635,7 +721,7 @@ public class DefaultConnectionTest { try (DefaultConnection conn = new DefaultConnection(mockHttpConnection)) { JSONBuilder cb = new JSONBuilder(); - conn.sendSignedRequest(requestUri, cb, DefaultConnectionTest.this.session); + conn.sendJwkSignedRequest(requestUri, cb, DefaultConnectionTest.this.session); } } 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 ed1afb51..9ea6e33c 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 @@ -43,6 +43,11 @@ public class DummyConnection implements Connection { throw new UnsupportedOperationException(); } + @Override + public void sendJwkSignedRequest(URI uri, JSONBuilder claims, Session session) { + throw new UnsupportedOperationException(); + } + @Override public int accept(int... httpStatus) throws AcmeException { throw new UnsupportedOperationException(); diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/it/RegistrationIT.java b/acme4j-client/src/test/java/org/shredzone/acme4j/it/RegistrationIT.java index 058545ed..2a6dd5c3 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/it/RegistrationIT.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/it/RegistrationIT.java @@ -14,7 +14,7 @@ package org.shredzone.acme4j.it; import static org.hamcrest.Matchers.*; -import static org.junit.Assert.*; +import static org.junit.Assert.assertThat; import java.net.URI; import java.security.KeyPair; @@ -25,7 +25,6 @@ import org.shredzone.acme4j.Registration; import org.shredzone.acme4j.RegistrationBuilder; import org.shredzone.acme4j.Session; import org.shredzone.acme4j.Status; -import org.shredzone.acme4j.exception.AcmeConflictException; import org.shredzone.acme4j.exception.AcmeException; import org.shredzone.acme4j.exception.AcmeUnauthorizedException; @@ -47,6 +46,9 @@ public class RegistrationIT extends AbstractPebbleIT { Registration reg = rb.create(session); URI location = reg.getLocation(); assertIsPebbleUri(location); + URI keyIdentifier = session.getKeyIdentifier(); + assertIsPebbleUri(keyIdentifier); + assertThat(keyIdentifier, is(location)); // TODO: Not yet supported by Pebble /* @@ -120,22 +122,6 @@ public class RegistrationIT extends AbstractPebbleIT { assertThat(newRegistration.getStatus(), is(Status.GOOD)); } - @Test - public void testDuplicate() throws AcmeException { - KeyPair keyPair = createKeyPair(); - Session session = new Session(pebbleURI(), keyPair); - - // First registration - new RegistrationBuilder().agreeToTermsOfService().create(session); - - try { - new RegistrationBuilder().agreeToTermsOfService().create(session); - fail("Successfully registered KeyPair a second time"); - } catch (AcmeConflictException ex) { - // This exception is expected - } - } - @Test @Ignore // TODO: Not yet supported by Pebble public void testDeactivate() throws AcmeException {