From 69a23e7bf69ba252c4247dcd0dd94143b7b9da45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20K=C3=B6rber?= Date: Tue, 6 Mar 2018 22:10:08 +0100 Subject: [PATCH] Avoid unnecessary de/encoding of nonces --- .../java/org/shredzone/acme4j/Session.java | 11 +++++----- .../acme4j/connector/Connection.java | 4 ++-- .../acme4j/connector/DefaultConnection.java | 11 +++++----- .../acme4j/provider/AbstractAcmeProvider.java | 2 +- .../org/shredzone/acme4j/SessionTest.java | 5 ++--- .../connector/DefaultConnectionTest.java | 20 +++++++++---------- .../acme4j/connector/DummyConnection.java | 2 +- .../shredzone/acme4j/toolbox/TestUtils.java | 2 +- 8 files changed, 27 insertions(+), 30 deletions(-) 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 a6397a44..eefe93b6 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/Session.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/Session.java @@ -40,8 +40,7 @@ public class Session { private final URI serverUri; private final AcmeProvider provider; - private byte[] nonce; - private JSON directoryJson; + private String nonce; private Locale locale = Locale.getDefault(); protected Instant directoryCacheExpiry; @@ -101,16 +100,16 @@ public class Session { } /** - * Gets the last nonce, or {@code null} if the session is new. + * Gets the last base64 encoded nonce, or {@code null} if the session is new. */ - public byte[] getNonce() { + public String getNonce() { return nonce; } /** - * Sets the nonce received by the server. + * Sets the base64 encoded nonce received by the server. */ - public void setNonce(byte[] nonce) { + public void setNonce(String nonce) { this.nonce = nonce; } 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 a92e9621..056f10a2 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 @@ -117,9 +117,9 @@ public interface Connection extends AutoCloseable { /** * Gets the nonce from the nonce header. * - * @return Nonce, or {@code null} if no nonce header was set + * @return Base64 encoded nonce, or {@code null} if no nonce header was set */ - byte[] getNonce(); + String getNonce(); /** * Gets a location from the {@code Location} header. 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 53244153..79b78984 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 @@ -37,7 +37,6 @@ import java.util.Optional; import java.util.regex.Matcher; import java.util.regex.Pattern; -import org.jose4j.base64url.Base64Url; import org.jose4j.jwk.PublicJsonWebKey; import org.jose4j.jws.JsonWebSignature; import org.jose4j.lang.JoseException; @@ -112,7 +111,7 @@ public class DefaultConnection implements Connection { throwAcmeException(); } - byte[] nonce = getNonce(); + String nonce = getNonce(); if (nonce == null) { throw new AcmeProtocolException("Server did not provide a nonce"); } @@ -247,12 +246,12 @@ public class DefaultConnection implements Connection { } @Override - public byte[] getNonce() { + public String getNonce() { assertConnectionIsOpen(); String nonceHeader = conn.getHeaderField(REPLAY_NONCE_HEADER); if (nonceHeader == null || nonceHeader.trim().isEmpty()) { - return null; //NOSONAR: consistent with other getters + return null; } if (!BASE64URL_PATTERN.matcher(nonceHeader).matches()) { @@ -261,7 +260,7 @@ public class DefaultConnection implements Connection { LOG.debug("Replay Nonce: {}", nonceHeader); - return Base64Url.decode(nonceHeader); + return nonceHeader; } @Override @@ -323,7 +322,7 @@ public class DefaultConnection implements Connection { 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("nonce", session.getNonce()); jws.getHeaders().setObjectHeaderValue("url", url); if (accountLocation == null) { jws.getHeaders().setJwkHeaderValue("jwk", jwk); diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/provider/AbstractAcmeProvider.java b/acme4j-client/src/main/java/org/shredzone/acme4j/provider/AbstractAcmeProvider.java index 441955ee..361bfea7 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/provider/AbstractAcmeProvider.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/provider/AbstractAcmeProvider.java @@ -54,7 +54,7 @@ public abstract class AbstractAcmeProvider implements AcmeProvider { conn.sendRequest(resolve(serverUri), session); // use nonce header if there is one, saves a HEAD request... - byte[] nonce = conn.getNonce(); + String nonce = conn.getNonce(); if (nonce != null) { session.setNonce(nonce); } 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 83283235..f1f0a225 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/SessionTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/SessionTest.java @@ -76,9 +76,8 @@ public class SessionTest { Session session = new Session(serverUri); assertThat(session.getNonce(), is(nullValue())); - byte[] data = "foo-nonce-bar".getBytes(); - session.setNonce(data); - assertThat(session.getNonce(), is(equalTo(data))); + session.setNonce(DUMMY_NONCE); + assertThat(session.getNonce(), is(equalTo(DUMMY_NONCE))); 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 e882c176..e3353edd 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 @@ -119,7 +119,7 @@ public class DefaultConnectionTest { @Test public void testGetNonceFromHeader() { when(mockUrlConnection.getHeaderField("Replay-Nonce")) - .thenReturn(Base64Url.encode(TestUtils.DUMMY_NONCE)); + .thenReturn(TestUtils.DUMMY_NONCE); try (DefaultConnection conn = new DefaultConnection(mockHttpConnection)) { conn.conn = mockUrlConnection; @@ -175,7 +175,7 @@ public class DefaultConnectionTest { assertThat(session.getNonce(), is(nullValue())); when(mockUrlConnection.getHeaderField("Replay-Nonce")) - .thenReturn(Base64Url.encode(TestUtils.DUMMY_NONCE)); + .thenReturn(TestUtils.DUMMY_NONCE); try (DefaultConnection conn = new DefaultConnection(mockHttpConnection)) { conn.resetNonce(session); @@ -660,8 +660,8 @@ public class DefaultConnectionTest { */ @Test public void testSendSignedRequest() throws Exception { - final byte[] nonce1 = "foo-nonce-1-foo".getBytes(); - final byte[] nonce2 = "foo-nonce-2-foo".getBytes(); + final String nonce1 = Base64Url.encode("foo-nonce-1-foo".getBytes()); + final String nonce2 = Base64Url.encode("foo-nonce-2-foo".getBytes()); final ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); when(mockUrlConnection.getResponseCode()).thenReturn(HttpURLConnection.HTTP_OK); @@ -679,7 +679,7 @@ public class DefaultConnectionTest { } @Override - public byte[] getNonce() { + public String getNonce() { assertThat(session, is(sameInstance(DefaultConnectionTest.this.session))); if (session.getNonce() == nonce1) { return nonce2; @@ -714,7 +714,7 @@ public class DefaultConnectionTest { StringBuilder expectedHeader = new StringBuilder(); expectedHeader.append('{'); - expectedHeader.append("\"nonce\":\"").append(Base64Url.encode(nonce1)).append("\","); + expectedHeader.append("\"nonce\":\"").append(nonce1).append("\","); expectedHeader.append("\"url\":\"").append(requestUrl).append("\","); expectedHeader.append("\"alg\":\"RS256\","); expectedHeader.append("\"kid\":\"").append(accountUrl).append('"'); @@ -735,8 +735,8 @@ public class DefaultConnectionTest { */ @Test public void testSendSignedRequestNoKid() throws Exception { - final byte[] nonce1 = "foo-nonce-1-foo".getBytes(); - final byte[] nonce2 = "foo-nonce-2-foo".getBytes(); + final String nonce1 = Base64Url.encode("foo-nonce-1-foo".getBytes()); + final String nonce2 = Base64Url.encode("foo-nonce-2-foo".getBytes()); final ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); when(mockUrlConnection.getResponseCode()).thenReturn(HttpURLConnection.HTTP_OK); @@ -754,7 +754,7 @@ public class DefaultConnectionTest { } @Override - public byte[] getNonce() { + public String getNonce() { assertThat(session, is(sameInstance(DefaultConnectionTest.this.session))); if (session.getNonce() == nonce1) { return nonce2; @@ -789,7 +789,7 @@ public class DefaultConnectionTest { StringBuilder expectedHeader = new StringBuilder(); expectedHeader.append('{'); - expectedHeader.append("\"nonce\":\"").append(Base64Url.encode(nonce1)).append("\","); + expectedHeader.append("\"nonce\":\"").append(nonce1).append("\","); expectedHeader.append("\"url\":\"").append(requestUrl).append("\","); expectedHeader.append("\"alg\":\"RS256\","); expectedHeader.append("\"jwk\":{"); 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 9851e51d..2bfe67ee 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 @@ -69,7 +69,7 @@ public class DummyConnection implements Connection { } @Override - public byte[] getNonce() { + public String getNonce() { throw new UnsupportedOperationException(); } diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/toolbox/TestUtils.java b/acme4j-client/src/test/java/org/shredzone/acme4j/toolbox/TestUtils.java index e72627bc..4bac66c0 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/toolbox/TestUtils.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/toolbox/TestUtils.java @@ -73,7 +73,7 @@ public final class TestUtils { public static final String ACME_SERVER_URI = "https://example.com/acme"; public static final String ACCOUNT_URL = "https://example.com/acme/account/1"; - public static final byte[] DUMMY_NONCE = "foo-nonce-foo".getBytes(); + public static final String DUMMY_NONCE = Base64Url.encode("foo-nonce-foo".getBytes()); private TestUtils() {