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.

Sign up for GitHub

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

Jump to bottom

fix: we are slicing keys and losing entropy. #2778

Closed
wants to merge 2 commits into from
Closed

fix: we are slicing keys and losing entropy. #2778

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
528167b
fix: we are slicing keys and losing entropy.
Aug 10, 2019
2263217
fix: just a small grammatical fix in comments
Aug 10, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 4 additions & 3 deletions server/src/main/scala/org/http4s/server/middleware/CSRF.scala
View file
Open in desktop
Expand Up @@ -446,7 +446,6 @@ object CSRF {
///

val SigningAlgo: String = "HmacSHA1"
val SHA1ByteLen: Int = 20
Copy link
Member

Choose a reason for hiding this comment

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

If we deprecate this value, we could release this as a fix on series/0.20.

Copy link
Author

Choose a reason for hiding this comment

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

I think we should (deprecate it). Do I need to do anything on my side to do that?

Copy link
Member

Choose a reason for hiding this comment

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

I just went ahead and did it, because it's easier than taking a reviewed PR and moving it to a different base. But deprecating a value looks like this.

val CSRFTokenLength: Int = 32

/** An instance of SecureRandom to generate
Expand Down Expand Up @@ -499,13 +498,15 @@ object CSRF {
/** Build a new HMACSHA1 Key for our CSRF Middleware
* from key bytes. This operation is unsafe, in that
* any amount less than 20 bytes will throw an exception when loaded
* into `Mac`, and any value above will be truncated (not good for entropy).
* into `Mac`. Any keys larger than 64 bytes are just hashed.
*
* For more information, refer to: https://tools.ietf.org/html/rfc2104#section-3
*
* Use for loading a key from a config file, after having generated
* one safely
*
*/
def buildSigningKey[F[_]](array: Array[Byte])(implicit F: Sync[F]): F[SecretKey] =
F.delay(new SecretKeySpec(array.slice(0, SHA1ByteLen), SigningAlgo))
F.delay(new SecretKeySpec(array, SigningAlgo))

}