[zaroth]

I opened an issue on their bug tracker a couple days ago because I had more fundamental questions about how their algorithm was designed.

HN comment: https://news.ycombinator.com/item?id=15204989

Github Issue: https://github.com/18F/identity-idp/issues/1665

Now I may be misunderstanding their implementation, but it seems to me actually worse than "this isn't FIPS 140". I take no issue with their use of scrypt at all. The complaints in this Gist actually seem to miss the mark to me.

The problem isn't scrypt, or storing a hash. The real problem is storing d which, seems to me, defeats the purpose of using an HSM in the first place!

Again, with the disclaimer that I'm not 100% certain that's what they're doing, it's just what the documentation says, and what it seems like is happening from reading the Ruby code. I don't develop in Ruby, but while the language is fairly easy to read, with all their variables named 'encryption_code', 'encryption_key', 'encrypted_key', 'encrypted_code', etc. it's easy to get lost.

EDIT: Can someone please co-validate my analysis please? Or please tell me what I got wrong! From what I can see, the design is obviously broken. I don't consider myself qualified to find flaws in other people's crypto so, when I find obvious flaws in what should be well designed crypto I get a little freaked out.

[aaronbwebber]

EDIT: OK, I think I see what's wrong with this analysis: https://github.com/18F/identity-idp/blob/master/spec/service.... They do include Z1 in the data they pass to KMS decrypt. This is because they have XORed the ciphertext with Z1 - so now in order to recover the initial ciphertext back (i.e. the cipher text you can decrypt with KMS to get R back), you need to XOR with Z1 again. So even if you get D from the database, you can't decrypt it unless you have Z1, and if you were trying to brute-force it, you would need to call KMS for every attempt with the Z1 for this attempt.

Original comment:

I am not any kind of crypto expert. But I agree with you.

I think they should be XORing with "AssignedSecret" (not "EncryptedAssignedSecret"). Then in order to compare password hashes you would need to decrypt the "EncryptedAssignedSecret" using KMS. (using the terminology in the description of the algorithm here: https://github.com/18F/identity-idp/blob/master/docs/encrypt...) (EDIT: you actually cover this approach in the GH issue, and point out a problem with it).

I think there is another approach like what the OP proposed where they should be using some portion of the scrypt hash (either Z1 or Z2 or just the whole thing) as part of the encryption context passed to KMS. But this seems a little more complicated to me. (EDIT: You also mention this as a possible solution in the GH issue).

I think there's also an approach using the GenerateDataKey API : http://docs.aws.amazon.com/kms/latest/APIReference/API_Gener...

Where you get the key and the key's cipher text back from KMS and store that, and then you have to use KMS to decrypt whatever you encrypted with the key.

[zaroth]

Yes, you need to know the password to decrypt. That's the basis of their very interesting PII protection scheme!

My point is you can brute-force the password without the HSM. And their docs specifically say this should not be possible. Isn't the whole point of the HSM to prevent that?

'hash' is derived from 'z2' and 'd' - and they give you 'd'. Password will produce 'z2'. So that's the brute-force attack.

To say it another way, the 'z1' path is secured. But there is the 'z2' path and it's wide open.

[dreae]

I've left a comment on your github issue, but the tl;dr is that the encryption document doesn't seem to reflect the code. The comment in the ruby test case suggests that the actual step is 'E = hash( Z2 + R )' instead of 'E = hash( Z2 + D )' where 'R' is the user's randomly generated 'AssignedKey' and 'D' is the masked HSM ciphertext 'KMS(R) ^ Z2'.

Perhaps somewhere during implementation someone realized the document doesn't make sense as written.