Skip to content
This repository has been archived by the owner on Jan 31, 2020. It is now read-only.

Proposal: Validation for breached passwords #259

Open
DragonBe opened this issue Apr 5, 2019 · 18 comments
Open

Proposal: Validation for breached passwords #259

DragonBe opened this issue Apr 5, 2019 · 18 comments

Comments

@DragonBe
Copy link
Contributor

DragonBe commented Apr 5, 2019

Hi,

I'm maintainer of dragonbe/hibp, a composer package to validate passwords against the Have I been pwned? API service to see if a provided password was already used in a breach.

Today (April 5, 2019) Symfony announced they integrate the same service as a validation component in their 4.3 version and I see this as a good way to offer this functionality as part of the framework toolkit. Too bad they're not using my service though.

So I want to propose a validation rule to check provided passwords against HIBP API.

@Ocramius
Copy link
Member

Ocramius commented Apr 5, 2019

@DragonBe I'd say "go ahead and implement" - this is very valuable.

As a note: please consider using a PSR-18 client as a dependency of the HIBP validator 👍

@DragonBe
Copy link
Contributor Author

DragonBe commented Apr 5, 2019

Thank you @Ocramius, I will provide a solid solution

@DragonBe
Copy link
Contributor Author

DragonBe commented Apr 5, 2019

I'm contemplating on naming conventions. The service is called "Have I been pwned?", but not everyone will recognise hibp as a reference to this service. Symfony called it NotPwned but that's not in line with the naming conventions for validators.

I was thinking about the following names:

  • passwordBreach
  • compromisedCredentials
  • commonUsedPassword

Where passwordBreach is my personal favourite. Any suggestions?

@Ocramius
Copy link
Member

Ocramius commented Apr 5, 2019 via email

@DragonBe
Copy link
Contributor Author

DragonBe commented Apr 6, 2019

Let's turn your proposal into a subject form "UndisclosedPassword" to match the other validators.

@DragonBe
Copy link
Contributor Author

DragonBe commented Apr 6, 2019

Hey @Ocramius,

I've got a first PoC as a test provided. In this use case I'm using my own hibp package to prevent maintaining 2 versions. I need to update my package first to ensure it implements PSR-7 and PSR-18 so users can make use of their own clients instead of the provided GuzzleHttp that's part of my package. Is this an approach you find OK or do you prefer to have all that logic build as a separate zend component?

How far do you want this component to deliver validation? The basic functionality is to use the provided API by Have I been pwned, but there's also an offline validation possible using downloaded password hash files. This functionality is for users who want to prevent sending password hash snippets over the wire.

@DragonBe
Copy link
Contributor Author

@Ocramius I have an issue for your suggestion implementing PSR18 for PHP 5.6. Since 5.6 is still a version supported by Zend Framework, I consider this a serious issue.

  [InvalidArgumentException]
  Package psr/http-client at version  has a PHP requirement incompatible with your PHP version (5.6.40)

Do you want me to remove the dependency for http-client or is support for 5.6 planned to be removed from ZF? Please advice.

@Ocramius
Copy link
Member

As mentioned in #264, I think we're long overdue with a PHP requirement bump. It is needed and it should just be done, but I'll still need @zendframework/community-review-team to ACK on it, since there is a policy (that I strongly disagree with) to keep the PHP dependency pinned until new major releases are performed.

@Ocramius
Copy link
Member

I raised zendframework/maintainers#59 about this

@DragonBe
Copy link
Contributor Author

I also requested in php-fig/http-client#12 to provide support for both php 5.6 as for 7.x as not everyone has the luxury to upgrade their systems. I know, it's a cheap shot, but if the minimum for 5.6 would have been supported like with http-message requiring a minimum of PHP 5.3.0 than at least everyone with older systems could make use of new features. 🤷‍♂️

@Ocramius
Copy link
Member

I disagree with actively supporting anything below 7.2 to date: old software should stick to old dependencies, or receive a brush-up like anything else.

I've noted that downgrading the dependencies is also not possible, since you'd have a major BC break in the interface: https://github.com/php-fig/http-client/issues/12#issuecomment-482897797

@DragonBe
Copy link
Contributor Author

I follow your chain of thoughts completely and I would love to agree with it fully. My experience is that most companies hire consultants/dev-teams for development of projects and have their hosting outsourced where requirement changes take months to accomplish if that's even the case. And most projects have been abandoned after delivery so no upgrades from those ends allows such an organisation to migrate over to the latest greatest. This is a process of decades, not years.

BTW, nothing prevents them to roll out a version 0.0.1 which doesn't break BC because it precedes it.

@Ocramius
Copy link
Member

What you just described is something I am extremely familiar with: trying to help by backporting is exactly like lending money to a gambling addict, sorry.

@DragonBe
Copy link
Contributor Author

No apologies needed, you make a fair point. 😄

I was looking more from the point of view where you were mowing the lawn of your neighbour because they lost their legs in a hit-and-run car accident and you just want to help out.

@Ocramius
Copy link
Member

That would be the consulting work to be done 😜

@javiereguiluz
Copy link

Minor comment: in case you missed it, Symfony ultimately renamed it from NotPwned to NotCompromisedPassword. Reference: symfony/symfony#30901

@DragonBe
Copy link
Contributor Author

@javiereguiluz most likely for the same reason we renamed our validator to "UndisclosedPassword", to make the name more meaningful for less tech savy users.

@weierophinney
Copy link
Member

This repository has been closed and moved to laminas/laminas-validator; a new issue has been opened at laminas/laminas-validator#2.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants