From c97392236d36fcbe03a2b096f4e7b8392fee7831 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20K=C3=B6rber?= Date: Sat, 19 Dec 2015 17:35:53 +0100 Subject: [PATCH] Use Replay-Nonce header from directory request If there is a Replay-Nonce on the first GET request on the directory resource, use it. It saves us a HEAD request before the first POST request. --- .../acme4j/connector/Connection.java | 19 ++--- .../acme4j/impl/AbstractAcmeClient.java | 7 ++ .../acme4j/impl/DefaultConnection.java | 63 ++++++--------- .../acme4j/impl/GenericAcmeClient.java | 4 + .../acme4j/impl/DefaultConnectionTest.java | 78 +++++++------------ .../acme4j/impl/DummyConnection.java | 10 +-- 6 files changed, 76 insertions(+), 105 deletions(-) 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 c30034ab..d5a8bf3d 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 @@ -29,17 +29,6 @@ import org.shredzone.acme4j.util.ClaimBuilder; */ public interface Connection extends AutoCloseable { - /** - * Forcedly starts a new {@link Session}. Usually this method is not required, as a - * session is automatically started if necessary. - * - * @param uri - * {@link URI} a HEAD request is sent to for starting the session - * @param session - * {@link Session} instance to be used for tracking - */ - void startSession(URI uri, Session session) throws AcmeException; - /** * Sends a simple GET request. * @@ -85,6 +74,14 @@ public interface Connection extends AutoCloseable { */ Map readDirectory() throws AcmeException; + /** + * Updates a {@link Session} by evaluating the HTTP response header. + * + * @param session + * {@link Session} instance to be updated + */ + void updateSession(Session session) throws AcmeException; + /** * Gets a location from the {@code Location} header. * diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/impl/AbstractAcmeClient.java b/acme4j-client/src/main/java/org/shredzone/acme4j/impl/AbstractAcmeClient.java index ed9b8bea..675510db 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/impl/AbstractAcmeClient.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/impl/AbstractAcmeClient.java @@ -72,6 +72,13 @@ public abstract class AbstractAcmeClient implements AcmeClient { */ protected abstract Connection createConnection(); + /** + * Returns the {@link Session} instance of this client. + */ + protected Session getSession() { + return session; + } + @Override public void newRegistration(Account account, Registration registration) throws AcmeException { LOG.debug("newRegistration"); diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/impl/DefaultConnection.java b/acme4j-client/src/main/java/org/shredzone/acme4j/impl/DefaultConnection.java index ed6d7a8c..37fef26c 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/impl/DefaultConnection.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/impl/DefaultConnection.java @@ -65,20 +65,6 @@ public class DefaultConnection implements Connection { this.httpConnector = httpConnector; } - @Override - public void startSession(URI uri, Session session) throws AcmeException { - try { - LOG.debug("Initial replay nonce from {}", uri); - HttpURLConnection localConn = httpConnector.openConnection(uri); - localConn.setRequestMethod("HEAD"); - localConn.connect(); - - session.setNonce(getNonceFromHeader(localConn)); - } catch (IOException ex) { - throw new AcmeException("Failed to request a nonce", ex); - } - } - @Override public int sendRequest(URI uri) throws AcmeException { try { @@ -105,8 +91,13 @@ public class DefaultConnection implements Connection { KeyPair keypair = account.getKeyPair(); if (session.getNonce() == null) { - startSession(uri, session); + LOG.debug("Getting initial nonce, HEAD {}", uri); + conn = httpConnector.openConnection(uri); + conn.setRequestMethod("HEAD"); + conn.connect(); + updateSession(session); } + if (session.getNonce() == null) { throw new AcmeException("No nonce available"); } @@ -139,7 +130,7 @@ public class DefaultConnection implements Connection { logHeaders(); - session.setNonce(getNonceFromHeader(conn)); + updateSession(session); return conn.getResponseCode(); } catch (JoseException | IOException ex) { @@ -235,6 +226,22 @@ public class DefaultConnection implements Connection { return resourceMap; } + @Override + public void updateSession(Session session) throws AcmeException { + String nonceHeader = conn.getHeaderField("Replay-Nonce"); + if (nonceHeader == null || nonceHeader.trim().isEmpty()) { + return; + } + + if (!BASE64URL_PATTERN.matcher(nonceHeader).matches()) { + throw new AcmeException("Invalid replay nonce: " + nonceHeader); + } + + LOG.debug("Replay Nonce: {}", nonceHeader); + + session.setNonce(Base64Url.decode(nonceHeader)); + } + @Override public URI getLocation() throws AcmeException { String location = conn.getHeaderField("Location"); @@ -293,30 +300,6 @@ public class DefaultConnection implements Connection { conn = null; } - /** - * Extracts a nonce from the header. - * - * @param localConn - * {@link HttpURLConnection} to get the nonce from - * @return Nonce - * @throws AcmeException - * if there was no {@code Replay-Nonce} header, or the nonce was invalid - */ - protected byte[] getNonceFromHeader(HttpURLConnection localConn) throws AcmeException { - String nonceHeader = localConn.getHeaderField("Replay-Nonce"); - if (nonceHeader == null || nonceHeader.trim().isEmpty()) { - throw new AcmeException("No replay nonce"); - } - - if (!BASE64URL_PATTERN.matcher(nonceHeader).matches()) { - throw new AcmeException("Invalid replay nonce: " + nonceHeader); - } - - LOG.debug("Replay Nonce: {}", nonceHeader); - - return Base64Url.decode(nonceHeader); - } - /** * Log all HTTP headers in debug mode. */ diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/impl/GenericAcmeClient.java b/acme4j-client/src/main/java/org/shredzone/acme4j/impl/GenericAcmeClient.java index 2edebaf4..c7e6dd84 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/impl/GenericAcmeClient.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/impl/GenericAcmeClient.java @@ -68,6 +68,10 @@ public class GenericAcmeClient extends AbstractAcmeClient { if (rc != HttpURLConnection.HTTP_OK) { conn.throwAcmeException(); } + + // use nonce header if there is one, saves a HEAD request... + conn.updateSession(getSession()); + directoryMap.putAll(conn.readDirectory()); } } diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/impl/DefaultConnectionTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/impl/DefaultConnectionTest.java index c048432f..85fa7d08 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/impl/DefaultConnectionTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/impl/DefaultConnectionTest.java @@ -15,7 +15,6 @@ package org.shredzone.acme4j.impl; import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; -import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.*; import static uk.co.datumedge.hamcrest.json.SameJSONAs.sameJSONAs; @@ -29,10 +28,8 @@ import java.security.KeyPair; import java.security.cert.X509Certificate; import java.util.Arrays; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Set; import org.jose4j.base64url.Base64Url; import org.jose4j.jwx.CompactSerializer; @@ -70,27 +67,28 @@ public class DefaultConnectionTest { } /** - * Test if {@link DefaultConnection#getNonceFromHeader(HttpURLConnection)} throws an - * exception if there is no {@code Replay-Nonce} header. + * Test if {@link DefaultConnection#updateSession(Session)} does nothing if there is + * no {@code Replay-Nonce} header. */ @Test public void testNoNonceFromHeader() throws AcmeException { when(mockUrlConnection.getHeaderField("Replay-Nonce")).thenReturn(null); + Session session = new Session(); + assertThat(session.getNonce(), is(nullValue())); try (DefaultConnection conn = new DefaultConnection(mockHttpConnection)) { - conn.getNonceFromHeader(mockUrlConnection); - fail("Expected to fail"); - } catch (AcmeException ex) { - assertThat(ex.getMessage(), is("No replay nonce")); + conn.conn = mockUrlConnection; + conn.updateSession(session); } + assertThat(session.getNonce(), is(nullValue())); verify(mockUrlConnection).getHeaderField("Replay-Nonce"); verifyNoMoreInteractions(mockUrlConnection); } /** - * Test that {@link DefaultConnection#getNonceFromHeader(HttpURLConnection)} extracts - * a {@code Replay-Nonce} header correctly. + * Test that {@link DefaultConnection#updateSession(Session)} extracts a + * {@code Replay-Nonce} header correctly. */ @Test public void testGetNonceFromHeader() throws AcmeException { @@ -99,18 +97,20 @@ public class DefaultConnectionTest { when(mockUrlConnection.getHeaderField("Replay-Nonce")) .thenReturn(Base64Url.encode(nonce)); + Session session = new Session(); try (DefaultConnection conn = new DefaultConnection(mockHttpConnection)) { - byte[] nonceFromHeader = conn.getNonceFromHeader(mockUrlConnection); - assertThat(nonceFromHeader, is(nonce)); + conn.conn = mockUrlConnection; + conn.updateSession(session); } + assertThat(session.getNonce(), is(nonce)); verify(mockUrlConnection).getHeaderField("Replay-Nonce"); verifyNoMoreInteractions(mockUrlConnection); } /** - * Test that {@link DefaultConnection#getNonceFromHeader(HttpURLConnection)} fails on - * an invalid {@code Replay-Nonce} header. + * Test that {@link DefaultConnection#updateSession(Session)} fails on an invalid + * {@code Replay-Nonce} header. */ @Test public void testInvalidNonceFromHeader() throws AcmeException { @@ -118,8 +118,10 @@ public class DefaultConnectionTest { when(mockUrlConnection.getHeaderField("Replay-Nonce")).thenReturn(badNonce); + Session session = new Session(); try (DefaultConnection conn = new DefaultConnection(mockHttpConnection)) { - conn.getNonceFromHeader(mockUrlConnection); + conn.conn = mockUrlConnection; + conn.updateSession(session); fail("Expected to fail"); } catch (AcmeException ex) { assertThat(ex.getMessage(), org.hamcrest.Matchers.startsWith("Invalid replay nonce")); @@ -247,28 +249,6 @@ public class DefaultConnectionTest { verifyNoMoreInteractions(mockUrlConnection); } - /** - * Test that a session is properly started. - */ - @Test - public void testStartSession() throws Exception { - byte[] nonce = "foo-nonce-foo".getBytes(); - - when(mockUrlConnection.getHeaderField("Replay-Nonce")) - .thenReturn(Base64Url.encode(nonce)); - - Session session = new Session(); - try (DefaultConnection conn = new DefaultConnection(mockHttpConnection)) { - conn.startSession(requestUri, session); - } - assertThat(session.getNonce(), is(nonce)); - - verify(mockUrlConnection).setRequestMethod("HEAD"); - verify(mockUrlConnection).connect(); - verify(mockUrlConnection).getHeaderField("Replay-Nonce"); - verifyNoMoreInteractions(mockUrlConnection); - } - /** * Test GET requests. */ @@ -294,44 +274,44 @@ public class DefaultConnectionTest { final byte[] nonce1 = "foo-nonce-1-foo".getBytes(); final byte[] nonce2 = "foo-nonce-2-foo".getBytes(); final Session testSession = new Session(); - final Set invoked = new HashSet<>(); final ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); when(mockUrlConnection.getOutputStream()).thenReturn(outputStream); - when(mockUrlConnection.getHeaderField("Replay-Nonce")).thenReturn(Base64Url.encode(nonce2)); try (DefaultConnection conn = new DefaultConnection(mockHttpConnection) { @Override - public void startSession(URI uri, Session session) throws AcmeException { - assertThat(uri, is(requestUri)); + public void updateSession(Session session) throws AcmeException { assertThat(session, is(sameInstance(testSession))); - assertThat(testSession.getNonce(), is(nullValue())); - invoked.add("startSession"); - session.setNonce(nonce1); + if (session.getNonce() == null) { + session.setNonce(nonce1); + } else if (session.getNonce() == nonce1) { + session.setNonce(nonce2); + } else { + fail("unknown nonce"); + } }; }) { ClaimBuilder cb = new ClaimBuilder(); cb.put("foo", 123).put("bar", "a-string"); - KeyPair keypair = TestUtils.createKeyPair(); Account account = new Account(keypair); conn.sendSignedRequest(requestUri, cb, testSession, account); } + verify(mockUrlConnection).setRequestMethod("HEAD"); + verify(mockUrlConnection, times(2)).connect(); + verify(mockUrlConnection).setRequestMethod("POST"); verify(mockUrlConnection).setRequestProperty("Accept", "application/json"); verify(mockUrlConnection).setRequestProperty("Accept-Charset", "utf-8"); verify(mockUrlConnection).setRequestProperty("Content-Type", "application/json"); verify(mockUrlConnection).setDoOutput(true); - verify(mockUrlConnection).connect(); verify(mockUrlConnection).setFixedLengthStreamingMode(outputStream.toByteArray().length); - verify(mockUrlConnection, atLeastOnce()).getHeaderField(anyString()); verify(mockUrlConnection).getOutputStream(); verify(mockUrlConnection).getResponseCode(); verifyNoMoreInteractions(mockUrlConnection); - assertThat(invoked, hasItem("startSession")); String[] written = CompactSerializer.deserialize(new String(outputStream.toByteArray(), "utf-8")); String header = Base64Url.decodeToUtf8String(written[0]); diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/impl/DummyConnection.java b/acme4j-client/src/test/java/org/shredzone/acme4j/impl/DummyConnection.java index f4b05b84..5858e65c 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/impl/DummyConnection.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/impl/DummyConnection.java @@ -32,11 +32,6 @@ import org.shredzone.acme4j.util.ClaimBuilder; */ public class DummyConnection implements Connection { - @Override - public void startSession(URI uri, Session session) throws AcmeException { - throw new UnsupportedOperationException(); - } - @Override public int sendRequest(URI uri) throws AcmeException { throw new UnsupportedOperationException(); @@ -62,6 +57,11 @@ public class DummyConnection implements Connection { throw new UnsupportedOperationException(); } + @Override + public void updateSession(Session session) throws AcmeException { + throw new UnsupportedOperationException(); + } + @Override public URI getLocation() throws AcmeException { throw new UnsupportedOperationException();