Custom blacklisted headers on SignatureV4

[stephpy]

Describe the feature

BACKWARD COMPATIBLE

Hi,

At this moment, SignatureV4 use a defined list of blacklisted headers. See Method and it cannot be overriden by client.

I can understand in most of case we have not to sign theses headers but in my case I need to sign them.

Would you accept a change in this way ? I would make the PR.

Use Case

We develop an API (Symfony) + a FRONTEND (React) and for large files we decided than FRONTEND will directly push files to S3.

Workflow:

• FRONTEND asks for a presigned url to API with some data like content_type, content_length ...
• API validate the request and if user has rights to upload this kind of media
• If it's ok, API generate a presigned url and give it to FRONTEND
• FRONTEND uses this presigned to upload the media

If we don't sign headers like content-type, content-length, FRONTEND could push any other media ....

Proposed Solution

<?php

class SignatureV4 implements SignatureInterface
{
   // ....

    /**
     * The following headers are not signed because signing these headers
     * would potentially cause a signature mismatch when sending a request
     * through a proxy or if modified at the HTTP client level.
     *
     * @var array
     */
    private array $blacklistedHeaders = [
        'cache-control' => true,
        'content-type' => true,
        'content-length' => true,
        'expect' => true,
        'max-forwards' => true,
        'pragma' => true,
        'range' => true,
        'te' => true,
        'if-match' => true,
        'if-none-match' => true,
        'if-modified-since' => true,
        'if-unmodified-since' => true,
        'if-range' => true,
        'accept' => true,
        'authorization' => true,
        'proxy-authorization' => true,
        'from' => true,
        'referer' => true,
        'user-agent' => true,
        'X-Amz-User-Agent' => true,
        'x-amzn-trace-id' => true,
        'aws-sdk-invocation-id' => true,
        'aws-sdk-retry' => true,
    ];

   // .....

    private function getHeaderBlacklist()
    {
        return $this->blacklistedHeaders;
    }

    public function allowHeader($header)
    {
        unset($this->blacklistedHeaders[$header]);
    }

    public function disallowHeader($header)
    {
        $this->blacklistedHeaders[$header] = true;
    }

Then

<?php

        $signer->allowHeader('content-type');
        $signer->allowHeader('content-length');

At this moment the way to get the signer is not really easy, may we could add a getSigner method ?

Other Information

No response

Acknowledgements

• [X] I may be able to implement this feature request
• [ ] This feature might incur a breaking change

SDK version used

3.220.3

Environment details (Version of PHP (php -v)? OS name and version, etc.)

8.1

[SamRemis]

@stephpy, I'd consider it if it were a common use case; there is an option open for you in the SDK right now to implement your own signature provider.

I can move this to discussions and give you suggestions on how to override the signature provider in the SDK by extending ours and overriding the getHeaderBlacklist() method to return an array without those headers.

My problem with this suggested solution is that it may be hard for customers to implement considering that you can only input a signature_provider, not a signature object. How would the SignatureV4 object generated by the provider be able to know which headers to allow?

I'd think it would be easier for customers to put this on a SignatureProvider, but that may be a less elegant solution. What do you think?

[stephpy]

Indeed, SignatureProvider and SignatureInterface does not provide the getHeaderBlacklist as public api.

Doing this implementation on SignatureV4 would only allow user to implement SignatureV4 or verifying at usage it's this implementation ... Not a good way, may be ... you're right.

Anyway, at this moment this decision is part of a private API and not sure it has to (by default doing it is great but users like me may like to securize more theirs calls without engage library responsabilities). An easy way would be to put method as "protected" ... it could be overriden by any implementation and user could override this list by just creating a CustomClass which overrides SignatureV4 or its parents.

Thank you for your time @SamRemis.

[stephpy]

At this moment I have to copy/paste S3SignatureV4 and SignatureV4 which are awesome implementations only to override this private method, and not sure it's useful here to be private.

[stephpy]

Thoughts ?

[Lensual]

I need this for my API Gateway project with custom SignatureV4 validator.

[jaxwilko]

+1 having the same issue with the same use case, making the method protected would be a perfect solution :)

[LukeTowers]

@SamRemis can we get #2505 merged please? We are running into this exact same issue & use case at @WinterCMS & @SpatialMedia as we implement support for streamed uploads to our AWS Driver plugin for Winter CMS.

Literally the exact same use case 😂

[SamRemis]

@LukeTowers Sorry for the delay, I'm happy to merge that when there's a changelog :) I requested that as a change from the person who submitted the PR. If they don't get it within a reasonable timeframe, I can try to see if I can force push it to the repo, or make a separate PR with the changelog.

Thanks for tagging me and bringing my attention back to this

[LukeTowers]

@SamRemis awesome, thanks! Looking forward to having it merged so we can finalize the header security measures in our AWS driver plugin (https://github.com/wintercms/wn-driveraws-plugin)

[yenfryherrerafeliz]

Hi @stephpy, I will close this issue since the following PR was merged.

Thanks!

[github-actions[bot]]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.