Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Tokenserver: Rewrite inlined Python code in Rust #1053

Merged
merged 3 commits into from
Apr 28, 2021

Conversation

ethowitz
Copy link
Contributor

Description

Replaces the inlined Python code with native Rust code. I replaced all of the Python functions with equivalent Rust functions, except for tokenlib#make_token and tokenlib#get_derived_secret, which I added Rust wrappers for.

Testing

I tested this by running the Python and Rust versions with the same input data and ensuring that the output was the same.
Python test file: https://gist.github.com/ethowitz/7d6b3590f4aae0c51b2df71297c1d303
Rust test code (I added this to syncstorage-rs's main function for expediency): https://gist.github.com/ethowitz/2ec06b230b2db66de6088ac30a8c9819

Python output:

dGVzdGluZzEyMw # URL-safe base64 encoded string with stripped padding
51c982a4554222dce10f7fd77145a92f87a6f55934403ff8d2546d6c0a4da872 # FxA metrics hash
402e41f93a922d7517f43668d2392e72 # hashed device ID
eyJ1c2VyX2lkIjogMTIzLCAic2FsdCI6ICIzMjkxYzQiLCAiZXhwaXJlcyI6IDE2MTk0NzM2MzQuMjQ1Mjc5Nn3kixJDJKmsgAc6GMizJZjB7IMwVBBexo8idSGHZUNlbg== # token
Sdqw4UlM3JIPKZ2tN2YU9N3ZrvA4dfCEEbI35fu7-ac= # derived secret

Rust output:

dGVzdGluZzEyMw # URL-safe base64 encoded string with stripped padding
51c982a4554222dce10f7fd77145a92f87a6f55934403ff8d2546d6c0a4da872 # FxA metrics hash
402e41f93a922d7517f43668d2392e72 # hashed device ID
eyJ1c2VyX2lkIjogMTIzLCAic2FsdCI6ICJlNGQ0Y2UiLCAiZXhwaXJlcyI6IDE2MTk0NzM1NzEuOTU4MTE5NH19_O2gNmQr7SzM-660260QllLQXysgrFzA7A4Da47mRA== # token
Sywsc6GJcQhSQoiT7ctlS2ltKNSdPbdvT2GbjaRXiwY= #derived secret

As you can see, the tokens and derived secrets differ, but these values differ across runs of the same version (e.g. when running the Python version twice, the values differ across the runs). I assume the tokenlib library adds nondeterminism (e.g. a nonce) so the values are different each time.

Issue(s)

Closes #1049

@ethowitz ethowitz requested a review from a team April 26, 2021 22:05
@jrconlin
Copy link
Member

Yep. The token is a base64 JSON blob that contains the user_id, salt, and expires timestamp, along with some padding noise. Looking at the tokens, I see that the user_ids match, with the salt and expires being different as would be expected.

// todo don't hardcode
// we're supposed to check the "duration" query
// param and use that if present (for testing)
thedict.set_item("expires", 300).unwrap(); // todo this needs to be converted to timestamp int (now + value * 1000)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could have this pull from env like you're doing above with an unwrap_or_else that uses 300 as the default.

It'd also be nice to have a section of Settings that defined all of these so that there's no risk of these getting lost in the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jrconlin I think this was done by someone else; I think it ended up in the diff since I changed the indentation or something 🤔 Do you think it's worth making this change here? Might be worth waiting for when I do the work for #1052, but happy to make the change here if you'd prefer that!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, not for this iteration. I was just noting the "todo". If it was something critical or important, I wouldn't have approved the changes 😉

@ethowitz ethowitz merged commit 34fe585 into master Apr 28, 2021
@ethowitz ethowitz deleted the refactor/tokenserver-rewrite-python-functions branch April 28, 2021 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tokenserver: Rewrite inlined Python functions in Rust
2 participants