Ineffective encryption
lukecyca opened this issue · 2 comments
I don’t understand the purpose of the encryption used in this library. Here is how it is working:
Encryption:
- Generate a 32-byte token:
A
- Generate a 32-byte key:
B
- Xor the token with the key to "encrypted" it:
A ^ B -> C
- Concatenate the key with the encrypted token:
BC
- Send all 64 bytes to the client as a cookie:
BC
Decryption:
- Accept the 64-byte cookie: BC
- Split it down the middle to get a 32-byte key:
B
, and a 32-byte encrypted blob:C
- Xor the blob with the key to get the decrypted token:
C ^ B -> A
This "crypto" is at best a thin layer of obfuscation. It would be logically equivilent to just send the original 32-byte token as a cookie. In fact, this would then be the Double Submit Cookie pattern.
If the intent is to use the Encrypted Token Pattern (described on the same page, and which I think is way more robust than Double Submit Cookies) then the algorithm would have to work like this:
- Generate a 32-byte token:
A
- Combine it with some other information such as RemoteAddr and a timestamp:
ABC
- Encrypt it with a secret key:
f(ABC, S) -> C
- Give the encrypted blob to the client in a cookie:
C
Then when a new request comes in, we:
- Decrypt the cookie:
g(C, S) -> ABC
- Ensure the decrypted token
A
agrees with what was supplied in the form or header - Ensure the decrypted RemoteAddr
B
agrees with that of the request - Ensure the timestamp
C
is within a reasonable expiry duration
The purpose of encryption in nosurf is not to hide the contents of the token from the end-user. It's a random sequence of bytes anyway, not some secret.
The point is to scramble the token to mitigate the BREACH attack. As token ends up being completely different on every request, AFAIK, it should be unobtainable by BREACH. This method is right there in the middle of the list of mitigations on the BREACH website:
Masking secrets (effectively randomizing by XORing with a random secret per request)
As far as that goes, the "encryption" algo used here is effective. Please correct me if I'm wrong, but as I see it, there is no issue here.
I hadn’t considered that purpose. That makes sense and is a sound approach as far as I understand. I do think that calling the process “encryption” is a bit misleading (it certainly misled me!).
Anyway, thanks for your informative reply.