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

Fixing user cookie issue #1063 #1116

Merged
merged 3 commits into from
Dec 13, 2016
Merged

Fixing user cookie issue #1063 #1116

merged 3 commits into from
Dec 13, 2016

Conversation

rcaa
Copy link

@rcaa rcaa commented Aug 15, 2016

No description provided.

Copy link
Member

@flaix flaix left a comment

Choose a reason for hiding this comment

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

It would be preferable to use a SecureRandom for the random cookie generation instead of Math.random.

flaix pushed a commit to flaix/gitblit that referenced this pull request Dec 10, 2016
To get proper entropy in user authentication cookie creation,
make use of `SecureRandom` instead of using `Math.random()`, or
`Random`.

Introduce our own wrapper `SecureRandom` around `java.security.SecureRandom`.
This a) makes sure that the PRNG is seeded on creation and not when
random bytes are retrieved, and
b) uses a static instance in the `UserModel` so that lags do not occur
during operation due to potentially seeding getting blocked on Unix
when reading from the system's entropy pool. To keep the random data
still secure, the static instance will reseed all 24 hours, also a
functionality of the wrapper class.

This fixes gitblit-org#1063 and extends and closes PR gitblit-org#1116
@rcaa
Copy link
Author

rcaa commented Dec 11, 2016

Done! @fzs

@flaix flaix modified the milestone: 1.9.0 Dec 13, 2016
flaix pushed a commit to flaix/gitblit that referenced this pull request Dec 13, 2016
Introduce our own wrapper `SecureRandom` around `java.security.SecureRandom`.
This a) makes sure that the PRNG is seeded on creation and not when
random bytes are retrieved, and
b) uses a static instance in the `UserModel` so that lags do not occur
during operation due to potentially seeding getting blocked on Unix
when reading from the system's entropy pool. To keep the random data
still secure, the static instance will reseed all 24 hours, also a
functionality of the wrapper class.

This fixes gitblit-org#1063 and extends and closes PR gitblit-org#1116
@flaix flaix merged commit a1fc7e7 into gitblit-org:master Dec 13, 2016
@flaix
Copy link
Member

flaix commented Dec 13, 2016

This was manually merged,
a) because the PR was created from rcaaa:master. Please use separate branches when creating pull requests, because any other push to your master branch will also show up in this pull request.
b) To improve on it by controlling the seeding of the PRNG.

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.

None yet

2 participants