#16285 closed Cleanup/optimization (fixed)
Remove a misleading comment from the signing code
Reported by: | Paul McMillan | Owned by: | nobody |
---|---|---|---|
Component: | Core (Other) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
lines 99 and 100 of the signing module indicate that the provided salt is used to prevent brute forcing. This is misleading, it's actually necessary for namespacing signed strings, so that users can't take a signed string from one part of Django and put it into a different part which will trust it since it was properly signed.
Attachments (1)
Change History (8)
by , 13 years ago
Attachment: | signing_comment.diff added |
---|
comment:1 by , 13 years ago
Patch needs improvement: | set |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 13 years ago
Patch needs improvement: | unset |
---|
In general, you're correct about different ways to use salt. In this case, that's not what's going on.
The "salt" here is going into a pre-seed for the salted-hmac module. That module already has other key material (the secret key) to prevent an attacker from forging the signature. Generally, salt is used to prevent pre-computed lookup table attacks, and is stored alongside the hashed material (and the plaintext is not stored at all). Salt is not meant to be kept more secret than the resulting hash.
In the signing module, we're using it differently. Since we expect the plaintext to be known to the attacker (this is why we're signing it, after all, to prove to ourselves that it hasn't been tampered with), using a salt to prevent rainbow table type attacks is pointless. We don't care that the attacker knows which plaintext hashes to which signature, since we provide both of those. What we do care about is that the secret key stay secret, since that's what prevents the attacker from generating their own signatures. Adding a salt doesn't change this property, since the attacker still has everything necessary to produce a signature except the secret key.
Using a random salt in this field makes your application less secure than using a single string for namespacing, since it allows a user to transfer a signature from one area of your program to another.
You might want to consult the original academic papers on HMAC, as well as look at the way salted_hmac is implemented in Django.
comment:3 by , 13 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
Although I have a small background in cryptography, I wasn't familiar with HMAC. Thanks for the explanation, it was very clear.
Patch applies cleanly on SVN trunk.
comment:5 by , 13 years ago
The documentation itself still mentions that the secret key can be used for preventing brute force attacks. Also I would recommend adding an example of why salting is a good thing to have. Feel free to steal from the one i have in itsdangerous: http://packages.python.org/itsdangerous/#the-salt
I think you can use salt for both puposes:
I accept the ticket because the second use-case sounds useful.
I set "patch needs improvement" because I'd like to make sure we don't dismiss the first one too quickly.