com.nimbusds.oauth2.sdk.auth.Secret inconsitent

[Rudy De Busscher]

There are a few issues with the Secret class.

Secret secret = new Secret(48);
System.out.println(secret.getValueBytes().length);

So generating a secret with a byte length of 48 returns a byte Array of length 64 because the byteArray from SecureRandom is Base64 encoded before assigned to value property.

But I have a blocking issue in the combination with ClientCredentialsSelector

Since the client authentication for the token Endpoint (as signed JWT) is performed client side with the byteArray and the server has only the Base64Encoded value. I have no way of supplying the Base64Encoded value to Secret and the ClientAuthentication validation will use the base64Decoded byte array.

So ClientAuthentication with always fail

[Rudy De Busscher]

Proposal, as patch file

Index: src/main/java/com/nimbusds/oauth2/sdk/auth/Secret.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- src/main/java/com/nimbusds/oauth2/sdk/auth/Secret.java (revision 7e31102eee7acdd623934d3a28b1588ceb687942) +++ src/main/java/com/nimbusds/oauth2/sdk/auth/Secret.java (revision ) @@ -23,6 +23,7 @@ import java.util.Arrays; import java.util.Date;

+import com.nimbusds.jose.util.Base64; import com.nimbusds.jose.util.Base64URL; import net.jcip.annotations.Immutable;

@@ -224,4 +225,10 @@ public int hashCode() { return Arrays.hashCode(value); } + + public static Secret forBase64EncodedString(final String base64Encoded) { + Secret result = new Secret("X"); // Alternative is to create a constructor for byte[] and expDate + result.value = new Base64(base64Encoded).decode(); + return result; + } } \ No newline at end of file

[Vladimir Dzhuvinov]

Thanks for the feedback.

So, if I understand the issue correctly, you need a way to get at the secret bytes, after base64 decoding, and not as the string representation of the base64?

Reposting your patch for better readability:

Index: src/main/java/com/nimbusds/oauth2/sdk/auth/Secret.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- src/main/java/com/nimbusds/oauth2/sdk/auth/Secret.java  (revision 7e31102eee7acdd623934d3a28b1588ceb687942)
+++ src/main/java/com/nimbusds/oauth2/sdk/auth/Secret.java  (revision )
@@ -23,6 +23,7 @@
 import java.util.Arrays;
 import java.util.Date;

+import com.nimbusds.jose.util.Base64;
 import com.nimbusds.jose.util.Base64URL;
 import net.jcip.annotations.Immutable;

@@ -224,4 +225,10 @@
    public int hashCode() {
        return Arrays.hashCode(value);
    }
+
+   public static Secret forBase64EncodedString(final String base64Encoded) {
+       Secret result = new Secret("X");  // Alternative is to create a constructor for byte[] and expDate
+       result.value = new Base64(base64Encoded).decode();
+       return result;
+   }
 }
\ No newline at end of file

[Rudy De Busscher]

Yes,

One can assume that when you ask for a secret of length 48, you get a byte array of length 48.

And also important, a way to instantiate the a Secret instance based on a BASE64 encoded value.

Since cryptographic signatures MUST ALWAYS be calculated based on the byte array and never on the BASE64 encoded string value. See also this comment on another project but related to JWT https://github.com/auth0/node-jsonwebtoken/issues/208#issuecomment-231861138

[Vladimir Dzhuvinov]

Hi Rudy,

I had a look at the Secret code.

base64url encoding is applied to the generated random byte array so that the secret can be printed out safely. Otherwise, if you generate a random byte array and convert it to UTF-8 directly, you will get gibberish on the screen. The secret string isn't truncated, because the underlying randomness that was specified must be preserved. A string consisting of 32 random printable chars doesn't have 32-byte randomness. For this reason the constructor should be left the way it is, otherwise randomness will be compromised.

As for secret usage in HMAC keys for JWTs, the OIDC spec requires the UTF-8 chars to be converted to their byte representation. So base64url decoding is not expected.

http://openid.net/specs/openid-connect-core-1_0.html#Signing

[Vladimir Dzhuvinov]

From http://openid.net/specs/openid-connect-core-1_0.html#SymmetricKeyEntropy

6.19. Symmetric Key Entropy

In Section 10.1 and Section 10.2, keys are derived from the client_secret value. Thus, when used with symmetric signing or encryption operations, client_secret values MUST contain sufficient entropy to generate cryptographically strong keys. Also, client_secret values MUST also contain at least the minimum of number of octets required for MAC keys for the particular algorithm used. So for instance, for HS256, the client_secret value MUST contain at least 32 octets (and almost certainly SHOULD contain more, since client_secret values are likely to use a restricted alphabet).

[Vladimir Dzhuvinov]

Not adding base64url / byte array constructor as the secrets are normally specified by strings (UTF-8).