From ade0207d6d8aef8b96efdf22ec972717ddedd51d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20K=C3=B6rber?= Date: Thu, 24 Dec 2015 16:28:57 +0100 Subject: [PATCH] Clean out challenge API --- .../shredzone/acme4j/challenge/Challenge.java | 7 +- .../acme4j/challenge/DnsChallenge.java | 62 ++++++++--------- .../acme4j/challenge/GenericChallenge.java | 22 ++----- .../acme4j/challenge/HttpChallenge.java | 43 ++++++------ .../challenge/ProofOfPossessionChallenge.java | 5 +- .../acme4j/challenge/TlsSniChallenge.java | 66 ++++++++----------- .../acme4j/impl/AbstractAcmeClient.java | 2 +- .../acme4j/challenge/DnsChallengeTest.java | 10 +-- .../challenge/GenericChallengeTest.java | 27 ++------ .../acme4j/challenge/HttpChallengeTest.java | 2 +- .../ProofOfPossessionChallengeTest.java | 6 +- .../acme4j/challenge/TlsSniChallengeTest.java | 13 +--- 12 files changed, 100 insertions(+), 165 deletions(-) diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/challenge/Challenge.java b/acme4j-client/src/main/java/org/shredzone/acme4j/challenge/Challenge.java index 2b5beacd..fb7a8ad4 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/challenge/Challenge.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/challenge/Challenge.java @@ -56,12 +56,11 @@ public interface Challenge extends Serializable { void unmarshall(Map map); /** - * Copies the current challenge state to the claim builder, as preparation for - * triggering it. + * Exports the response state, as preparation for triggering the challenge. * * @param cb - * {@link ClaimBuilder} to copy the challenge state to + * {@link ClaimBuilder} to copy the response to */ - void marshall(ClaimBuilder cb); + void respond(ClaimBuilder cb); } diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/challenge/DnsChallenge.java b/acme4j-client/src/main/java/org/shredzone/acme4j/challenge/DnsChallenge.java index a6258597..63aea1c2 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/challenge/DnsChallenge.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/challenge/DnsChallenge.java @@ -37,27 +37,17 @@ public class DnsChallenge extends GenericChallenge { private String authorization = null; /** - * Returns the token to be used for this challenge. + * Authorizes the {@link Challenge} by signing it with an {@link Account}. + * + * @param account + * {@link Account} to sign the challenge with */ - public String getToken() { - return get(KEY_TOKEN); - } - - /** - * Sets the token to be used. - */ - public void setToken(String token) { - put(KEY_TOKEN, token); - } - - /** - * Returns the authorization string to be used for the response. - */ - public String getAuthorization() { - if (authorization == null) { - throw new IllegalStateException("Challenge is not authorized yet"); + public void authorize(Account account) { + if (account == null) { + throw new NullPointerException("account must not be null"); } - return authorization; + + authorization = getToken() + '.' + Base64Url.encode(jwkThumbprint(account.getKeyPair().getPublic())); } /** @@ -76,25 +66,15 @@ public class DnsChallenge extends GenericChallenge { } } - /** - * Authorizes the {@link Challenge} by signing it with an {@link Account}. - * - * @param account - * {@link Account} to sign the challenge with - */ - public void authorize(Account account) { - if (account == null) { - throw new NullPointerException("account must not be null"); + @Override + public void respond(ClaimBuilder cb) { + if (authorization == null) { + throw new IllegalStateException("Challenge is not authorized yet"); } - authorization = getToken() + '.' + Base64Url.encode(jwkThumbprint(account.getKeyPair().getPublic())); - } - - @Override - public void marshall(ClaimBuilder cb) { - cb.put(KEY_KEY_AUTHORIZSATION, getAuthorization()); - cb.put(KEY_TYPE, getType()); + super.respond(cb); cb.put(KEY_TOKEN, getToken()); + cb.put(KEY_KEY_AUTHORIZATION, getAuthorization()); } @Override @@ -102,4 +82,16 @@ public class DnsChallenge extends GenericChallenge { return TYPE.equals(type); } + private String getToken() { + return get(KEY_TOKEN); + } + + private String getAuthorization() { + if (authorization == null) { + throw new IllegalStateException("Challenge is not authorized yet"); + } + + return authorization; + } + } diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/challenge/GenericChallenge.java b/acme4j-client/src/main/java/org/shredzone/acme4j/challenge/GenericChallenge.java index 30308329..575c25d0 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/challenge/GenericChallenge.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/challenge/GenericChallenge.java @@ -36,6 +36,10 @@ import org.shredzone.acme4j.util.ClaimBuilder; * A generic implementation of {@link Challenge}. It can be used as a base class for * actual challenge implemenation, but it is also used if the ACME server offers a * proprietary challenge that is unknown to acme4j. + *

+ * Subclasses must override {@link GenericChallenge#acceptable(String)} so it only + * accepts the own type. {@link GenericChallenge#respond(ClaimBuilder)} should be + * overridden to put all required data to the response. * * @author Richard "Shred" Körber */ @@ -47,7 +51,7 @@ public class GenericChallenge implements Challenge { protected static final String KEY_URI = "uri"; protected static final String KEY_VALIDATED = "validated"; protected static final String KEY_TOKEN = "token"; - protected static final String KEY_KEY_AUTHORIZSATION = "keyAuthorization"; + protected static final String KEY_KEY_AUTHORIZATION = "keyAuthorization"; private transient Map data = new HashMap<>(); @@ -95,8 +99,8 @@ public class GenericChallenge implements Challenge { } @Override - public void marshall(ClaimBuilder cb) { - cb.putAll(data); + public void respond(ClaimBuilder cb) { + cb.put(KEY_TYPE, getType()); } /** @@ -122,18 +126,6 @@ public class GenericChallenge implements Challenge { return (T) data.get(key); } - /** - * Puts a value to the challenge state. - * - * @param key - * Key - * @param value - * Value, may be {@code null} - */ - protected void put(String key, Object value) { - data.put(key, value); - } - /** * Computes a JWK Thumbprint. It is frequently used in responses. * diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/challenge/HttpChallenge.java b/acme4j-client/src/main/java/org/shredzone/acme4j/challenge/HttpChallenge.java index 80b7b75e..e4d38e48 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/challenge/HttpChallenge.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/challenge/HttpChallenge.java @@ -32,6 +32,20 @@ public class HttpChallenge extends GenericChallenge { private String authorization = null; + /** + * Authorizes the {@link Challenge} by signing it with an {@link Account}. + * + * @param account + * {@link Account} to sign the challenge with + */ + public void authorize(Account account) { + if (account == null) { + throw new NullPointerException("account must not be null"); + } + + authorization = getToken() + '.' + Base64Url.encode(jwkThumbprint(account.getKeyPair().getPublic())); + } + /** * Returns the token to be used for this challenge. */ @@ -39,13 +53,6 @@ public class HttpChallenge extends GenericChallenge { return get(KEY_TOKEN); } - /** - * Sets the token to be used. - */ - public void setToken(String token) { - put(KEY_TOKEN, token); - } - /** * Returns the authorization string to be used for the response. *

@@ -60,25 +67,15 @@ public class HttpChallenge extends GenericChallenge { return authorization; } - /** - * Authorizes the {@link Challenge} by signing it with an {@link Account}. - * - * @param account - * {@link Account} to sign the challenge with - */ - public void authorize(Account account) { - if (account == null) { - throw new NullPointerException("account must not be null"); + @Override + public void respond(ClaimBuilder cb) { + if (authorization == null) { + throw new IllegalStateException("Challenge is not authorized yet"); } - authorization = getToken() + '.' + Base64Url.encode(jwkThumbprint(account.getKeyPair().getPublic())); - } - - @Override - public void marshall(ClaimBuilder cb) { - cb.put(KEY_KEY_AUTHORIZSATION, getAuthorization()); - cb.put(KEY_TYPE, getType()); + super.respond(cb); cb.put(KEY_TOKEN, getToken()); + cb.put(KEY_KEY_AUTHORIZATION, getAuthorization()); } @Override diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/challenge/ProofOfPossessionChallenge.java b/acme4j-client/src/main/java/org/shredzone/acme4j/challenge/ProofOfPossessionChallenge.java index 47b81538..e48f90c4 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/challenge/ProofOfPossessionChallenge.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/challenge/ProofOfPossessionChallenge.java @@ -115,13 +115,14 @@ public class ProofOfPossessionChallenge extends GenericChallenge { } @Override - public void marshall(ClaimBuilder cb) { + public void respond(ClaimBuilder cb) { if (validation == null) { throw new IllegalStateException("not validated"); } + super.respond(cb); + try { - cb.put(KEY_TYPE, getType()); cb.put("authorization", JsonUtil.parseJson(validation)); } catch (JoseException ex) { // should not happen, as the JSON is prevalidated in the setter diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/challenge/TlsSniChallenge.java b/acme4j-client/src/main/java/org/shredzone/acme4j/challenge/TlsSniChallenge.java index 825cb0a9..dddaa631 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/challenge/TlsSniChallenge.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/challenge/TlsSniChallenge.java @@ -28,51 +28,16 @@ import org.shredzone.acme4j.util.ClaimBuilder; */ public class TlsSniChallenge extends GenericChallenge { private static final long serialVersionUID = 7370329525205430573L; + private static final char[] HEX = "0123456789abcdef".toCharArray(); /** * Challenge type name: {@value} */ public static final String TYPE = "tls-sni-01"; - private static final char[] HEX = "0123456789abcdef".toCharArray(); - private String authorization = null; private String subject = null; - /** - * Returns the token to be used for this challenge. - */ - public String getToken() { - return get(KEY_TOKEN); - } - - /** - * Sets the token to be used. - */ - public void setToken(String token) { - put(KEY_TOKEN, token); - } - - /** - * Returns the authorization string. - */ - public String getAuthorization() { - if (authorization == null) { - throw new IllegalStateException("Challenge is not authorized yet"); - } - return authorization; - } - - /** - * Return the subject to generate a self-signed certificate for. - */ - public String getSubject() { - if (authorization == null) { - throw new IllegalStateException("Challenge is not authorized yet"); - } - return subject; - } - /** * Authorizes the {@link Challenge} by signing it with an {@link Account}. * @@ -90,11 +55,22 @@ public class TlsSniChallenge extends GenericChallenge { subject = hash.substring(0, 32) + '.' + hash.substring(32) + ".acme.invalid"; } + /** + * Return the subject to generate a self-signed certificate for. + */ + public String getSubject() { + if (authorization == null) { + throw new IllegalStateException("Challenge is not authorized yet"); + } + return subject; + } + + @Override - public void marshall(ClaimBuilder cb) { - cb.put(KEY_KEY_AUTHORIZSATION, getAuthorization()); - cb.put(KEY_TYPE, getType()); + public void respond(ClaimBuilder cb) { + super.respond(cb); cb.put(KEY_TOKEN, getToken()); + cb.put(KEY_KEY_AUTHORIZATION, getAuthorization()); } @Override @@ -127,4 +103,16 @@ public class TlsSniChallenge extends GenericChallenge { } } + private String getToken() { + return get(KEY_TOKEN); + } + + private String getAuthorization() { + if (authorization == null) { + throw new IllegalStateException("Challenge is not authorized yet"); + } + + return authorization; + } + } 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 74d9a619..fe193704 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 @@ -272,7 +272,7 @@ public abstract class AbstractAcmeClient implements AcmeClient { try (Connection conn = createConnection()) { ClaimBuilder claims = new ClaimBuilder(); claims.putResource("challenge"); - challenge.marshall(claims); + challenge.respond(claims); int rc = conn.sendSignedRequest(challenge.getLocation(), claims, session, account); if (rc != HttpURLConnection.HTTP_OK && rc != HttpURLConnection.HTTP_ACCEPTED) { diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/challenge/DnsChallengeTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/challenge/DnsChallengeTest.java index 1d36c4c6..7a7b78c3 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/challenge/DnsChallengeTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/challenge/DnsChallengeTest.java @@ -33,8 +33,6 @@ import org.shredzone.acme4j.util.TestUtils; */ public class DnsChallengeTest { - private static final String TOKEN = - "pNvmJivs0WCko2suV7fhe-59oFqyYx_yB7tx6kIMAyE"; private static final String KEY_AUTHORIZATION = "pNvmJivs0WCko2suV7fhe-59oFqyYx_yB7tx6kIMAyE.HnWjTDnyqlCrm6tZ-6wX-TrEXgRdeNu9G71gqxSO6o0"; @@ -53,20 +51,18 @@ public class DnsChallengeTest { assertThat(challenge.getStatus(), is(Status.PENDING)); try { - challenge.getAuthorization(); - fail("getAuthorization() without previous authorize()"); + challenge.getDigest(); + fail("getDigest() without previous authorize()"); } catch (IllegalStateException ex) { // expected } challenge.authorize(account); - assertThat(challenge.getToken(), is(TOKEN)); - assertThat(challenge.getAuthorization(), is(KEY_AUTHORIZATION)); assertThat(challenge.getDigest(), is("rzMmotrIgsithyBYc0vgiLUEEKYx0WetQRgEF2JIozA")); ClaimBuilder cb = new ClaimBuilder(); - challenge.marshall(cb); + challenge.respond(cb); assertThat(cb.toString(), sameJSONAs("{\"keyAuthorization\"=\"" + KEY_AUTHORIZATION + "\"}").allowingExtraUnexpectedFields()); diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/challenge/GenericChallengeTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/challenge/GenericChallengeTest.java index ec38784e..f8c8bf61 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/challenge/GenericChallengeTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/challenge/GenericChallengeTest.java @@ -67,38 +67,19 @@ public class GenericChallengeTest { } /** - * Test get and put methods. - */ - public void testGetPut() { - GenericChallenge challenge = new GenericChallenge(); - - challenge.put("a-string", "foo"); - challenge.put("a-number", 1234); - - assertThat((String) challenge.get("a-string"), is("foo")); - assertThat((Integer) challenge.get("a-number"), is(1234)); - - ClaimBuilder cb = new ClaimBuilder(); - challenge.marshall(cb); - assertThat(cb.toString(), sameJSONAs("{\"a-string\":\"foo\",\"a-number\":1234}") - .allowingExtraUnexpectedFields()); - } - - /** - * Test that marshalling results in an identical JSON like the one that was - * unmarshalled. + * Test that {@link GenericChallenge#respond(ClaimBuilder)} contains the type. */ @Test - public void testMarshall() throws JoseException { + public void testRespond() throws JoseException { String json = TestUtils.getJson("genericChallenge"); GenericChallenge challenge = new GenericChallenge(); challenge.unmarshall(JsonUtil.parseJson(json)); ClaimBuilder cb = new ClaimBuilder(); - challenge.marshall(cb); + challenge.respond(cb); - assertThat(cb.toString(), sameJSONAs(json)); + assertThat(cb.toString(), sameJSONAs("{\"type\"=\"generic-01\"}")); } /** diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/challenge/HttpChallengeTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/challenge/HttpChallengeTest.java index 0e7d2be1..2829013f 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/challenge/HttpChallengeTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/challenge/HttpChallengeTest.java @@ -65,7 +65,7 @@ public class HttpChallengeTest { assertThat(challenge.getAuthorization(), is(KEY_AUTHORIZATION)); ClaimBuilder cb = new ClaimBuilder(); - challenge.marshall(cb); + challenge.respond(cb); assertThat(cb.toString(), sameJSONAs("{\"keyAuthorization\"=\"" + KEY_AUTHORIZATION + "\"}").allowingExtraUnexpectedFields()); diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/challenge/ProofOfPossessionChallengeTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/challenge/ProofOfPossessionChallengeTest.java index 85781152..0d932b4c 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/challenge/ProofOfPossessionChallengeTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/challenge/ProofOfPossessionChallengeTest.java @@ -54,7 +54,7 @@ public class ProofOfPossessionChallengeTest { assertThat(challenge.getStatus(), is(Status.PENDING)); try { - challenge.marshall(new ClaimBuilder()); + challenge.respond(new ClaimBuilder()); fail("marshall() without previous authorize()"); } catch (IllegalStateException ex) { // expected @@ -63,7 +63,7 @@ public class ProofOfPossessionChallengeTest { challenge.authorize(account, domainKeyPair, "example.org"); ClaimBuilder cb = new ClaimBuilder(); - challenge.marshall(cb); + challenge.respond(cb); assertThat(cb.toString(), sameJSONAs("{\"type\"=\"" + ProofOfPossessionChallenge.TYPE + "\",\"authorization\"=" @@ -90,7 +90,7 @@ public class ProofOfPossessionChallengeTest { challenge.importValidation(validation); ClaimBuilder cb = new ClaimBuilder(); - challenge.marshall(cb); + challenge.respond(cb); assertThat(cb.toString(), sameJSONAs("{\"type\"=\"" + ProofOfPossessionChallenge.TYPE + "\",\"authorization\"=" + validation diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/challenge/TlsSniChallengeTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/challenge/TlsSniChallengeTest.java index 5e6568f6..b35eda55 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/challenge/TlsSniChallengeTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/challenge/TlsSniChallengeTest.java @@ -33,8 +33,6 @@ import org.shredzone.acme4j.util.TestUtils; */ public class TlsSniChallengeTest { - private static final String TOKEN = - "VNLBdSiZ3LppU2CRG8bilqlwq4DuApJMg3ZJowU6JhQ"; private static final String KEY_AUTHORIZATION = "VNLBdSiZ3LppU2CRG8bilqlwq4DuApJMg3ZJowU6JhQ.HnWjTDnyqlCrm6tZ-6wX-TrEXgRdeNu9G71gqxSO6o0"; @@ -52,13 +50,6 @@ public class TlsSniChallengeTest { assertThat(challenge.getType(), is(TlsSniChallenge.TYPE)); assertThat(challenge.getStatus(), is(Status.PENDING)); - try { - challenge.getAuthorization(); - fail("getAuthorization() without previous authorize()"); - } catch (IllegalStateException ex) { - // expected - } - try { challenge.getSubject(); fail("getSubject() without previous authorize()"); @@ -68,12 +59,10 @@ public class TlsSniChallengeTest { challenge.authorize(account); - assertThat(challenge.getToken(), is(TOKEN)); - assertThat(challenge.getAuthorization(), is(KEY_AUTHORIZATION)); assertThat(challenge.getSubject(), is("14e2350a04434f93c2e0b6012968d99d.ed459b6a7a019d9695609b8514f9d63d.acme.invalid")); ClaimBuilder cb = new ClaimBuilder(); - challenge.marshall(cb); + challenge.respond(cb); assertThat(cb.toString(), sameJSONAs("{\"keyAuthorization\"=\"" + KEY_AUTHORIZATION + "\"}").allowingExtraUnexpectedFields());