I like this idea. Probably, that's too early at this point, but consider adding notion of compt/non-compat feature flags into tokens. Imagine the new version of token has some additional restriction field, which older code does not understand. In that case, even if the token signature is correct, the older code should not accept the new token since unsupported non-compat feature flags is present.
But in the essence this looks great, IMO. Best regards, Alexey On Wed, Jan 25, 2017 at 12:52 PM, Todd Lipcon <[email protected]> wrote: > Actually had one more idea... how about: > message AuthenticationToken { > }; > > message AuthorizationToken { > }; > > message TokenPB { > // The time at which this token expires, in seconds since the > // unix epoch. > optional int64 expire_unix_epoch_seconds = 1; > > oneof token { > AuthenticationToken authn = 2; > AuthenticationToken authz = 3; > } > }; > > message SignedTokenPB { > // The actual token contents. This is a serialized TokenPB protobuf. > 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 = 2; > > // The cryptographic signature of 'token_contents'. > optional bytes signature = 3; > > // The sequence number of the key which produced 'signature'. > optional int64 signing_key_seq_num = 4; > }; > > > > On Wed, Jan 25, 2017 at 12:44 PM, Todd Lipcon <[email protected]> wrote: > >> On Wed, Jan 25, 2017 at 12:40 PM, Dan Burkert <[email protected]> wrote: >> >>> I think it must go in the 'token_contents' itself, otherwise it can be >>> modified by a malicious client. Other than that, looks good. >>> >>> >> well, if it went into a separate field, then we'd have something like: >> >> optional bytes token_contents = 1; >> optional int64 expiration_timestamp = 2; >> >> // Signature of the string: '<32-bit big-endian length of token_contents> >> token_contents <64-bit big-endian expiration>' >> optional bytes signature = 3; >> >> so they could try to modify it, but the signature would fail. >> >> >> >>> On Wed, Jan 25, 2017 at 12:37 PM, Todd Lipcon <[email protected]> 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 >>>> >>> >>> >> >> >> -- >> Todd Lipcon >> Software Engineer, Cloudera >> > > > > -- > Todd Lipcon > Software Engineer, Cloudera >
