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.

Sign up for GitHub

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

Jump to bottom

WebHook signature check fails when using PSR-7 message header lines #1001

Closed
davidlung opened this issue Aug 27, 2020 · 4 comments
Closed

WebHook signature check fails when using PSR-7 message header lines #1001

davidlung opened this issue Aug 27, 2020 · 4 comments

Comments

@davidlung
Copy link

php version 7.4.7
stripe-php version: 7.49.0

Use case:

I'm using the stripe CLI to listen and forward webhooks to my dev application.
Before execute the webhook at my application, i have to verify the confidentiality of the requrest by passing the stripe-signature header and the webhook secret to Webhook::constructEvent.

Approach
I use a PSR-7 message object to get the http header line for stripe-signature and pass it to construct the event.

Problem
The stripe-signature header consists of multiple and comma separated values. The PSR-7 message explodes multiple header values and saves internal as array. When get the header line from PSR message, the message implodes these values back into a normalized comma separated single value, where as each comma (can) has a trailing whitespace.

e.g. from t=xxxxxx,v1=xxxxxxxx,v0=xxxxxxx to t=xxxxxx, v1=xxxxxxxx, v0=xxxxxxx

Stripe uses Stripe\WebhookSignature::getSignatures(header, scheme) to extract the header values by exploding by comma and '='. For getting the signatures, it checks for the given scheme e.g. "v1" to collect the value.

The sticking point is an prepended whitespace at the exploded value schemes. The check " v1" === "v1" fails and no signatures are collected which results in a verification error.

Solution
Use trim whitespace before compare scheme.

private static function getSignatures($header, $scheme)
{
        $signatures = [];
        $items = \explode(',', $header);

        foreach ($items as $item) {
            $itemParts = \explode('=', $item, 2);
            if (trim($itemParts[0]) === $scheme) {  // <----------- trim whitespaces
                \array_push($signatures, $itemParts[1]);
            }
        }

        return $signatures;
}
@ianjabour-stripe ianjabour-stripe mentioned this issue Aug 27, 2020
Merged
@ianjabour-stripe
Copy link
Contributor

ianjabour-stripe commented Aug 27, 2020 •
edited
Loading

@davidlung thank you for filing this issue, we have the fix in progress.

One thing we just wanted to clarify is the use case which is causing the failure. We just want to be sure we can thoroughly reproduce this on our end.

We are wondering why PSR-7 cannot preserve formatting and how difficult it would be to bypass the tool and use the original formatting from the header.

We hesitate to merge this change because when our client libraries check the webhook signature they expect to be passed the exact raw header that they got from Stripe, and we don't want to relax this restriction without a truly compelling reason.

@davidlung
Copy link
Author

This is not the point of PSR-7 directly, the most PSR-7 libraries (e.g. Guzzle, one of the most popular or psr-7 by Nyholm from Symfony core team) are considering the RFC for http messages which defines the following part (OWS, Optional whitespaces):

For protocol elements where optional whitespace is
preferred to improve readability, a sender SHOULD generate the
optional whitespace as a single SP

However, a lot of people using these libraries that implode multiple field values with whitespace, therefore this is not a individual issue which can be handled by the developer who uses this library.

RFC:
https://tools.ietf.org/html/rfc7230#section-3.2.3
Guzzle psr-7:
https://github.com/guzzle/psr7/blob/master/src/MessageTrait.php#L68
Nyholm:
https://github.com/Nyholm/psr7/blob/master/src/MessageTrait.php#L75

@ianjabour-stripe
Copy link
Contributor

@davidlung thank you for taking the time to file this issue and provide context around PSR-7. The team decided to merge in the suggested change which should hopefully unblock you.

@ilhanby
Copy link

ilhanby commented Dec 30, 2021

$event = \Stripe\Webhook::constructEvent(
$payload, $sig_header, 'example_webhook_secret', 600
);

This solved my problem, I hope it solves the problem of other people living in it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@davidlung @ilhanby @ianjabour-stripe