Skip to content

Conversation

@steven-fulll
Copy link

The issue

We had a case of invalid signature created by the bundle for another PHP app using this very same bundle to verify the signature. After some digging, there is an incompatibility between signature creation and signature verification :

  • Signature creation uses the following data :
    • Method
    • Host
    • Path
    • Body
    • TTL if required
  • Signature verification uses the following data :
    • Method
    • Host
    • Path
    • A url decoded version of the body
    • TTL if required
      This means that if the body contains any character that can be changed by this function (from %00 to %FF) the signature will always be invalid. This rawurldecoded already existed in the Rezzza bundle, and this seems to be a bug.

Explored solutions

Remove the rawurldecoded in the listener

This seems to be the healthiest solution but this might create a signature incompatibility with how the signature is verified today, mostly for products that does not use this bundle (node, front, etc...).

Ignore it and leave the responsability to the products that creates the signature to fix it

The problem with this solution is mainly with the Psr7RequestSigner that requires a RequestInterface and attach the signature to it. To correctly use it, we have to create a Request with the rawurldecoded data, use the signer, get the signature from this request and attach it to a second request with the real data.

Sign the request with rawurldecoded data

This is the chosen solution, a compromise between no BC-break and code usability.

BC-Break ?

This seems like a BC-Break solution because it changes how the signature is generated, but it simply couldn't work before.

For body without values that are between %00 and %FF the signature will be exactly the same.
For body with thoses values, the signature will be from invalid to valid for this very same bundle.

$this->method,
$this->host,
$this->pathInfo,
rawurldecode($this->content),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you can only add a boolean to the rawurldecode function or not and not duplicate all the code no ?

@steven-fulll steven-fulll merged commit 68ccb7d into master Jun 25, 2025
4 checks passed
@steven-fulll steven-fulll deleted the fix-sign-request-rawurldecode branch June 25, 2025 12:07
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

Successfully merging this pull request may close these issues.

6 participants