search

Yubico / php-u2flib-server

(OBSOLETE) U2F library in PHP
https://developers.yubico.com/php-u2flib-server/
BSD 2-Clause "Simplified" License
288 stars 68 forks source link

Type safety, CSPRNG, avoid chr() #64

Closed paragonie-scott closed 6 years ago

paragonie-scott commented 6 years ago

https://paragonie.com/blog/2017/02/cryptographically-secure-php-development#chr

paragonie-scott commented 6 years ago

I got this in an email but don't see the comment attached to it:

Question to the maintainers: This change will break compatibility with older PHP versions < 7.0 I'm not up to speed with the development guidelines when version compatibility is concerned. Is this something to consider here, or at least to mention in the upcoming release :)

If the comment was not removed due to realizing the error, I'd just like to point out that random_compat takes care of PHP 5 compatibility with random_bytes().

MKodde commented 6 years ago

Yep, that was me :smile: After writing that comment I realized the random_compat polyfill was also added to the project.

paragonie-scott commented 6 years ago

@klali Is Yubico continuing to support their open source libraries? I've not seen any movement on any of the outstanding issues or pull requests in some time.

Considering that PHP powers a lot of the Internet, I feel that neglecting this market will negatively impact the security of the Internet as a whole.

klali commented 6 years ago

Thanks for poking me.

I would say that the U2F libraries currently existing is in a sort of maintainance mode with webauthn happening. With that said we should be clearer on that and communicating what we consider should be happening with them.

As for this pull request I'm fine with most of it. How important do you consider the switch from openssl_random_pseudo_bytes() ? I'd like to avoid adding more dependencies if we don't have to.

paragonie-scott commented 6 years ago

I would say that the U2F libraries currently existing is in a sort of maintainance mode with webauthn happening.

Funny that you should mention that...

How important do you consider the switch from openssl_random_pseudo_bytes() ?

OpenSSL's userspace PRNG has caused collisions in PHP software before. Your users are much better served by the kernel's CSPRNG, which is what random_compat uses (and what PHP 7+ does, natively).

klali commented 6 years ago

Agreed the points you make there, yet webauthn seems to be happening in the browsers which is the framework we have to work in here.

Fair enough on prng, going to merge this. Thank you!

klali commented 6 years ago

So the psalm checks seem to fail now that this is merged (https://travis-ci.org/Yubico/php-u2flib-server/jobs/419535637) but worked on the pull-request, do you have an inkling as to what's going on here?