SecretKeySpec: SignatureAlgorithm ignored -- check broken

[2019-05-10]

jjwt 0.11.2

• create a token with a SecretKeySpec:
  new SecretKeySpec(MyKeyString, SignatureAlgorithm.HS256.getJcaName())
• check created token and the SignatureAlgorithm is HS384 instead.

If I look at io.jsonwebtoken.forSigningKey(Key key) the SignatureAlgorithm is completely ignored -- instead some length test decides what algorithm is to be used.
So, inevitably, the check in the JWTParser fails with a baffling error!

How is that even possible?

• why is an algorithm even required if it is not used?
• why is it not used?
• what's the idea in calculating an algorithm which by necessity will break the token check on the other end where the user will obviously expect the very same algorithm set when creating the token?

[2019-05-10]

Actually, it's even worse:
Error processing validation response for: ... The verification key's size is 328 bits which is not secure enough for the HS384 algorithm.

So, obviously the Parser does not even arrive at the same algorithm as the Signer, despite using the very same key!

[bdemers]

Hi @2019-05-10,

I'm not sure I'm following your description, can you add a quick snippet to help us reproduce the issue?

I took a quick pass at creating an example, but this works as expected:

// populate random bytes
byte[] secretBytes = new byte[32];
new SecureRandom().nextBytes(secretBytes);

// generate key using keyspec
Key secretKey1 = new SecretKeySpec(secretBytes, SignatureAlgorithm.HS256.getJcaName());
SignatureAlgorithm algorithm1 = SignatureAlgorithm.forSigningKey(secretKey1);
System.out.println("alg for key1: " + algorithm1);

// generate key using util method
Key secretKey2 = Keys.hmacShaKeyFor(secretBytes);
SignatureAlgorithm algorithm2 = SignatureAlgorithm.forSigningKey(secretKey2);
System.out.println("alg for key2: " + algorithm2);

// show key equality
System.out.println("Keys are equal: " + secretKey1.equals(secretKey2));

[2019-05-10]

Of course it would, since you are deliberately limiting the key's length to 32.

final String key = "YWJjZGVmZ2hpamtsbW5vcHFyZXN0dXZ3eHl6QUJDREVGR0hJSktMTU5PUFFSU1RVVldYWVoK";
final ZonedDateTime now = ZonedDateTime.now();
final String jwt = Jwts.builder()
        .setClaims(Jwts.claims().setSubject("2022030209").setAudience("Foo"))
        .setIssuedAt(Date.from(tNow.toInstant()))
        .setExpiration(Date.from(tNow.plus(6000, ChronoUnit.SECONDS).toInstant()))
        .signWith(new SecretKeySpec(Decoders.BASE64.decode(key), SignatureAlgorithm.HS256.getJcaName()))
        .compact();

results in the following token
eyJhbGciOiJIUzM4NCJ9.eyJzdWIiOiIyMDIyMDMwMjA5IiwiYXVkIjoiRm9vIiwiaWF0IjoxNjQ2MjA3OTE1LCJleHAiOjE2NDYyMTM5MTV9.eVB7UzKMaQZyjN3-plGsQeMo63uM9ATayalh1c2wbmoRdEsOlbMT3LaFym40rj-O
which is, as a check reveals, created with HS384 instead of the stated HS256.

Have a look at SignatureAlgorithm forSigningKey(Key) and you'll see that the key's algorithm doesn't matter at all. Instead the code mindlessly checks the length of the key and thus arrives at the wrong algorithm.

Why public JwtBuilder signWith(Key key) invokes SignatureAlgorithm forSigningKey(Key) at all instead of checking the Key for an algorithm first is a mystery to me.

[2019-05-10]

Scratch that last paragraph about signWith(Key) -- forSigningKey(Key) is supposed to do that check.

[bdemers]

@2019-05-10 ahh, thanks, i see what you mean now.

[stale]

This issue has been automatically marked as stale due to inactivity for 60 or more days. It will be closed in 7 days if no further activity occurs.

[lhazlewood]

This was resolved via 620cc5d:

jjwt/impl/src/main/java/io/jsonwebtoken/impl/security/DefaultMacAlgorithm.java

Lines 56 to 95 in 26026d6

	static {
	JCA_NAME_MAP = new LinkedHashMap<>(6);
	// In addition to JCA names, PKCS12 OIDs are added to these per
	// https://bugs.openjdk.java.net/browse/JDK-8243551 as well:
	JCA_NAME_MAP.put(HS256.getJcaName().toUpperCase(Locale.ENGLISH), HS256); // for case-insensitive lookup
	JCA_NAME_MAP.put(HS256_OID, HS256);
	JCA_NAME_MAP.put(HS384.getJcaName().toUpperCase(Locale.ENGLISH), HS384);
	JCA_NAME_MAP.put(HS384_OID, HS384);
	JCA_NAME_MAP.put(HS512.getJcaName().toUpperCase(Locale.ENGLISH), HS512);
	JCA_NAME_MAP.put(HS512_OID, HS512);
	}
	private final int minKeyBitLength; //in bits
	private DefaultMacAlgorithm(int digestBitLength) {
	this("HS" + digestBitLength, "HmacSHA" + digestBitLength, digestBitLength);
	}
	DefaultMacAlgorithm(String id, String jcaName, int minKeyBitLength) {
	super(id, jcaName);
	Assert.isTrue(minKeyBitLength > 0, "minKeyLength must be greater than zero.");
	this.minKeyBitLength = minKeyBitLength;
	}
	@Override
	public int getKeyBitLength() {
	return this.minKeyBitLength;
	}
	private boolean isJwaStandard() {
	return JWA_STANDARD_IDS.contains(getId());
	}
	private static boolean isJwaStandardJcaName(String jcaName) {
	String key = jcaName.toUpperCase(Locale.ENGLISH);
	return JCA_NAME_MAP.containsKey(key);
	}

[lhazlewood]

Closing per 620cc5d and that the old SignatureAlgorithm enum is deprecated. mac algorithm lookup in master is now:

https://github.com/jwtk/jjwt/blame/26026d63cdf2a07c058695894ba1a983e1bb8cea/impl/src/main/java/io/jsonwebtoken/impl/security/DefaultMacAlgorithm.java#L97-L119