Add AuthenticationTokenHash
#1985
Conversation
tgeoghegan
force-pushed
the
timg/move-auth-tokens
branch
from
06b3f3b to
e6ab04d
Compare
| value: &[u8; SHA256_OUTPUT_LEN], | ||
| serializer: S, | ||
| ) -> Result<S::Ok, S::Error> { | ||
| serializer.serialize_str(&URL_SAFE_NO_PAD.encode(value)) |
There was a problem hiding this comment.
It strikes me as unusual to encode a sha256 hash as base64, rather than hex, but I can't think of anything actually wrong with it.
| self.aggregator_auth_token() | ||
| .map(AuthenticationTokenHash::from) | ||
| .zip(incoming_auth_token) | ||
| .map(|(own_token_hash, incoming_token)| own_token_hash.validate(incoming_token)) | ||
| .unwrap_or(false) |
There was a problem hiding this comment.
(options work as Iterators which will always have either 0 (if None) or 1 (if Some) elements, so you can do with them whatever you can do with any iterator. this enables nice tricks like this sometimes.)
|
Note that I only reviewed 2577838. |
core/src/auth_tokens.rs
Outdated
| let mut any_non_equals = false; | ||
| // First loop: consume "normal" characters, stop when we see an equals sign for padding or | ||
| // reach the end of the input. | ||
| for c in &mut iter { |
There was a problem hiding this comment.
Any reason not to use a regex here? I lean away from them normally but a small value defined by BNF-like grammar is a perfect place for a regex IMO.
I think it would be something like [-a-zA-Z0-9._~+/]+=*.
There was a problem hiding this comment.
(we could also just try decoding as base64, since I suppose that's the intent -- though that does lose some non-base64 values that are allowed by the BNF, such as A=======.)
There was a problem hiding this comment.
I introduced this in #1978. It got moved in #1984, and it'll disappear from here once it's rebased on main. A regex sounds good, it may be able to run faster too. As described in #1978, base64-decoding would be more strict than necessary, both in terms of the character set allowed, the choice of padding vs. no padding, and limitations on length (excluding padding). I can make a follow-up PR to swap in a regex here.
There was a problem hiding this comment.
Sorry, I should have called out that this PR is stacked on #1984.
When acting as an HTTP server, Janus only needs a hash of the aggregator authentication token to validate incoming requests, and can avoid storing the actual token. This commit adds `janus_core::auth_tokens::AuthenticationTokenHash` which will be used to implement that. Part of #1509
tgeoghegan
force-pushed
the
timg/add-authentication-token-hash
branch
from
2577838 to
e9786db
Compare
When acting as an HTTP server, Janus only needs a hash of the aggregator authentication token to validate incoming requests, and can avoid storing the actual token. This commit adds
janus_core::auth_tokens::AuthenticationTokenHashwhich will be used to implement that.Part of #1509