Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error loading a wrong certificate with PHP 7.4 #1456

Closed
jjanvier opened this issue Feb 24, 2020 · 10 comments
Closed

Error loading a wrong certificate with PHP 7.4 #1456

jjanvier opened this issue Feb 24, 2020 · 10 comments

Comments

@jjanvier
Copy link

jjanvier commented Feb 24, 2020

With PHP 7.4 and phpseclib 2.0.24, we encounter the following problem:

Trying to access array offset on value of type bool in  /srv/pim/vendor/phpseclib/phpseclib/phpseclib/File/ASN1.php line 534

This error comes from one of our test (it_adds_a_violation_if_the_certificate_is_wrongly_formatted) that triggers underneath this code:

    if (!$this->x509->loadX509('a string to represent a wrongly formatted certificate')) {
            $this->context
                ->buildViolation($constraint->invalidMessage)
                ->addViolation()
            ;

            return;
        }

The method ASN1::asn1map receives false as value for $decoded. Which then makes the method crash on line 534.
This comes from X509::loadX509 where the method decodeBER returns [0 => false]. See here.

Sadly, I can't propose a fix as I don't really get what's going on here.

@terrafrost
Copy link
Member

Can you provide a copy of the certificate that's causing the issue? This will enable me to implement a fix and to then include a unit test for it.

Thanks!

@jjanvier
Copy link
Author

The problem occurs when it's not a certificate that is provided, but a "random" string instead. For instance a string to represent a wrongly formatted certificate like in my example.

I'd rather have a real "business" exception thrown than this PHP error. Or maybe there is way to validate that a string is a certificate before calling this->x509->loadX509($certificate)?

@terrafrost
Copy link
Member

As an update to this... I'm able to reproduce the issue. I'll try to have a fix in place this weekend.

Thanks!

@jjanvier
Copy link
Author

Thank you very much @terrafrost !

@terrafrost
Copy link
Member

fc0832a should fix this.

The commit immediately before that one has a unit test for this.

Thanks!

@jjanvier
Copy link
Author

jjanvier commented Mar 3, 2020

Thank you very much!

@jjanvier
Copy link
Author

Hello @terrafrost

Will this bug fix be available with the next 2.0.26? And if yes, do you have an ETA for it?

@terrafrost
Copy link
Member

Will this bug fix be available with the next 2.0.26?

Yes.

And if yes, do you have an ETA for it?

I'll try to do it this week.

Thanks!

@jjanvier
Copy link
Author

Perfect, thank you very much 🥇

@terrafrost
Copy link
Member

Perfect, thank you very much 🥇

2.0.26 has been released - thanks!

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

No branches or pull requests

2 participants