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

Token Mechanism #2453

Merged
merged 3 commits into from
Apr 16, 2018
Merged

Token Mechanism #2453

merged 3 commits into from
Apr 16, 2018

Conversation

TheAspens
Copy link
Member

This pull request updates the method for creating random strings (tokens) to use cryptographically secure functions and it adds a token table and functions for interacting with the table. This will be used by both issue #2447 and issue #2451

Kevin Reed added 3 commits April 6, 2018 09:15
that is considered cryptographically secure.  This required adding the
random_compat library (MIT License) to provide support for PHP versions
before 7.
Add script that will delete expired tokens once a day
@TheAspens
Copy link
Member Author

The design of this was documented under "Token Generation" and "Token Usage" here: https://boinc.berkeley.edu/trac/wiki/RightToErasure#TokenGeneration (which is the full design document for issue #2447 )

@davidpanderson
Copy link
Contributor

PHP provides a function for making crypto-secure random strings:
http://php.net/manual/en/function.openssl-random-pseudo-bytes.php

@TheAspens
Copy link
Member Author

This code uses http://php.net/manual/en/function.random-bytes.php which is also a PHP function and is recommended as superior to openssl-random-psuedo-bytes.php.

The inclusion of https://github.com/paragonie/random_compat is recommended on the php.net website with this comment under the description of random-bytes again see http://php.net/manual/en/function.random-bytes.php

Note: Although this function was added to PHP in PHP 7.0, a » userland implementation is available for PHP 5.2 to 5.6, inclusive.

There are lots of recommendations that direct people to use random-bytes.php as preferred over openssl-random-psuedo-bytes.php

@TheAspens
Copy link
Member Author

@davidpanderson - do you have any other objections or can you go ahead and merge this?

@davidpanderson
Copy link
Contributor

Looks good; sorry for the delay.

Small note: at some point I started using "double" for unix timestamps because of 2038.
But it doesn't matter.

@davidpanderson davidpanderson merged commit a5ea95a into master Apr 16, 2018
@TheAspens TheAspens deleted the knr_tokens branch April 17, 2018 14:48
@JuhaSointusalo
Copy link
Contributor

Sorry for being late. You allow token.expire_time to be null. Was that intentional?

@TheAspens
Copy link
Member Author

@JuhaSointusalo - Yes - I wanted to allow for tokens that do not expire.

@JuhaSointusalo
Copy link
Contributor

@TheAspens Ok, thanks for explaining. I'm not sure if looking up valid non-expiring tokens works but I suppose that can wait until the more urgent stuff is done.

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.

3 participants