Subject: [jose] implementing JOSE in Java: jsoncrypto

From: <[email-masked]>
Date: Wed, 1 Aug 2012 22:37:52 +0200

I moved my JOSE implementation out of my openinfocard project into a new repository:
https://code.google.com/p/jsoncrypto/

One way to dive into this API is to look at the JUNIT test cases. They go through all the API calls and test the examples provided in the specs.
https://code.google.com/p/jsoncrypto/source/browse/trunk/testsrc/org/xmldap/json/WebTokenTest.java

Axel

https://code.google.com/p/jsoncrypto/source/browse/trunk/src/org/xmldap/json/WebToken.java

---

From: "Manger, James H" <[email-masked]>
Date: Thu, 2 Aug 2012 11:26:20 +1000

Axel,

> https://code.google.com/p/jsoncrypto/

It is good to see another JOSE implementation.

Does jsoncrypto implement the "MUST understand everything" rule?

  [JWE-05, section 4]
  Implementations MUST understand the entire contents of the
  header; otherwise, the JWE MUST be rejected.

  [JWE-05, section 6]
  4. The resulting JWE Header MUST be validated to only include
     parameters and values whose syntax and semantics are both
     understood and supported.

Jsoncrypto does not appear to implement this rule. It appears to take the obvious approach of looking in the header for the fields it needs -- and ignoring anything else that might be there.

[Regardless of any possible merit in a "MUST understand everything" rule, the fact that it will often not be implemented seems like a really good reason to drop the rule so the spec reflects reality.]

--
James Manger

---

From: <[email-masked]>
Date: Thu, 2 Aug 2012 06:55:30 +0200

I wanted to wait a little more before implementing that because the discussion here about it seems to bring too many changes to the code.
There are other things too that need to be improved. But jsoncrypto implements all proposed alg and enc values other than none (which I find strange) and dir (which I had implemented first but the group at that time seemed to think that this might make it too easy for developers to use the same cek too often (always)).

-----Original Message-----
From: Manger, James H [mailto:[email-masked]] 
Sent: Thursday, August 02, 2012 3:26 AM
To: Nennker, Axel; [email-masked]
Subject: RE: implementing JOSE in Java: jsoncrypto

Axel,

  [JWE-05, section 4]

  [JWE-05, section 6]

--
James Manger

---

From: <[email-masked]>
Date: Thu, 2 Aug 2012 11:14:03 +0200

Done.
checkHeader function is called in all encrypt, decrypt, sign and verify functions now.

Please keep those improvement suggestions coming. This code is not final.

Axel

       public static void checkHeader(JSONObject jweHeader) throws Exception {
                @SuppressWarnings("rawtypes")
                Iterator iter = jweHeader.keys();
                while (iter.hasNext()) {
                        String key = (String) iter.next();
                        if ("alg".equals(key)) continue;
                        if ("enc".equals(key)) continue;
                        if ("int".equals(key)) continue;
                        if ("kdf".equals(key)) continue;
                        if ("iv".equals(key)) continue;
                        if ("epk".equals(key)) continue;
                        if ("zip".equals(key)) continue;
                        if ("jku".equals(key)) continue;
                        if ("jwk".equals(key)) continue;
                        if ("x5u".equals(key)) continue;
                        if ("x5t".equals(key)) continue;
                        if ("x5c".equals(key)) continue;
                        if ("kid".equals(key)) continue;
                        if ("typ".equals(key)) continue;
                        if ("cty".equals(key)) continue;
                        if ("jku".equals(key)) continue;
                        if ("jwk".equals(key)) continue;
                        throw new Exception("unknown key in header: " + key);
                }
        }

-----Original Message-----
From: Manger, James H [mailto:[email-masked]] 
Sent: Thursday, August 02, 2012 3:26 AM
To: Nennker, Axel; [email-masked]
Subject: RE: implementing JOSE in Java: jsoncrypto

Axel,

  [JWE-05, section 4]

  [JWE-05, section 6]

--
James Manger
_______________________________________________
jose mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/jose

---

From: "Manger, James H" <[email-masked]>
Date: Thu, 2 Aug 2012 23:54:28 +1000

> checkHeader function is called in all encrypt, decrypt, sign and verify

That was pretty quick, but also petty awful.
If an app using your library now receives a JWE with, say, a "creationTime" field (suggested at the IETF meeting I think) it will fail unnecessarily. The app needs to modify the *library source code* and recompile to continue.

checkHeader accepts a "zip" field, but I don't think the jsoncrypto code implements this field (same for "kdf", "typ" etc).

checkHeader accepts an "epk" field when "alg" is "RSA-OAEP" even though it cannot understand such a combination because it hasn't been defined.

checkHeader accepts "kdf" even if its value is an object (eg "kdf":{"xyz":...}) instead of having a string value. Again, who knows what that might mean?

You have introduced some more failure modes. There are still plenty of JOSE message that your code will accept despite not understanding. Your library was more useful without checkHeader().

P.S. Slightly better code might be:
if (!knownKeysSet.containsAll(jweHeader.keys()) throw SomeException(...)

--
James Manger

> I wanted to wait a little more before implementing that because the
> discussion here about it seems to bring too many changes to the code.
> There are other things too that need to be improved. But jsoncrypto
> implements all proposed alg and enc values other than none (which I
> find strange) and dir (which I had implemented first but the group at
> that time seemed to think that this might make it too easy for
> developers to use the same cek too often (always)).
> 
> Jsoncrypto does not appear to implement this rule. It appears to take
> the obvious approach of looking in the header for the fields it needs -
> - and ignoring anything else that might be there.
> 
> [Regardless of any possible merit in a "MUST understand everything"
> rule, the fact that it will often not be implemented seems like a
> really good reason to drop the rule so the spec reflects reality.]
> 
> --
> James Manger

---

From: <[email-masked]>
Date: Thu, 2 Aug 2012 17:57:52 +0200

That was the quick "solution" trying to resolve the security issues with unsupported header fields; not the "meaningful" check. 
I am not sure whether the spec requires that implementations MUST reject headers that contain meaningless fields like in your epk example below. 

BTW: jsoncrypto.org has an issue tracker. 

I am not sure about the "usefulness" of my library with or without checkHeader. It only tries to implement the "MUST" unknown-fields-rejection requirement from the spec not more.
Maybe I am reading the spec not sharp enough?

Axel

Ps: thanks for the suggestion using the Set of know headers. I thought about that too but then decided against it because I feel that I should then implement "everything" in data instead of code... knownJwsHeaders, knownJweHeaders, supportedAEADalgs, etc
I suggest to discuss "beautiful code" privately?! E.g. this implementation currently is not object oriented at all. There could be a class JweHeader that has a constructor that parses the jweHeaderStr and throws if it is not meaningful... Or a class RSAJwt that knows the header field that make sense if alg = RSA1_5 ...

-----Original Message-----
From: [email-masked] [mailto:[email-masked]] On Behalf Of Manger, James H
Sent: Thursday, August 02, 2012 3:54 PM
To: Nennker, Axel; [email-masked]
Subject: Re: [jose] implementing JOSE in Java: jsoncrypto

--
James Manger

---

From: Anil Saldhana <[email-masked]>
Date: Thu, 02 Aug 2012 11:23:44 -0500

Axel,
   thank you very much for an implementation of JOSE in Java. Rather 
than create an umpteen number of OSS projects, it is better to have one 
or two conforming implementations that everybody can re-use.

In your implementation, rather than check on individual "alg","typ" etc 
against a key, it is better to just check against a set of allowed 
parameter set/array (as James mentioned).

Regards,
Anil