I think it must go in the 'token_contents' itself, otherwise it can be
modified by a malicious client. Other than that, looks good.
- Dan
On Wed, Jan 25, 2017 at 12:37 PM, Todd Lipcon <t...@cloudera.com> wrote:
> Hey folks
>
> I'm working on the token signing/verification stuff at the moment. Curious
> to solicit some opinions on this:
>
>
> message TokenPB {
> // The actual token contents. This is typically a serialized
> // protobuf of its own. However, we use a 'bytes' field, since
> // protobuf doesn't guarantee that if two implementations serialize
> // a protobuf, they'll necessary get bytewise identical results,
> // particularly in the presence of unknown fields.
> optional bytes token_contents = 1;
>
> // The cryptographic signature of 'token_contents'.
> optional bytes signature = 2;
>
> // The sequence number of the key which produced 'signature'.
> optional int64_t signing_key_seq_num = 3;
> };
>
> The thing that's currently missing is an expiration timestamp of the
> signature. I have two options here:
>
> *Option A*) say that the TokenPB itself doesn't capture expiration, and
> if a particular type of token needs expiration, it would have to put an
> 'expiration time' in its token contents itself.
>
> *pros:*
> - token signing/verification is just a simple operation on the
> 'token_contents' string
>
> *Cons:*
> - would likely end up with redundant code between AuthN and AuthZ tokens,
> both of which need expiration. However, that code isn't very complicated
> (just a timestamp comparison) so maybe not a big deal?
>
> *Option B)* add an expiration timestamp field to TokenPB
> *pros:*
> - consolidate the expiration checking code into TokenVerifier
> *cons:*
> - now in order to sign/verify a token, we actually need to be signing
> something like a concatenation of 'token_contents + signature'. Not too
> hard to construct this concatenation, but it does add some complexity.
>
> Any strong opinions either way?
>
> -Todd
> --
> Todd Lipcon
> Software Engineer, Cloudera
>
