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

Check if the openssl_sign went OK #1102

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

melroy89
Copy link
Member

@melroy89 melroy89 commented Sep 9, 2024

In case openssl_sign fails the $signature will be null. Resulting into errors like this:

{"message":"Warning: openssl_sign(): Supplied key param cannot be coerced into a private key","context":{"exception":{"class":"ErrorException","message":"Warning: openssl_sign(): Supplied key param cannot be coerced into a private key","code":0,"file":"/var/www/kbin.melroy.org/html/src/Service/ActivityPub/ApHttpClient.php:504"}},"level":400,"level_name":"ERROR","channel":"php","datetime":"2024-09-09T21:33:35.100575+02:00","extra":{}}
{"message":"Error thrown while handling message App\\Message\\ActivityPub\\Outbox\\DeliverMessage. Sending for retry #2 using 3600000 ms delay. Error: \"Handling \"App\\Message\\ActivityPub\\Outbox\\DeliverMessage\" failed: base64_encode(): Argument #1 ($string) must be of type string, null given\"","context":{"class":"App\\Message\\ActivityPub\\Outbox\\DeliverMessage","message_id":3420233,"retryCount":2,"delay":3600000,"error":"Handling \"App\\Message\\ActivityPub\\Outbox\\DeliverMessage\" failed: base64_encode(): Argument #1 ($string) must be of type string, null given","exception":{"class":"Symfony\\Component\\Messenger\\Exception\\HandlerFailedException","message":"Handling \"App\\Message\\ActivityPub\\Outbox\\DeliverMessage\" failed: base64_encode(): Argument #1 ($string) must be of type string, null given","code":0,"file":"/var/www/kbin.melroy.org/html/vendor/symfony/messenger/Middleware/HandleMessageMiddleware.php:124","previous":{"class":"TypeError","message":"base64_encode(): Argument #1 ($string) must be of type string, null given","code":0,"file":"/var/www/kbin.melroy.org/html/src/Service/ActivityPub/ApHttpClient.php:505"}}},"level":300,"level_name":"WARNING","channel":"messenger","datetime":"2024-09-09T21:33:35.100878+02:00","extra":{}}

We can better check on the return value of openssl_sign to validate if the signature actually succeeded or not.

If not, we now log a better error message. And we leave out the Signature header for now. We could also throw an error and abort. Ideas are welcome here.


Also we now free the memory of the $key using the appropriate openssl_free_key method.

@melroy89 melroy89 added backend Backend related issues and pull requests php Pull requests that update PHP code needs feedback Requires a greater consensus to make an informed decision labels Sep 9, 2024
@melroy89 melroy89 added this to the v1.7.2 milestone Sep 12, 2024
@melroy89 melroy89 added the bug Something isn't working label Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Backend related issues and pull requests bug Something isn't working needs feedback Requires a greater consensus to make an informed decision php Pull requests that update PHP code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant