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

The library silently fails when signing a token in case of openssl error #132

Closed
magnetik opened this issue Oct 27, 2016 · 6 comments
Closed
Labels
Milestone

Comments

@magnetik
Copy link

In the file src/Signer/Rsa.php, in the method createHash, the output of the method openssl_sign is not used.

In case of error (mine was RSA_sign:digest too big for rsa key) the processing continues and it makes the debugging quite difficult.

@lcobucci lcobucci added the Bug label Oct 28, 2016
@lcobucci
Copy link
Owner

@magnetik thanks, in order to be BC compatible I'll throw an InvalidArgumentException there with the openssl_error_string() as message.

@lcobucci lcobucci added this to the 3.2.1 milestone Oct 28, 2016
@magnetik
Copy link
Author

Awesome 👍

@lcobucci lcobucci modified the milestones: 4.0.0, 3.3.0 Oct 28, 2016
@lcobucci
Copy link
Owner

@magnetik do you have an example that fails so we could create a test for that?

@magnetik
Copy link
Author

<?php

$key = "-----BEGIN RSA PRIVATE KEY-----
MGECAQACEQC4MRKSVsq5XnRBrJoX6+rnAgMBAAECECO8SZkgw6Yg66A6SUly/3kC
CQDtPXZtCQWJuwIJAMbBu17GDOrFAggopfhNlFcjkwIIVjb7G+U0/TECCEERyvxP
TWdN
-----END RSA PRIVATE KEY-----";

$opensslKey = openssl_pkey_get_private($key);

$signature = null;
$result = openssl_sign("foobar", $signature, $opensslKey);
var_dump($result);
var_dump(openssl_error_string());

@lcobucci
Copy link
Owner

Nice, thanks ;)

lcobucci added a commit that referenced this issue Oct 31, 2016
@lcobucci
Copy link
Owner

I've added the validation, tested "manually" and already release 3.2.1 with that patch.
#135 will apply the same thing for v4 that will be done after #129.

Thanks again @magnetik!

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

No branches or pull requests

2 participants