Skip to content
This repository has been archived by the owner on Mar 12, 2020. It is now read-only.

Remove time component from for AES-GSM keys #326

Closed
carsonfarmer opened this issue Oct 23, 2018 · 3 comments
Closed

Remove time component from for AES-GSM keys #326

carsonfarmer opened this issue Oct 23, 2018 · 3 comments
Assignees
Labels
bug Something isn't working CRITICAL Critically effects UX in progress security Security related

Comments

@carsonfarmer
Copy link
Member

carsonfarmer commented Oct 23, 2018

We should just move to reading 44 bytes from crypto/rand Reader and be done with it. This gives us a cryptographically secure random number generator, even on mobile.

@carsonfarmer carsonfarmer added bug Something isn't working security Security related CRITICAL Critically effects UX in progress labels Oct 23, 2018
@sanderpick
Copy link
Member

Ah yeah, duh... relic from lazily wanting a pre-utf8 compatible AES key. We can just base58 encode the random bytes when we need it as a string for URL parameters.

@sanderpick sanderpick changed the title Switch to crypto random bytes (crypto/rand Reader) for AES-GSM keys Remove time component from for AES-GSM keys Oct 24, 2018
sanderpick added a commit that referenced this issue Oct 24, 2018
This was a pretty silly way to avoid dealing with string encoding. Of course, I didn't come back and
fix it until after someone else noticed ;) With this change plus the thread encryption updates in
05a269c, we'd be ready for an actual security review.

fixes #326
@sanderpick
Copy link
Member

done in 386b63c

@carsonfarmer
Copy link
Member Author

Thanks for the quick turnaround on that one @sanderpick:

With this change plus the thread encryption updates in 05a269c, we'd be ready for an actual security review.

For those following along at home, this fix should make it into release once #300 is done and merged.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working CRITICAL Critically effects UX in progress security Security related
Projects
None yet
Development

No branches or pull requests

2 participants