Scott, On Wed, Sep 12, 2012 at 12:57 PM, Scott MacVicar <sc...@macvicar.net> wrote:
> Concerns about the RFC after talking with someone (Alok) on our security > team at work. > > "There is no requirement for them to be cryptographically secure. " > What stops the salt from being cryptographically secure? I think it should > be a goal or we should state what parts aren't cryptographically secure, is > it the random data source? > What stops it? Nothing. You can use a CS random string for a salt. That's fine. The point was that salts don't need to be CS to work. There's no requirement that they be CS (which is what I said). In fact, the only requirement is that they be unique. Not purely globally unique, but unique to the point that there's a very insignificant chance that anyone else is using the same settings with that same salt. Even if it's on a different application... Now, given that the only good CS source on most machines is /dev/random, and that it is blocking if it runs out of entropy, there's good reason to not use them unless you need them. So my approach is to only use CS randomness when I need it (key generation mainly). Cases when the quality of randomness significantly impacts the security. So the line is in there because the generated random salt is generated by /dev/urandom. I just wanted to make that distinction clear. > "The salt parameter, if provided, will be used in place of an > auto-generated salt." > This is setting someone up for failure by letting them put in something > weak, you should be forced to get an auto-generated salt. If this is for > unit testing then it should be explicitly stated. > It's there because some use-cases may dictate using a CS random salt. In those cases, you can read from /dev/random (or use mcrypt, etc) to generate the salt. The vast majority should not generate that salt parameter though. And that will be reflected in the documentation. > The documentation also needs improved around password_needs_rehash. Tell > the developer that at login time (or any other password validation time), > if the options have changed, the password can be rehashed. > Absolutely. That's why that function exists. I did not try to make the RFC into end-user documentation. In fact, I explicitly tried to avoid doing that. I wanted the RFC to be the technical justification and explanation. The end-user documentation will be made later, and it will include information like that (including examples)... > E_WARNING in a crypto function is also bad. Throw an exception or fatal. > There's no reason to just raise a warning and continue execution, if > something went wrong in crypto then its a bad time for everyone. I partially agree. I think a FATAL is a bit much (especially if it's a recoverable error, such as an invalid param). And exceptions are out (not allowed in core functions). I think the warning is the best approach, since it will always return a value conducive with the error (NULL or FALSE, depending on the function). If it returns a value, it's because no warning was thrown... Anthony
