mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jan Schlicht <...@mesosphere.io>
Subject Re: Review Request 56667: Added support for JSON Web Tokens.
Date Wed, 22 Feb 2017 14:20:00 GMT


> On Feb. 20, 2017, 8:36 a.m., Greg Mann wrote:
> > Regarding the description: I'm curious how exactly the current implementation isn't
compliant with RFCs 7515/7519? The one thing I noticed was the lack of support for the 'crit'
header parameter.

There isn't support for `alg=none` and it is strongly recommended to also support `alg=RS256`.
Standard claims aren't validated, though it's up to the specific applications to specify which
of these claims are mandatory; it would make sense to validate them as part of a general-purpose
JWT implementation. Decoded JSON isn't tested for line breaks, whitespaces, correct UTF-8
encoding.


> On Feb. 20, 2017, 8:36 a.m., Greg Mann wrote:
> > 3rdparty/libprocess/include/process/jwt.hpp, lines 44-48
> > <https://reviews.apache.org/r/56667/diff/2/?file=1637101#file1637101line44>
> >
> >     Do we want to include support for the 'crit' header parameter for spec compliance?

If JWT parsing is kept internal only, e.g. we only parse JWTs created by the secret generator,
it's IMHO okay to leave out 'crit' parameter handling, because we wouldn't have it in any
header.


> On Feb. 20, 2017, 8:36 a.m., Greg Mann wrote:
> > 3rdparty/libprocess/include/process/jwt.hpp, line 77
> > <https://reviews.apache.org/r/56667/diff/2/?file=1637101#file1637101line77>
> >
> >     Do you think it's worth using a `JSON::Object` for the header? This would let
the module accommodate arbitrary header keys (AKA 'Private Header Parameter Names' from RFC-7515),
which could be useful for users who want to use the module for other purposes?

Same as above: It depends on what the scope of this implementation should be. An internal-only
JWT implementation doesn't need a `JSON::Object` for the header, because we control what will
be in the header. Of course, if this module should be more general-purpose, this would need
to be changed, along with some other changes. But then we could also strive for full RFC compliance,
which would mean (among others) support for `alg=none`, probably `alg=RS256` and other subtleties.


- Jan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56667/#review166057
-----------------------------------------------------------


On Feb. 16, 2017, 10:35 a.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56667/
> -----------------------------------------------------------
> 
> (Updated Feb. 16, 2017, 10:35 a.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-7001
>     https://issues.apache.org/jira/browse/MESOS-7001
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> JSON Web Tokens can be used to create claim-based access tokens and is
> typically used for HTTP authentication.
> This implementation is intended for internal use, e.g. Mesos is supposed
> to only parse tokens that it also created. It doesn't fully comply with
> RFC 7519. Currently the only supported cryptographic algorithm is HMAC
> with SHA-256.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/Makefile.am 75386184108214e67a58c328258ec204099d638c 
>   3rdparty/libprocess/include/process/jwt.hpp PRE-CREATION 
>   3rdparty/libprocess/src/jwt.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/jwt_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/56667/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message