password_hash() deprecate salt option - thoughts? View as Markdown

I think deprecating it is a good idea, and looking at the documentation it
does mention that not providing it is the intended option; so it isn't a
complete surprise for it to become deprecated.

All,

Ever since we introduced password_hash() in 5.5, I've been watching
its usage as much as possible. I've setup google alerts and such, as
well as auditing implementations I've found on github to try to
understand how it's used.

One thing has become abundantly clear to me: the salt option is
dangerous. I've yet to see a single usage of the salt option that has
been even decent. Every usage ranges from bad (passing mt_rand()
output) to dangerous (static strings) to insane (passing the password
as its own salt).

I've come to the conclusion that I don't think we should allow users
to specify the salt. The crypt() API still exists if users have a need
to generate their own salt. Having it in the simplified API is simply
adding a risk factor without any significant justification.

So I'd like to hear your thoughts about raising E_DEPRECATED when the
salt option is specified in 7.0, with ultimately removing the option
in a later version.

Additionally, I know this is after the RFC freeze deadline, so if you
want to postpone the deprecation to 7.1, that's fine. I just think
it's worth discussion (and if there's consensus to put it in 7.0, then
great).

Thanks,

Anthony

All,

Ever since we introduced password_hash() in 5.5, I've been watching
its usage as much as possible. I've setup google alerts and such, as
well as auditing implementations I've found on github to try to
understand how it's used.

One thing has become abundantly clear to me: the salt option is
dangerous. I've yet to see a single usage of the salt option that has
been even decent. Every usage ranges from bad (passing mt_rand()
output) to dangerous (static strings) to insane (passing the password
as its own salt).

I've come to the conclusion that I don't think we should allow users
to specify the salt. The crypt() API still exists if users have a need
to generate their own salt. Having it in the simplified API is simply
adding a risk factor without any significant justification.

So I'd like to hear your thoughts about raising E_DEPRECATED when the
salt option is specified in 7.0, with ultimately removing the option
in a later version.

Additionally, I know this is after the RFC freeze deadline, so if you
want to postpone the deprecation to 7.1, that's fine. I just think
it's worth discussion (and if there's consensus to put it in 7.0, then
great).

Thanks,

Anthony

--

+1

I'd even go as far as adding a big red warning about custom salts to the manual page.

On Tue, Mar 31, 2015 at 8:49 PM, Anthony Ferrara ircmaxell@gmail.com
wrote:

All,

Ever since we introduced password_hash() in 5.5, I've been watching
its usage as much as possible. I've setup google alerts and such, as
well as auditing implementations I've found on github to try to
understand how it's used.

One thing has become abundantly clear to me: the salt option is
dangerous. I've yet to see a single usage of the salt option that has
been even decent. Every usage ranges from bad (passing mt_rand()
output) to dangerous (static strings) to insane (passing the password
as its own salt).

I've come to the conclusion that I don't think we should allow users
to specify the salt. The crypt() API still exists if users have a need
to generate their own salt. Having it in the simplified API is simply
adding a risk factor without any significant justification.

So I'd like to hear your thoughts about raising E_DEPRECATED when the
salt option is specified in 7.0, with ultimately removing the option
in a later version.

Additionally, I know this is after the RFC freeze deadline, so if you
want to postpone the deprecation to 7.1, that's fine. I just think
it's worth discussion (and if there's consensus to put it in 7.0, then
great).

Agree with deprecating the custom salt option and also okay with doing it
in 7.0.

Nikita

On Tue, Mar 31, 2015 at 7:49 PM, Anthony Ferrara ircmaxell@gmail.com
wrote:

All,

Ever since we introduced password_hash() in 5.5, I've been watching
its usage as much as possible. I've setup google alerts and such, as
well as auditing implementations I've found on github to try to
understand how it's used.

One thing has become abundantly clear to me: the salt option is
dangerous. I've yet to see a single usage of the salt option that has
been even decent. Every usage ranges from bad (passing mt_rand()
output) to dangerous (static strings) to insane (passing the password
as its own salt).

I've come to the conclusion that I don't think we should allow users
to specify the salt. The crypt() API still exists if users have a need
to generate their own salt. Having it in the simplified API is simply
adding a risk factor without any significant justification.

So I'd like to hear your thoughts about raising E_DEPRECATED when the
salt option is specified in 7.0, with ultimately removing the option
in a later version.

Additionally, I know this is after the RFC freeze deadline, so if you
want to postpone the deprecation to 7.1, that's fine. I just think
it's worth discussion (and if there's consensus to put it in 7.0, then
great).

Thanks,

Anthony

No objections here. You're going with your gut on this and it seems like a
good call. We did make a freeze and although I'd like it in 7.0 we should
probably stick to our process here and put it in 7.1.

To be fair, there's always a quick *.1 release anyway once we find
real-world bugs that need patching quickly ;-)