From 67f80ec745af62bbbeb90a329900d83e40abbf6b Mon Sep 17 00:00:00 2001 From: RTLCoil Date: Thu, 16 Jul 2020 01:27:02 +0300 Subject: [PATCH 1/2] Add support of SHA-256 algorithm in calculation of auth signatures --- .../main/java/com/cloudinary/Cloudinary.java | 10 ++-- .../java/com/cloudinary/Configuration.java | 59 ++++++++++++++++++- .../src/main/java/com/cloudinary/Signer.java | 36 +++++++++++ .../src/main/java/com/cloudinary/Url.java | 11 +--- .../src/main/java/com/cloudinary/Util.java | 19 ++---- .../signing/ApiResponseSignatureVerifier.java | 21 ++++++- .../NotificationRequestSignatureVerifier.java | 14 ++++- .../api/signing/SignedPayloadValidator.java | 22 ++----- .../cloudinary/api/signing/package-info.java | 2 +- .../ApiResponseSignatureVerifierTest.java | 10 ++++ ...ificationRequestSignatureVerifierTest.java | 13 ++++ .../com/cloudinary/test/CloudinaryTest.java | 24 ++++++++ 12 files changed, 192 insertions(+), 49 deletions(-) create mode 100644 cloudinary-core/src/main/java/com/cloudinary/Signer.java diff --git a/cloudinary-core/src/main/java/com/cloudinary/Cloudinary.java b/cloudinary-core/src/main/java/com/cloudinary/Cloudinary.java index 85748a95..d855902a 100644 --- a/cloudinary-core/src/main/java/com/cloudinary/Cloudinary.java +++ b/cloudinary-core/src/main/java/com/cloudinary/Cloudinary.java @@ -130,7 +130,7 @@ public String signedPreloadedImage(Map result) { } public String apiSignRequest(Map paramsToSign, String apiSecret) { - return Util.produceSignature(paramsToSign, apiSecret); + return Util.produceSignature(paramsToSign, apiSecret, config.signatureAlgorithm); } /** @@ -139,7 +139,7 @@ public String apiSignRequest(Map paramsToSign, String apiSecret) * Cloudinary can asynchronously process your e.g. image uploads requests. This is achieved by calling back API you * specified during preparing of upload request as soon as it has been processed. See Upload Notifications in * Cloudinary documentation for more details. In order to make sure it is Cloudinary calling your API back, hashed - * message authentication codes (HMAC's) based on SHA-1 hashing function and configured Cloudinary API secret key + * message authentication codes (HMAC's) based on agreed hashing function and configured Cloudinary API secret key * are used for signing the requests. * * The following method serves as a convenient utility to perform the verification procedure. @@ -151,14 +151,14 @@ public String apiSignRequest(Map paramsToSign, String apiSecret) * @return whether request signature is valid or not */ public boolean verifyNotificationSignature(String body, String timestamp, String signature, long validFor) { - return new NotificationRequestSignatureVerifier(config.apiSecret).verifySignature(body, timestamp, signature, validFor); + return new NotificationRequestSignatureVerifier(config.apiSecret, config.signatureAlgorithm).verifySignature(body, timestamp, signature, validFor); } /** * Verifies that Cloudinary API response is genuine by checking its signature. * * Cloudinary can add a signature value in the response to API methods returning public id's and versions. In order - * to make sure it is genuine Cloudinary response, hashed message authentication codes (HMAC's) based on SHA-1 hashing + * to make sure it is genuine Cloudinary response, hashed message authentication codes (HMAC's) based on agreed hashing * function and configured Cloudinary API secret key are used for signing the responses. * * The following method serves as a convenient utility to perform the verification procedure. @@ -169,7 +169,7 @@ public boolean verifyNotificationSignature(String body, String timestamp, String * @return whether response signature is valid or not */ public boolean verifyApiResponseSignature(String publicId, String version, String signature) { - return new ApiResponseSignatureVerifier(config.apiSecret).verifySignature(publicId, version, signature); + return new ApiResponseSignatureVerifier(config.apiSecret, config.signatureAlgorithm).verifySignature(publicId, version, signature); } public void signRequest(Map params, Map options) { diff --git a/cloudinary-core/src/main/java/com/cloudinary/Configuration.java b/cloudinary-core/src/main/java/com/cloudinary/Configuration.java index 0400276f..42205f92 100644 --- a/cloudinary-core/src/main/java/com/cloudinary/Configuration.java +++ b/cloudinary-core/src/main/java/com/cloudinary/Configuration.java @@ -20,6 +20,9 @@ public class Configuration { public final static String VERSION = "1.0.2"; public final static String USER_AGENT = "cld-android-" + VERSION; public static final boolean DEFAULT_IS_LONG_SIGNATURE = false; + public static final Signer DEFAULT_SIGNATURE_ALGORITHM = Signer.SHA1; + + private static final String CONFIG_PROP_SIGNATURE_ALGORITHM = "signature_algorithm"; public String cloudName; public String apiKey; @@ -43,11 +46,32 @@ public class Configuration { public AuthToken authToken; public boolean forceVersion = true; public boolean longUrlSignature = DEFAULT_IS_LONG_SIGNATURE; + public Signer signatureAlgorithm = DEFAULT_SIGNATURE_ALGORITHM; public Configuration() { } - private Configuration(String cloudName, String apiKey, String apiSecret, String secureDistribution, String cname, String uploadPrefix, boolean secure, boolean privateCdn, boolean cdnSubdomain, boolean shorten, String callback, String proxyHost, int proxyPort, Boolean secureCdnSubdomain, boolean useRootPath, int timeout, boolean loadStrategies, boolean forceVersion, boolean longUrlSignature) { + private Configuration( + String cloudName, + String apiKey, + String apiSecret, + String secureDistribution, + String cname, + String uploadPrefix, + boolean secure, + boolean privateCdn, + boolean cdnSubdomain, + boolean shorten, + String callback, + String proxyHost, + int proxyPort, + Boolean secureCdnSubdomain, + boolean useRootPath, + int timeout, + boolean loadStrategies, + boolean forceVersion, + boolean longUrlSignature, + Signer signatureAlgorithm) { this.cloudName = cloudName; this.apiKey = apiKey; this.apiSecret = apiSecret; @@ -67,6 +91,7 @@ private Configuration(String cloudName, String apiKey, String apiSecret, String this.loadStrategies = loadStrategies; this.forceVersion = forceVersion; this.longUrlSignature = longUrlSignature; + this.signatureAlgorithm = signatureAlgorithm; } @SuppressWarnings("rawtypes") @@ -104,6 +129,7 @@ public void update(Map config) { this.properties.putAll(properties); } this.longUrlSignature = ObjectUtils.asBoolean(config.get("long_url_signature"), DEFAULT_IS_LONG_SIGNATURE); + this.signatureAlgorithm = Signer.getByName(ObjectUtils.asString(config.get(CONFIG_PROP_SIGNATURE_ALGORITHM), DEFAULT_SIGNATURE_ALGORITHM.toString())); } @SuppressWarnings("rawtypes") @@ -133,6 +159,7 @@ public Map asMap() { map.put("force_version", forceVersion); map.put("properties", new HashMap(properties)); map.put("long_url_signature", longUrlSignature); + map.put(CONFIG_PROP_SIGNATURE_ALGORITHM, signatureAlgorithm.toString()); return map; } @@ -162,6 +189,7 @@ public Configuration(Configuration other) { this.loadStrategies = other.loadStrategies; this.properties.putAll(other.properties); this.longUrlSignature = other.longUrlSignature; + this.signatureAlgorithm = other.signatureAlgorithm; } /** @@ -272,6 +300,7 @@ public static class Builder { private AuthToken authToken; private boolean forceVersion = true; private boolean longUrlSignature = DEFAULT_IS_LONG_SIGNATURE; + private Signer signatureAlgorithm = DEFAULT_SIGNATURE_ALGORITHM; /** * Set the HTTP connection timeout. @@ -288,7 +317,27 @@ public Builder setTimeout(int timeout) { * Creates a {@link Configuration} with the arguments supplied to this builder */ public Configuration build() { - final Configuration configuration = new Configuration(cloudName, apiKey, apiSecret, secureDistribution, cname, uploadPrefix, secure, privateCdn, cdnSubdomain, shorten, callback, proxyHost, proxyPort, secureCdnSubdomain, useRootPath, timeout, loadStrategies, forceVersion, longUrlSignature); + final Configuration configuration = new Configuration( + cloudName, + apiKey, + apiSecret, + secureDistribution, + cname, + uploadPrefix, + secure, + privateCdn, + cdnSubdomain, + shorten, + callback, + proxyHost, + proxyPort, + secureCdnSubdomain, + useRootPath, + timeout, + loadStrategies, + forceVersion, + longUrlSignature, + signatureAlgorithm); configuration.clientHints = clientHints; return configuration; } @@ -412,6 +461,11 @@ public Builder setIsLongUrlSignature(boolean isLong) { return this; } + public Builder setSignatureAlgorithm(Signer signatureAlgorithm) { + this.signatureAlgorithm = signatureAlgorithm; + return this; + } + /** * Initialize builder from existing {@link Configuration} * @@ -440,6 +494,7 @@ public Builder from(Configuration other) { this.authToken = other.authToken == null ? null : other.authToken.copy(); this.forceVersion = other.forceVersion; this.longUrlSignature = other.longUrlSignature; + this.signatureAlgorithm = other.signatureAlgorithm; return this; } } diff --git a/cloudinary-core/src/main/java/com/cloudinary/Signer.java b/cloudinary-core/src/main/java/com/cloudinary/Signer.java new file mode 100644 index 00000000..0a216de9 --- /dev/null +++ b/cloudinary-core/src/main/java/com/cloudinary/Signer.java @@ -0,0 +1,36 @@ +package com.cloudinary; + +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; + +public class Signer { + private final String algorithmName; + + private Signer(String algorithmName) { + this.algorithmName = algorithmName; + } + + public byte[] sign(String s) { + try { + return MessageDigest.getInstance(algorithmName).digest(Util.getUTF8Bytes(s)); + } catch (NoSuchAlgorithmException e) { + throw new RuntimeException("Unexpected exception", e); + } + } + + public static final Signer SHA1 = new Signer("SHA-1"); + public static final Signer SHA256 = new Signer("SHA-256"); + + public static Signer getByName(String algorithmName) { + if ("SHA-256".equals(algorithmName)) { + return Signer.SHA256; + } + + return Signer.SHA1; + } + + @Override + public String toString() { + return algorithmName; + } +} diff --git a/cloudinary-core/src/main/java/com/cloudinary/Url.java b/cloudinary-core/src/main/java/com/cloudinary/Url.java index 3d9733c9..612a7062 100644 --- a/cloudinary-core/src/main/java/com/cloudinary/Url.java +++ b/cloudinary-core/src/main/java/com/cloudinary/Url.java @@ -4,8 +4,6 @@ import java.net.MalformedURLException; import java.net.URL; import java.net.URLDecoder; -import java.security.MessageDigest; -import java.security.NoSuchAlgorithmException; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -389,18 +387,13 @@ public String generate(String source) { if (signUrl && (authToken == null || authToken.equals(AuthToken.NULL_AUTH_TOKEN))) { - MessageDigest md = null; - try { - md = MessageDigest.getInstance(longUrlSignature ? "SHA-256" : "SHA-1"); - } catch (NoSuchAlgorithmException e) { - throw new RuntimeException("Unexpected exception", e); - } + Signer signer = longUrlSignature ? Signer.SHA256 : config.signatureAlgorithm; String toSign = StringUtils.join(new String[]{transformationStr, sourceToSign}, "/"); toSign = StringUtils.removeStartingChars(toSign, '/'); toSign = StringUtils.mergeSlashesInUrl(toSign); - byte[] digest = md.digest(Util.getUTF8Bytes(toSign + this.config.apiSecret)); + byte[] digest = signer.sign(toSign + this.config.apiSecret); signature = Base64Coder.encodeURLSafeString(digest); signature = "s--" + signature.substring(0, longUrlSignature ? 32 : 8) + "--"; } diff --git a/cloudinary-core/src/main/java/com/cloudinary/Util.java b/cloudinary-core/src/main/java/com/cloudinary/Util.java index dead1129..4b892e4a 100644 --- a/cloudinary-core/src/main/java/com/cloudinary/Util.java +++ b/cloudinary-core/src/main/java/com/cloudinary/Util.java @@ -4,8 +4,6 @@ import com.cloudinary.utils.StringUtils; import org.cloudinary.json.JSONObject; -import java.security.MessageDigest; -import java.security.NoSuchAlgorithmException; import java.util.*; public class Util { @@ -323,18 +321,19 @@ public static byte[] getUTF8Bytes(String string) { /** * Calculates signature, or hashed message authentication code (HMAC) of provided parameters name-value pairs and - * secret value using SHA-1 hashing algorithm. + * secret value using supported hashing algorithm. *

* Argument for hashing function is built by joining sorted parameter name-value pairs into single string in the * same fashion as HTTP GET method uses, and concatenating the result with secret value in the end. Method supports * arrays/collections as parameter values. In this case, the elements of array/collection are joined into single * comma-delimited string prior to inclusion into the result. * - * @param paramsToSign parameter name-value pairs list represented as instance of {@link Map} - * @param apiSecret secret value + * @param paramsToSign parameter name-value pairs list represented as instance of {@link Map} + * @param apiSecret secret value + * @param algorithmType type of hashing algorithm to use for calculation of HMAC * @return hex-string representation of signature calculated based on provided parameters map and secret */ - public static String produceSignature(Map paramsToSign, String apiSecret) { + public static String produceSignature(Map paramsToSign, String apiSecret, Signer algorithmType) { Collection params = new ArrayList(); for (Map.Entry param : new TreeMap(paramsToSign).entrySet()) { if (param.getValue() instanceof Collection) { @@ -348,13 +347,7 @@ public static String produceSignature(Map paramsToSign, String a } } String to_sign = StringUtils.join(params, "&"); - MessageDigest md = null; - try { - md = MessageDigest.getInstance("SHA-1"); - } catch (NoSuchAlgorithmException e) { - throw new RuntimeException("Unexpected exception", e); - } - byte[] digest = md.digest(getUTF8Bytes(to_sign + apiSecret)); + byte[] digest = algorithmType.sign(to_sign + apiSecret); return StringUtils.encodeHexString(digest); } } diff --git a/cloudinary-core/src/main/java/com/cloudinary/api/signing/ApiResponseSignatureVerifier.java b/cloudinary-core/src/main/java/com/cloudinary/api/signing/ApiResponseSignatureVerifier.java index 33224204..29eacdf4 100644 --- a/cloudinary-core/src/main/java/com/cloudinary/api/signing/ApiResponseSignatureVerifier.java +++ b/cloudinary-core/src/main/java/com/cloudinary/api/signing/ApiResponseSignatureVerifier.java @@ -1,5 +1,6 @@ package com.cloudinary.api.signing; +import com.cloudinary.Signer; import com.cloudinary.Util; import com.cloudinary.utils.ObjectUtils; import com.cloudinary.utils.StringUtils; @@ -11,6 +12,7 @@ */ public class ApiResponseSignatureVerifier { private final String secretKey; + private final Signer signatureAlgorithmType; /** * Initializes new instance of {@code ApiResponseSignatureVerifier} class with a secret key required to perform @@ -24,6 +26,23 @@ public ApiResponseSignatureVerifier(String secretKey) { } this.secretKey = secretKey; + this.signatureAlgorithmType = Signer.SHA1; + } + + /** + * Initializes new instance of {@code ApiResponseSignatureVerifier} class with a secret key required to perform + * API response signatures verification. + * + * @param secretKey shared secret key string which is used to sign and verify authenticity of API responses + * @param algorithmType type of hashing algorithm to use for calculation of HMACs + */ + public ApiResponseSignatureVerifier(String secretKey, Signer algorithmType) { + if (StringUtils.isBlank(secretKey)) { + throw new IllegalArgumentException("Secret key is required"); + } + + this.secretKey = secretKey; + signatureAlgorithmType = algorithmType; } /** @@ -41,6 +60,6 @@ public ApiResponseSignatureVerifier(String secretKey) { public boolean verifySignature(String publicId, String version, String signature) { return Util.produceSignature(ObjectUtils.asMap( "public_id", emptyIfNull(publicId), - "version", emptyIfNull(version)), secretKey).equals(signature); + "version", emptyIfNull(version)), secretKey, signatureAlgorithmType).equals(signature); } } diff --git a/cloudinary-core/src/main/java/com/cloudinary/api/signing/NotificationRequestSignatureVerifier.java b/cloudinary-core/src/main/java/com/cloudinary/api/signing/NotificationRequestSignatureVerifier.java index 497e491e..a3d33372 100644 --- a/cloudinary-core/src/main/java/com/cloudinary/api/signing/NotificationRequestSignatureVerifier.java +++ b/cloudinary-core/src/main/java/com/cloudinary/api/signing/NotificationRequestSignatureVerifier.java @@ -1,5 +1,7 @@ package com.cloudinary.api.signing; +import com.cloudinary.Signer; + import static com.cloudinary.utils.StringUtils.emptyIfNull; /** @@ -15,7 +17,17 @@ public class NotificationRequestSignatureVerifier { * @param secretKey shared secret key string which is used to sign and verify authenticity of notifications */ public NotificationRequestSignatureVerifier(String secretKey) { - this.signedPayloadValidator = new SignedPayloadValidator(secretKey); + this.signedPayloadValidator = new SignedPayloadValidator(secretKey, Signer.SHA1); + } + + /** + * Initializes new instance of {@code NotificationRequestSignatureVerifier} with secret key value. + * + * @param secretKey shared secret key string which is used to sign and verify authenticity of notifications + * @param signatureAlgorithmType type of hashing algorithm to use for calculation of HMACs + */ + public NotificationRequestSignatureVerifier(String secretKey, Signer signatureAlgorithmType) { + this.signedPayloadValidator = new SignedPayloadValidator(secretKey, signatureAlgorithmType); } /** diff --git a/cloudinary-core/src/main/java/com/cloudinary/api/signing/SignedPayloadValidator.java b/cloudinary-core/src/main/java/com/cloudinary/api/signing/SignedPayloadValidator.java index 40339db6..cf83dc5c 100644 --- a/cloudinary-core/src/main/java/com/cloudinary/api/signing/SignedPayloadValidator.java +++ b/cloudinary-core/src/main/java/com/cloudinary/api/signing/SignedPayloadValidator.java @@ -1,39 +1,27 @@ package com.cloudinary.api.signing; -import com.cloudinary.Util; +import com.cloudinary.Signer; import com.cloudinary.utils.StringUtils; -import java.security.MessageDigest; -import java.security.NoSuchAlgorithmException; - import static com.cloudinary.utils.StringUtils.emptyIfNull; class SignedPayloadValidator { private final String secretKey; - private final MessageDigest messageDigest; + private final Signer signer; - SignedPayloadValidator(String secretKey) { + SignedPayloadValidator(String secretKey, Signer signatureAlgorithmType) { if (StringUtils.isBlank(secretKey)) { throw new IllegalArgumentException("Secret key is required"); } this.secretKey = secretKey; - this.messageDigest = acquireMessageDigest(); + this.signer = signatureAlgorithmType; } boolean validateSignedPayload(String signedPayload, String signature) { String expectedSignature = - StringUtils.encodeHexString( - messageDigest.digest(Util.getUTF8Bytes(emptyIfNull(signedPayload) + secretKey))); + StringUtils.encodeHexString(signer.sign(emptyIfNull(signedPayload) + secretKey)); return expectedSignature.equals(signature); } - - private static MessageDigest acquireMessageDigest() { - try { - return MessageDigest.getInstance("SHA-1"); - } catch (NoSuchAlgorithmException e) { - throw new RuntimeException("Unexpected exception", e); - } - } } diff --git a/cloudinary-core/src/main/java/com/cloudinary/api/signing/package-info.java b/cloudinary-core/src/main/java/com/cloudinary/api/signing/package-info.java index a95f5e74..9c75e3bd 100644 --- a/cloudinary-core/src/main/java/com/cloudinary/api/signing/package-info.java +++ b/cloudinary-core/src/main/java/com/cloudinary/api/signing/package-info.java @@ -2,6 +2,6 @@ * The package holds classes used internally to implement verification procedures of authenticity and integrity of * client communication with Cloudinary servers. Verification is in most cases based on calculating and comparing so called * signatures, or hashed message authentication codes (HMAC) - string values calculated based on message payload, some - * secret key value shared between communicating parties and SHA-1 hashing function. + * secret key value shared between communicating parties and agreed hashing function. */ package com.cloudinary.api.signing; \ No newline at end of file diff --git a/cloudinary-core/src/test/java/com/cloudinary/api/signing/ApiResponseSignatureVerifierTest.java b/cloudinary-core/src/test/java/com/cloudinary/api/signing/ApiResponseSignatureVerifierTest.java index a198b8fc..2c944926 100644 --- a/cloudinary-core/src/test/java/com/cloudinary/api/signing/ApiResponseSignatureVerifierTest.java +++ b/cloudinary-core/src/test/java/com/cloudinary/api/signing/ApiResponseSignatureVerifierTest.java @@ -1,5 +1,6 @@ package com.cloudinary.api.signing; +import com.cloudinary.Signer; import org.junit.Test; import static org.junit.Assert.assertFalse; @@ -23,4 +24,13 @@ public void testVerifySignatureFail() { assertFalse(actual); } + + @Test + public void testVerifySignatureSHA256() { + ApiResponseSignatureVerifier verifier = new ApiResponseSignatureVerifier("X7qLTrsES31MzxxkxPPA-pAGGfU", Signer.SHA256); + + boolean actual = verifier.verifySignature("tests/logo.png", "1", "cc69ae4ed73303fbf4a55f2ae5fc7e34ad3a5c387724bfcde447a2957cacdfea"); + + assertTrue(actual); + } } diff --git a/cloudinary-core/src/test/java/com/cloudinary/api/signing/NotificationRequestSignatureVerifierTest.java b/cloudinary-core/src/test/java/com/cloudinary/api/signing/NotificationRequestSignatureVerifierTest.java index c38bf47a..5a86f9c4 100644 --- a/cloudinary-core/src/test/java/com/cloudinary/api/signing/NotificationRequestSignatureVerifierTest.java +++ b/cloudinary-core/src/test/java/com/cloudinary/api/signing/NotificationRequestSignatureVerifierTest.java @@ -1,5 +1,6 @@ package com.cloudinary.api.signing; +import com.cloudinary.Signer; import org.junit.Test; import static org.junit.Assert.assertFalse; @@ -68,4 +69,16 @@ public void testVerifySignatureFailWhenStillValidButSignatureDoesntMatch() { assertFalse(actual); } + + @Test + public void testVerifySignatureSHA256() { + NotificationRequestSignatureVerifier verifier = new NotificationRequestSignatureVerifier("someApiSecret", Signer.SHA256); + + boolean actual = verifier.verifySignature( + "{}", + "0", + "d5497e1a206ad0ba29ad09a7c0c5f22e939682d15009c15ab3199f62fefbd14b"); + + assertTrue(actual); + } } diff --git a/cloudinary-core/src/test/java/com/cloudinary/test/CloudinaryTest.java b/cloudinary-core/src/test/java/com/cloudinary/test/CloudinaryTest.java index 3f03fef8..df9b30c0 100644 --- a/cloudinary-core/src/test/java/com/cloudinary/test/CloudinaryTest.java +++ b/cloudinary-core/src/test/java/com/cloudinary/test/CloudinaryTest.java @@ -738,6 +738,14 @@ public void testSignedUrl() { assertEquals(expected, actual); } + @Test + public void testSignedUrlSHA256() { + cloudinary.config.signatureAlgorithm = Signer.SHA256; + + String url = cloudinary.url().signed(true).generate("sample.jpg"); + assertEquals(DEFAULT_UPLOAD_PATH + "s--2hbrSMPO--/sample.jpg", url); + } + @Test public void testResponsiveWidth() { // should support responsive width @@ -1312,6 +1320,20 @@ public void testCloudinaryUrlEmptyScheme() { Configuration.from(cloudinaryUrl); } + @Test + public void testApiSignRequestSHA1() { + cloudinary.config.signatureAlgorithm = Signer.SHA1; + String signature = cloudinary.apiSignRequest(ObjectUtils.asMap("cloud_name", "dn6ot3ged", "timestamp", 1568810420, "username", "user@cloudinary.com"), "hdcixPpR2iKERPwqvH6sHdK9cyac"); + assertEquals("14c00ba6d0dfdedbc86b316847d95b9e6cd46d94", signature); + } + + @Test + public void testApiSignRequestSHA256() { + cloudinary.config.signatureAlgorithm = Signer.SHA256; + String signature = cloudinary.apiSignRequest(ObjectUtils.asMap("cloud_name", "dn6ot3ged", "timestamp", 1568810420, "username", "user@cloudinary.com"), "hdcixPpR2iKERPwqvH6sHdK9cyac"); + assertEquals("45ddaa4fa01f0c2826f32f669d2e4514faf275fe6df053f1a150e7beae58a3bd", signature); + } + private void assertFieldsEqual(Object a, Object b) throws IllegalAccessException { assertEquals("Two objects must be the same class", a.getClass(), b.getClass()); Field[] fields = a.getClass().getFields(); @@ -1351,6 +1373,8 @@ private void setRandomValue(Random rand, Field field, Object instance) throws Il Map map = new HashMap(); map.put(cloudinary.randomPublicId(), rand.nextInt()); field.set(instance, map); + } else if (field.get(instance) instanceof Signer) { + field.set(instance, rand.nextBoolean() ? Signer.SHA1 : Signer.SHA256); } else { throw new IllegalArgumentException("Object have unexpected field type, randomizing not supported: " + field.getName() + ", type: " + field.getType().getSimpleName()); } From 54ebecd2083ab885ed09525cf9fe200c4a61fef1 Mon Sep 17 00:00:00 2001 From: RTLCoil Date: Thu, 30 Jul 2020 09:50:53 +0300 Subject: [PATCH 2/2] Fix review comments --- .../java/com/cloudinary/Configuration.java | 12 ++--- .../com/cloudinary/SignatureAlgorithm.java | 19 ++++++++ .../src/main/java/com/cloudinary/Signer.java | 36 --------------- .../src/main/java/com/cloudinary/Url.java | 8 ++-- .../src/main/java/com/cloudinary/Util.java | 44 ++++++++++++++++--- .../signing/ApiResponseSignatureVerifier.java | 14 +++--- .../NotificationRequestSignatureVerifier.java | 10 ++--- .../api/signing/SignedPayloadValidator.java | 12 ++--- .../ApiResponseSignatureVerifierTest.java | 4 +- ...ificationRequestSignatureVerifierTest.java | 4 +- .../com/cloudinary/test/CloudinaryTest.java | 14 +++--- 11 files changed, 101 insertions(+), 76 deletions(-) create mode 100644 cloudinary-core/src/main/java/com/cloudinary/SignatureAlgorithm.java delete mode 100644 cloudinary-core/src/main/java/com/cloudinary/Signer.java diff --git a/cloudinary-core/src/main/java/com/cloudinary/Configuration.java b/cloudinary-core/src/main/java/com/cloudinary/Configuration.java index 42205f92..b21aa799 100644 --- a/cloudinary-core/src/main/java/com/cloudinary/Configuration.java +++ b/cloudinary-core/src/main/java/com/cloudinary/Configuration.java @@ -20,7 +20,7 @@ public class Configuration { public final static String VERSION = "1.0.2"; public final static String USER_AGENT = "cld-android-" + VERSION; public static final boolean DEFAULT_IS_LONG_SIGNATURE = false; - public static final Signer DEFAULT_SIGNATURE_ALGORITHM = Signer.SHA1; + public static final SignatureAlgorithm DEFAULT_SIGNATURE_ALGORITHM = SignatureAlgorithm.SHA1; private static final String CONFIG_PROP_SIGNATURE_ALGORITHM = "signature_algorithm"; @@ -46,7 +46,7 @@ public class Configuration { public AuthToken authToken; public boolean forceVersion = true; public boolean longUrlSignature = DEFAULT_IS_LONG_SIGNATURE; - public Signer signatureAlgorithm = DEFAULT_SIGNATURE_ALGORITHM; + public SignatureAlgorithm signatureAlgorithm = DEFAULT_SIGNATURE_ALGORITHM; public Configuration() { } @@ -71,7 +71,7 @@ private Configuration( boolean loadStrategies, boolean forceVersion, boolean longUrlSignature, - Signer signatureAlgorithm) { + SignatureAlgorithm signatureAlgorithm) { this.cloudName = cloudName; this.apiKey = apiKey; this.apiSecret = apiSecret; @@ -129,7 +129,7 @@ public void update(Map config) { this.properties.putAll(properties); } this.longUrlSignature = ObjectUtils.asBoolean(config.get("long_url_signature"), DEFAULT_IS_LONG_SIGNATURE); - this.signatureAlgorithm = Signer.getByName(ObjectUtils.asString(config.get(CONFIG_PROP_SIGNATURE_ALGORITHM), DEFAULT_SIGNATURE_ALGORITHM.toString())); + this.signatureAlgorithm = SignatureAlgorithm.valueOf(ObjectUtils.asString(config.get(CONFIG_PROP_SIGNATURE_ALGORITHM), DEFAULT_SIGNATURE_ALGORITHM.name())); } @SuppressWarnings("rawtypes") @@ -300,7 +300,7 @@ public static class Builder { private AuthToken authToken; private boolean forceVersion = true; private boolean longUrlSignature = DEFAULT_IS_LONG_SIGNATURE; - private Signer signatureAlgorithm = DEFAULT_SIGNATURE_ALGORITHM; + private SignatureAlgorithm signatureAlgorithm = DEFAULT_SIGNATURE_ALGORITHM; /** * Set the HTTP connection timeout. @@ -461,7 +461,7 @@ public Builder setIsLongUrlSignature(boolean isLong) { return this; } - public Builder setSignatureAlgorithm(Signer signatureAlgorithm) { + public Builder setSignatureAlgorithm(SignatureAlgorithm signatureAlgorithm) { this.signatureAlgorithm = signatureAlgorithm; return this; } diff --git a/cloudinary-core/src/main/java/com/cloudinary/SignatureAlgorithm.java b/cloudinary-core/src/main/java/com/cloudinary/SignatureAlgorithm.java new file mode 100644 index 00000000..c96fb1b3 --- /dev/null +++ b/cloudinary-core/src/main/java/com/cloudinary/SignatureAlgorithm.java @@ -0,0 +1,19 @@ +package com.cloudinary; + +/** + * Defines supported algorithms for generating/verifying hashed message authentication codes (HMAC). + */ +public enum SignatureAlgorithm { + SHA1("SHA-1"), + SHA256("SHA-256"); + + private final String algorithmId; + + SignatureAlgorithm(String algorithmId) { + this.algorithmId = algorithmId; + } + + public String getAlgorithmId() { + return algorithmId; + } +} diff --git a/cloudinary-core/src/main/java/com/cloudinary/Signer.java b/cloudinary-core/src/main/java/com/cloudinary/Signer.java deleted file mode 100644 index 0a216de9..00000000 --- a/cloudinary-core/src/main/java/com/cloudinary/Signer.java +++ /dev/null @@ -1,36 +0,0 @@ -package com.cloudinary; - -import java.security.MessageDigest; -import java.security.NoSuchAlgorithmException; - -public class Signer { - private final String algorithmName; - - private Signer(String algorithmName) { - this.algorithmName = algorithmName; - } - - public byte[] sign(String s) { - try { - return MessageDigest.getInstance(algorithmName).digest(Util.getUTF8Bytes(s)); - } catch (NoSuchAlgorithmException e) { - throw new RuntimeException("Unexpected exception", e); - } - } - - public static final Signer SHA1 = new Signer("SHA-1"); - public static final Signer SHA256 = new Signer("SHA-256"); - - public static Signer getByName(String algorithmName) { - if ("SHA-256".equals(algorithmName)) { - return Signer.SHA256; - } - - return Signer.SHA1; - } - - @Override - public String toString() { - return algorithmName; - } -} diff --git a/cloudinary-core/src/main/java/com/cloudinary/Url.java b/cloudinary-core/src/main/java/com/cloudinary/Url.java index 612a7062..fad9ece8 100644 --- a/cloudinary-core/src/main/java/com/cloudinary/Url.java +++ b/cloudinary-core/src/main/java/com/cloudinary/Url.java @@ -17,6 +17,8 @@ import com.cloudinary.utils.ObjectUtils; import com.cloudinary.utils.StringUtils; +import static com.cloudinary.SignatureAlgorithm.SHA256; + public class Url { private final Cloudinary cloudinary; private final Configuration config; @@ -387,14 +389,14 @@ public String generate(String source) { if (signUrl && (authToken == null || authToken.equals(AuthToken.NULL_AUTH_TOKEN))) { - Signer signer = longUrlSignature ? Signer.SHA256 : config.signatureAlgorithm; + SignatureAlgorithm signatureAlgorithm = longUrlSignature ? SHA256 : config.signatureAlgorithm; String toSign = StringUtils.join(new String[]{transformationStr, sourceToSign}, "/"); toSign = StringUtils.removeStartingChars(toSign, '/'); toSign = StringUtils.mergeSlashesInUrl(toSign); - byte[] digest = signer.sign(toSign + this.config.apiSecret); - signature = Base64Coder.encodeURLSafeString(digest); + byte[] hash = Util.hash(toSign + this.config.apiSecret, signatureAlgorithm); + signature = Base64Coder.encodeURLSafeString(hash); signature = "s--" + signature.substring(0, longUrlSignature ? 32 : 8) + "--"; } diff --git a/cloudinary-core/src/main/java/com/cloudinary/Util.java b/cloudinary-core/src/main/java/com/cloudinary/Util.java index 4b892e4a..2c9d217c 100644 --- a/cloudinary-core/src/main/java/com/cloudinary/Util.java +++ b/cloudinary-core/src/main/java/com/cloudinary/Util.java @@ -4,6 +4,8 @@ import com.cloudinary.utils.StringUtils; import org.cloudinary.json.JSONObject; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; import java.util.*; public class Util { @@ -321,7 +323,7 @@ public static byte[] getUTF8Bytes(String string) { /** * Calculates signature, or hashed message authentication code (HMAC) of provided parameters name-value pairs and - * secret value using supported hashing algorithm. + * secret value using default hashing algorithm (SHA1). *

* Argument for hashing function is built by joining sorted parameter name-value pairs into single string in the * same fashion as HTTP GET method uses, and concatenating the result with secret value in the end. Method supports @@ -330,10 +332,27 @@ public static byte[] getUTF8Bytes(String string) { * * @param paramsToSign parameter name-value pairs list represented as instance of {@link Map} * @param apiSecret secret value - * @param algorithmType type of hashing algorithm to use for calculation of HMAC * @return hex-string representation of signature calculated based on provided parameters map and secret */ - public static String produceSignature(Map paramsToSign, String apiSecret, Signer algorithmType) { + public static String produceSignature(Map paramsToSign, String apiSecret) { + return produceSignature(paramsToSign, apiSecret, SignatureAlgorithm.SHA1); + } + + /** + * Calculates signature, or hashed message authentication code (HMAC) of provided parameters name-value pairs and + * secret value using specified hashing algorithm. + *

+ * Argument for hashing function is built by joining sorted parameter name-value pairs into single string in the + * same fashion as HTTP GET method uses, and concatenating the result with secret value in the end. Method supports + * arrays/collections as parameter values. In this case, the elements of array/collection are joined into single + * comma-delimited string prior to inclusion into the result. + * + * @param paramsToSign parameter name-value pairs list represented as instance of {@link Map} + * @param apiSecret secret value + * @param signatureAlgorithm type of hashing algorithm to use for calculation of HMAC + * @return hex-string representation of signature calculated based on provided parameters map and secret + */ + public static String produceSignature(Map paramsToSign, String apiSecret, SignatureAlgorithm signatureAlgorithm) { Collection params = new ArrayList(); for (Map.Entry param : new TreeMap(paramsToSign).entrySet()) { if (param.getValue() instanceof Collection) { @@ -347,7 +366,22 @@ public static String produceSignature(Map paramsToSign, String a } } String to_sign = StringUtils.join(params, "&"); - byte[] digest = algorithmType.sign(to_sign + apiSecret); - return StringUtils.encodeHexString(digest); + byte[] hash = Util.hash(to_sign + apiSecret, signatureAlgorithm); + return StringUtils.encodeHexString(hash); + } + + /** + * Computes hash from input string using specified algorithm. + * + * @param input string which to compute hash from + * @param signatureAlgorithm algorithm to use for computing hash + * @return array of bytes of computed hash value + */ + public static byte[] hash(String input, SignatureAlgorithm signatureAlgorithm) { + try { + return MessageDigest.getInstance(signatureAlgorithm.getAlgorithmId()).digest(Util.getUTF8Bytes(input)); + } catch (NoSuchAlgorithmException e) { + throw new RuntimeException("Unexpected exception", e); + } } } diff --git a/cloudinary-core/src/main/java/com/cloudinary/api/signing/ApiResponseSignatureVerifier.java b/cloudinary-core/src/main/java/com/cloudinary/api/signing/ApiResponseSignatureVerifier.java index 29eacdf4..1dbae00d 100644 --- a/cloudinary-core/src/main/java/com/cloudinary/api/signing/ApiResponseSignatureVerifier.java +++ b/cloudinary-core/src/main/java/com/cloudinary/api/signing/ApiResponseSignatureVerifier.java @@ -1,6 +1,6 @@ package com.cloudinary.api.signing; -import com.cloudinary.Signer; +import com.cloudinary.SignatureAlgorithm; import com.cloudinary.Util; import com.cloudinary.utils.ObjectUtils; import com.cloudinary.utils.StringUtils; @@ -12,7 +12,7 @@ */ public class ApiResponseSignatureVerifier { private final String secretKey; - private final Signer signatureAlgorithmType; + private final SignatureAlgorithm signatureAlgorithm; /** * Initializes new instance of {@code ApiResponseSignatureVerifier} class with a secret key required to perform @@ -26,7 +26,7 @@ public ApiResponseSignatureVerifier(String secretKey) { } this.secretKey = secretKey; - this.signatureAlgorithmType = Signer.SHA1; + this.signatureAlgorithm = SignatureAlgorithm.SHA1; } /** @@ -34,15 +34,15 @@ public ApiResponseSignatureVerifier(String secretKey) { * API response signatures verification. * * @param secretKey shared secret key string which is used to sign and verify authenticity of API responses - * @param algorithmType type of hashing algorithm to use for calculation of HMACs + * @param signatureAlgorithm type of hashing algorithm to use for calculation of HMACs */ - public ApiResponseSignatureVerifier(String secretKey, Signer algorithmType) { + public ApiResponseSignatureVerifier(String secretKey, SignatureAlgorithm signatureAlgorithm) { if (StringUtils.isBlank(secretKey)) { throw new IllegalArgumentException("Secret key is required"); } this.secretKey = secretKey; - signatureAlgorithmType = algorithmType; + this.signatureAlgorithm = signatureAlgorithm; } /** @@ -60,6 +60,6 @@ public ApiResponseSignatureVerifier(String secretKey, Signer algorithmType) { public boolean verifySignature(String publicId, String version, String signature) { return Util.produceSignature(ObjectUtils.asMap( "public_id", emptyIfNull(publicId), - "version", emptyIfNull(version)), secretKey, signatureAlgorithmType).equals(signature); + "version", emptyIfNull(version)), secretKey, signatureAlgorithm).equals(signature); } } diff --git a/cloudinary-core/src/main/java/com/cloudinary/api/signing/NotificationRequestSignatureVerifier.java b/cloudinary-core/src/main/java/com/cloudinary/api/signing/NotificationRequestSignatureVerifier.java index a3d33372..62ad579b 100644 --- a/cloudinary-core/src/main/java/com/cloudinary/api/signing/NotificationRequestSignatureVerifier.java +++ b/cloudinary-core/src/main/java/com/cloudinary/api/signing/NotificationRequestSignatureVerifier.java @@ -1,6 +1,6 @@ package com.cloudinary.api.signing; -import com.cloudinary.Signer; +import com.cloudinary.SignatureAlgorithm; import static com.cloudinary.utils.StringUtils.emptyIfNull; @@ -17,17 +17,17 @@ public class NotificationRequestSignatureVerifier { * @param secretKey shared secret key string which is used to sign and verify authenticity of notifications */ public NotificationRequestSignatureVerifier(String secretKey) { - this.signedPayloadValidator = new SignedPayloadValidator(secretKey, Signer.SHA1); + this.signedPayloadValidator = new SignedPayloadValidator(secretKey, SignatureAlgorithm.SHA1); } /** * Initializes new instance of {@code NotificationRequestSignatureVerifier} with secret key value. * * @param secretKey shared secret key string which is used to sign and verify authenticity of notifications - * @param signatureAlgorithmType type of hashing algorithm to use for calculation of HMACs + * @param signatureAlgorithm type of hashing algorithm to use for calculation of HMACs */ - public NotificationRequestSignatureVerifier(String secretKey, Signer signatureAlgorithmType) { - this.signedPayloadValidator = new SignedPayloadValidator(secretKey, signatureAlgorithmType); + public NotificationRequestSignatureVerifier(String secretKey, SignatureAlgorithm signatureAlgorithm) { + this.signedPayloadValidator = new SignedPayloadValidator(secretKey, signatureAlgorithm); } /** diff --git a/cloudinary-core/src/main/java/com/cloudinary/api/signing/SignedPayloadValidator.java b/cloudinary-core/src/main/java/com/cloudinary/api/signing/SignedPayloadValidator.java index cf83dc5c..771bdbe0 100644 --- a/cloudinary-core/src/main/java/com/cloudinary/api/signing/SignedPayloadValidator.java +++ b/cloudinary-core/src/main/java/com/cloudinary/api/signing/SignedPayloadValidator.java @@ -1,26 +1,28 @@ package com.cloudinary.api.signing; -import com.cloudinary.Signer; +import com.cloudinary.SignatureAlgorithm; +import com.cloudinary.Util; import com.cloudinary.utils.StringUtils; import static com.cloudinary.utils.StringUtils.emptyIfNull; class SignedPayloadValidator { private final String secretKey; - private final Signer signer; + private final SignatureAlgorithm signatureAlgorithm; - SignedPayloadValidator(String secretKey, Signer signatureAlgorithmType) { + SignedPayloadValidator(String secretKey, SignatureAlgorithm signatureAlgorithm) { if (StringUtils.isBlank(secretKey)) { throw new IllegalArgumentException("Secret key is required"); } this.secretKey = secretKey; - this.signer = signatureAlgorithmType; + this.signatureAlgorithm = signatureAlgorithm; } boolean validateSignedPayload(String signedPayload, String signature) { String expectedSignature = - StringUtils.encodeHexString(signer.sign(emptyIfNull(signedPayload) + secretKey)); + StringUtils.encodeHexString(Util.hash(emptyIfNull(signedPayload) + secretKey, + signatureAlgorithm)); return expectedSignature.equals(signature); } diff --git a/cloudinary-core/src/test/java/com/cloudinary/api/signing/ApiResponseSignatureVerifierTest.java b/cloudinary-core/src/test/java/com/cloudinary/api/signing/ApiResponseSignatureVerifierTest.java index 2c944926..5449f555 100644 --- a/cloudinary-core/src/test/java/com/cloudinary/api/signing/ApiResponseSignatureVerifierTest.java +++ b/cloudinary-core/src/test/java/com/cloudinary/api/signing/ApiResponseSignatureVerifierTest.java @@ -1,6 +1,6 @@ package com.cloudinary.api.signing; -import com.cloudinary.Signer; +import com.cloudinary.SignatureAlgorithm; import org.junit.Test; import static org.junit.Assert.assertFalse; @@ -27,7 +27,7 @@ public void testVerifySignatureFail() { @Test public void testVerifySignatureSHA256() { - ApiResponseSignatureVerifier verifier = new ApiResponseSignatureVerifier("X7qLTrsES31MzxxkxPPA-pAGGfU", Signer.SHA256); + ApiResponseSignatureVerifier verifier = new ApiResponseSignatureVerifier("X7qLTrsES31MzxxkxPPA-pAGGfU", SignatureAlgorithm.SHA256); boolean actual = verifier.verifySignature("tests/logo.png", "1", "cc69ae4ed73303fbf4a55f2ae5fc7e34ad3a5c387724bfcde447a2957cacdfea"); diff --git a/cloudinary-core/src/test/java/com/cloudinary/api/signing/NotificationRequestSignatureVerifierTest.java b/cloudinary-core/src/test/java/com/cloudinary/api/signing/NotificationRequestSignatureVerifierTest.java index 5a86f9c4..a5d2e096 100644 --- a/cloudinary-core/src/test/java/com/cloudinary/api/signing/NotificationRequestSignatureVerifierTest.java +++ b/cloudinary-core/src/test/java/com/cloudinary/api/signing/NotificationRequestSignatureVerifierTest.java @@ -1,6 +1,6 @@ package com.cloudinary.api.signing; -import com.cloudinary.Signer; +import com.cloudinary.SignatureAlgorithm; import org.junit.Test; import static org.junit.Assert.assertFalse; @@ -72,7 +72,7 @@ public void testVerifySignatureFailWhenStillValidButSignatureDoesntMatch() { @Test public void testVerifySignatureSHA256() { - NotificationRequestSignatureVerifier verifier = new NotificationRequestSignatureVerifier("someApiSecret", Signer.SHA256); + NotificationRequestSignatureVerifier verifier = new NotificationRequestSignatureVerifier("someApiSecret", SignatureAlgorithm.SHA256); boolean actual = verifier.verifySignature( "{}", diff --git a/cloudinary-core/src/test/java/com/cloudinary/test/CloudinaryTest.java b/cloudinary-core/src/test/java/com/cloudinary/test/CloudinaryTest.java index df9b30c0..9c4e508c 100644 --- a/cloudinary-core/src/test/java/com/cloudinary/test/CloudinaryTest.java +++ b/cloudinary-core/src/test/java/com/cloudinary/test/CloudinaryTest.java @@ -740,7 +740,7 @@ public void testSignedUrl() { @Test public void testSignedUrlSHA256() { - cloudinary.config.signatureAlgorithm = Signer.SHA256; + cloudinary.config.signatureAlgorithm = SignatureAlgorithm.SHA256; String url = cloudinary.url().signed(true).generate("sample.jpg"); assertEquals(DEFAULT_UPLOAD_PATH + "s--2hbrSMPO--/sample.jpg", url); @@ -1322,14 +1322,14 @@ public void testCloudinaryUrlEmptyScheme() { @Test public void testApiSignRequestSHA1() { - cloudinary.config.signatureAlgorithm = Signer.SHA1; + cloudinary.config.signatureAlgorithm = SignatureAlgorithm.SHA1; String signature = cloudinary.apiSignRequest(ObjectUtils.asMap("cloud_name", "dn6ot3ged", "timestamp", 1568810420, "username", "user@cloudinary.com"), "hdcixPpR2iKERPwqvH6sHdK9cyac"); assertEquals("14c00ba6d0dfdedbc86b316847d95b9e6cd46d94", signature); } @Test public void testApiSignRequestSHA256() { - cloudinary.config.signatureAlgorithm = Signer.SHA256; + cloudinary.config.signatureAlgorithm = SignatureAlgorithm.SHA256; String signature = cloudinary.apiSignRequest(ObjectUtils.asMap("cloud_name", "dn6ot3ged", "timestamp", 1568810420, "username", "user@cloudinary.com"), "hdcixPpR2iKERPwqvH6sHdK9cyac"); assertEquals("45ddaa4fa01f0c2826f32f669d2e4514faf275fe6df053f1a150e7beae58a3bd", signature); } @@ -1373,10 +1373,14 @@ private void setRandomValue(Random rand, Field field, Object instance) throws Il Map map = new HashMap(); map.put(cloudinary.randomPublicId(), rand.nextInt()); field.set(instance, map); - } else if (field.get(instance) instanceof Signer) { - field.set(instance, rand.nextBoolean() ? Signer.SHA1 : Signer.SHA256); + } else if (fieldType instanceof Class && Enum.class.isAssignableFrom((Class) fieldType)) { + field.set(instance, randomEnum((Class) fieldType, rand)); } else { throw new IllegalArgumentException("Object have unexpected field type, randomizing not supported: " + field.getName() + ", type: " + field.getType().getSimpleName()); } } + + private > T randomEnum(Class clazz, Random random) { + return clazz.getEnumConstants()[random.nextInt(clazz.getEnumConstants().length)]; + } }