From 3ad325782b3d6aa1e3f6a32b4aeeaaba24a83847 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20K=C3=B6rber?= Date: Fri, 22 Sep 2023 11:20:31 +0200 Subject: [PATCH] Add method to set arbitrary MAC algorithm (#141) --- .../org/shredzone/acme4j/AccountBuilder.java | 29 ++++++++++++++-- .../shredzone/acme4j/toolbox/JoseUtils.java | 6 ++-- .../shredzone/acme4j/AccountBuilderTest.java | 34 ++++++++++++++++--- .../acme4j/toolbox/JoseUtilsTest.java | 20 +++++++---- src/doc/docs/usage/account.md | 2 ++ 5 files changed, 76 insertions(+), 15 deletions(-) diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/AccountBuilder.java b/acme4j-client/src/main/java/org/shredzone/acme4j/AccountBuilder.java index 5f254ee7..f5c499ae 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/AccountBuilder.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/AccountBuilder.java @@ -14,11 +14,14 @@ package org.shredzone.acme4j; import static java.util.Objects.requireNonNull; +import static org.jose4j.jws.AlgorithmIdentifiers.*; +import static org.shredzone.acme4j.toolbox.JoseUtils.macKeyAlgorithm; import java.net.URI; import java.security.KeyPair; import java.util.ArrayList; import java.util.List; +import java.util.Set; import javax.crypto.SecretKey; import javax.crypto.spec.SecretKeySpec; @@ -55,6 +58,7 @@ import org.slf4j.LoggerFactory; */ public class AccountBuilder { private static final Logger LOG = LoggerFactory.getLogger(AccountBuilder.class); + private static final Set VALID_ALGORITHMS = Set.of(HMAC_SHA256, HMAC_SHA384, HMAC_SHA512); private final List contacts = new ArrayList<>(); private @Nullable Boolean termsOfServiceAgreed; @@ -62,6 +66,7 @@ public class AccountBuilder { private @Nullable String keyIdentifier; private @Nullable KeyPair keyPair; private @Nullable SecretKey macKey; + private @Nullable String macAlgorithm; /** * Add a contact URI to the list of contacts. @@ -210,6 +215,25 @@ public class AccountBuilder { return withKeyIdentifier(kid, new SecretKeySpec(encodedKey, "HMAC")); } + /** + * Sets the MAC key algorithm that is provided by the CA. To be used in combination + * with key identifier. By default, the algorithm is deduced from the size of the + * MAC key. If a different size is needed, it can be set using this method. + * + * @param macAlgorithm + * the algorithm to be set in the {@code alg} field, e.g. {@code "HS512"}. + * @return itself + * @since 3.1.0 + */ + public AccountBuilder withMacAlgorithm(String macAlgorithm) { + var algorithm = requireNonNull(macAlgorithm, "macAlgorithm"); + if (!VALID_ALGORITHMS.contains(algorithm)) { + throw new IllegalArgumentException("Invalid MAC algorithm: " + macAlgorithm); + } + this.macAlgorithm = algorithm; + return this; + } + /** * Creates a new account. *

@@ -254,9 +278,10 @@ public class AccountBuilder { if (termsOfServiceAgreed != null) { claims.put("termsOfServiceAgreed", termsOfServiceAgreed); } - if (keyIdentifier != null) { + if (keyIdentifier != null && macKey != null) { + var algorithm = macAlgorithm != null ? macAlgorithm : macKeyAlgorithm(macKey); claims.put("externalAccountBinding", JoseUtils.createExternalAccountBinding( - keyIdentifier, keyPair.getPublic(), macKey, resourceUrl)); + keyIdentifier, keyPair.getPublic(), macKey, algorithm, resourceUrl)); } if (onlyExisting != null) { claims.put("onlyReturnExisting", onlyExisting); diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/toolbox/JoseUtils.java b/acme4j-client/src/main/java/org/shredzone/acme4j/toolbox/JoseUtils.java index 9fa759c2..2f0a7cad 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/toolbox/JoseUtils.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/toolbox/JoseUtils.java @@ -115,12 +115,14 @@ public final class JoseUtils { * {@link PublicKey} of the account to register * @param macKey * {@link SecretKey} to sign the key identifier with + * @param macAlgorithm + * Algorithm of the MAC key * @param resource * "newAccount" resource URL * @return Created JSON structure */ public static Map createExternalAccountBinding(String kid, - PublicKey accountKey, SecretKey macKey, URL resource) { + PublicKey accountKey, SecretKey macKey, String macAlgorithm, URL resource) { try { var keyJwk = PublicJsonWebKey.Factory.newPublicJwk(accountKey); @@ -128,7 +130,7 @@ public final class JoseUtils { innerJws.setPayload(keyJwk.toJson()); innerJws.getHeaders().setObjectHeaderValue("url", resource); innerJws.getHeaders().setObjectHeaderValue("kid", kid); - innerJws.setAlgorithmHeaderValue(macKeyAlgorithm(macKey)); + innerJws.setAlgorithmHeaderValue(macAlgorithm); innerJws.setKey(macKey); innerJws.setDoKeyValidation(false); innerJws.sign(); diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/AccountBuilderTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/AccountBuilderTest.java index 173ba5db..a7cd2fd8 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/AccountBuilderTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/AccountBuilderTest.java @@ -15,6 +15,7 @@ package org.shredzone.acme4j; import static net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatException; import static org.shredzone.acme4j.toolbox.TestUtils.getJSON; import static org.shredzone.acme4j.toolbox.TestUtils.url; @@ -22,8 +23,13 @@ import java.net.HttpURLConnection; import java.net.URL; import java.security.KeyPair; +import edu.umd.cs.findbugs.annotations.Nullable; import org.jose4j.jwx.CompactSerializer; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.NullAndEmptySource; +import org.junit.jupiter.params.provider.ValueSource; import org.mockito.Mockito; import org.shredzone.acme4j.connector.Resource; import org.shredzone.acme4j.provider.TestableConnectionProvider; @@ -105,11 +111,16 @@ public class AccountBuilderTest { /** * Test if a new account with Key Identifier can be created. */ - @Test - public void testRegistrationWithKid() throws Exception { + @ParameterizedTest + @CsvSource({ + "SHA-256,HS256,", "SHA-384,HS384,", "SHA-512,HS512,", + "SHA-256,HS256,HS256", "SHA-384,HS384,HS384", "SHA-512,HS512,HS512", + "SHA-512,HS256,HS256" + }) + public void testRegistrationWithKid(String keyAlg, String expectedMacAlg, @Nullable String macAlg) throws Exception { var accountKey = TestUtils.createKeyPair(); var keyIdentifier = "NCC-1701"; - var macKey = TestUtils.createSecretKey("SHA-256"); + var macKey = TestUtils.createSecretKey(keyAlg); var provider = new TestableConnectionProvider() { @Override @@ -127,7 +138,7 @@ public class AccountBuilderTest { var encodedPayload = binding.get("payload").asString(); var serialized = CompactSerializer.serialize(encodedHeader, encodedPayload, encodedSignature); - JoseUtilsTest.assertExternalAccountBinding(serialized, resourceUrl, keyIdentifier, macKey); + JoseUtilsTest.assertExternalAccountBinding(serialized, resourceUrl, keyIdentifier, macKey, expectedMacAlg); return HttpURLConnection.HTTP_CREATED; } @@ -148,6 +159,9 @@ public class AccountBuilderTest { var builder = new AccountBuilder(); builder.useKeyPair(accountKey); builder.withKeyIdentifier(keyIdentifier, AcmeUtils.base64UrlEncode(macKey.getEncoded())); + if (macAlg != null) { + builder.withMacAlgorithm(macAlg); + } var session = provider.createSession(); var login = builder.createLogin(session); @@ -157,6 +171,18 @@ public class AccountBuilderTest { provider.close(); } + /** + * Test if invalid mac algorithms are rejected. + */ + @ParameterizedTest + @NullAndEmptySource + @ValueSource(strings = {"foo", "null", "false", "none", "HS-256", "hs256", "HS128", "RS256"}) + public void testRejectInvalidMacAlg(@Nullable String macAlg) { + assertThatException().isThrownBy(() -> { + new AccountBuilder().withMacAlgorithm(macAlg); + }).isInstanceOfAny(IllegalArgumentException.class, NullPointerException.class); + } + /** * Test if an existing account is properly returned. */ diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/toolbox/JoseUtilsTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/toolbox/JoseUtilsTest.java index a4464378..74192efc 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/toolbox/JoseUtilsTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/toolbox/JoseUtilsTest.java @@ -30,6 +30,8 @@ import org.jose4j.jws.JsonWebSignature; import org.jose4j.jwx.CompactSerializer; import org.jose4j.lang.JoseException; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; /** * Unit tests for {@link JoseUtils}. @@ -159,22 +161,23 @@ public class JoseUtilsTest { /** * Test if an external account binding is correctly created. */ - @Test - public void testCreateExternalAccountBinding() throws Exception { + @ParameterizedTest + @CsvSource({"SHA-256,HS256", "SHA-384,HS384", "SHA-512,HS512", "SHA-512,HS256"}) + public void testCreateExternalAccountBinding(String keyAlg, String macAlg) throws Exception { var accountKey = TestUtils.createKeyPair(); var keyIdentifier = "NCC-1701"; - var macKey = TestUtils.createSecretKey("SHA-256"); + var macKey = TestUtils.createSecretKey(keyAlg); var resourceUrl = url("http://example.com/acme/resource"); var binding = JoseUtils.createExternalAccountBinding( - keyIdentifier, accountKey.getPublic(), macKey, resourceUrl); + keyIdentifier, accountKey.getPublic(), macKey, macAlg, resourceUrl); var encodedHeader = binding.get("protected").toString(); var encodedSignature = binding.get("signature").toString(); var encodedPayload = binding.get("payload").toString(); var serialized = CompactSerializer.serialize(encodedHeader, encodedPayload, encodedSignature); - assertExternalAccountBinding(serialized, resourceUrl, keyIdentifier, macKey); + assertExternalAccountBinding(serialized, resourceUrl, keyIdentifier, macKey, macAlg); } /** @@ -282,9 +285,12 @@ public class JoseUtilsTest { * Expected key identifier * @param macKey * Expected {@link SecretKey} + * @param macAlg + * Expected algorithm */ public static void assertExternalAccountBinding(String serialized, URL resourceUrl, - String keyIdentifier, SecretKey macKey) { + String keyIdentifier, SecretKey macKey, + String macAlg) { try { var jws = new JsonWebSignature(); jws.setCompactSerialization(serialized); @@ -293,7 +299,7 @@ public class JoseUtilsTest { assertThat(jws.getHeader("url")).isEqualTo(resourceUrl.toString()); assertThat(jws.getHeader("kid")).isEqualTo(keyIdentifier); - assertThat(jws.getHeader("alg")).isEqualTo("HS256"); + assertThat(jws.getHeader("alg")).isEqualTo(macAlg); var decodedPayload = jws.getPayload(); var expectedPayload = new StringBuilder(); diff --git a/src/doc/docs/usage/account.md b/src/doc/docs/usage/account.md index b5f01d65..72a126b1 100644 --- a/src/doc/docs/usage/account.md +++ b/src/doc/docs/usage/account.md @@ -148,3 +148,5 @@ Account account = new AccountBuilder() ``` For your convenience, you can also pass a base64 encoded MAC Key as `String`. + +The MAC algorithm is automatically set from the size of the MAC key. If a different algorithm is required, it can be set using `withMacAlgorithm()`.