Calling Developers!
We are reenergizing our code contribution process! Learn More

What are the Slack Archives?

It’s a history of our time together in the Slack Community! There’s a ton of knowledge in here, so feel free to search through the archives for a possible answer to your question.

Because this space is not active, you won’t be able to create a new post or comment here. If you have a question or want to start a discussion about something, head over to our categories and pick one to post in! You can always refer back to a post from Slack Archives if needed; just copy the link to use it as a reference..

Hi all, I am looking at the commit regarding the security issue in `spryker/http` specifically thi

Options
UM9F81RCP
UM9F81RCP Posts: 516 🧑🏻‍🚀 - Cadet

Hi all,

I am looking at the commit regarding the security issue in spryker/http specifically this part

shouldn’t we return at line 35? The environment variable must have higher priority right? If not, why don’t we just start by reading the config:

        $uriSignerSecret = $this->get(
            HttpConstants::URI_SIGNER_SECRET_KEY,
            $uriSignerSecret,
        );

Because in the config we can set it to read the environment and return null if the env is not set. In that case the configuration will return null and the error will be triggered.

Something like this:

$config[HttpConstants::URI_SIGNER_SECRET_KEY] = getenv('SPRYKER_ZED_REQUEST_TOKEN') ?: null;

Comments

  • U01F7P3D9NH
    U01F7P3D9NH Posts: 60 🧑🏻‍🚀 - Cadet
    Options

    First solution was revoked, with 1.7.1 you don´t have to add the constant to config

  • UM9F81RCP
    UM9F81RCP Posts: 516 🧑🏻‍🚀 - Cadet
    Options

    sorry I did not get it

  • U01F7P3D9NH
    U01F7P3D9NH Posts: 60 🧑🏻‍🚀 - Cadet
    Options

    Just remove the entries in config related to the secret, do the composer update and make sure spryker/http it is 1.7.1

  • UM9F81RCP
    UM9F81RCP Posts: 516 🧑🏻‍🚀 - Cadet
    Options

    by doing this we make hard dependency on an external environment variable

  • UM9F81RCP
    UM9F81RCP Posts: 516 🧑🏻‍🚀 - Cadet
    Options

    you meant removing the config constants and depending on the env vars only… did I get you right?

  • U01F7P3D9NH
    U01F7P3D9NH Posts: 60 🧑🏻‍🚀 - Cadet
    Options

    Are you in Spryker PaaS or different setup?

  • UM9F81RCP
    UM9F81RCP Posts: 516 🧑🏻‍🚀 - Cadet
    Options

    not in PaaS

  • U01F7P3D9NH
    U01F7P3D9NH Posts: 60 🧑🏻‍🚀 - Cadet
    Options

    ah, sorry (was talking about the PaaS). During error evaluation, I added the SPRYKER_ZED_REQUEST_TOKEN to my deploy file to pass the issue. But it was with 1.7.0

  • UK7KBE2JW
    UK7KBE2JW Posts: 463 🧑🏻‍🚀 - Cadet
    Options

    Is this already public or taken from email?
    Since:

    The information contained in this document is strictly confidential. It is intended only for the respective addressee and may not be disclosed and/or distributed without the prior consent of Spryker Systems GmbH
    
  • U01TZ93MPSQ
    U01TZ93MPSQ Posts: 40 🧑🏻‍🚀 - Cadet
    edited April 2022
    Options

    The code discussed here is public on GitHub

  • U01TZ93MPSQ
    U01TZ93MPSQ Posts: 40 🧑🏻‍🚀 - Cadet
    Options

    @U01F7P3D9NH You do still have to add a config variable. In the 1.7.0 version the config entry was pulling its value from the env variable. In 1.7.1 it wants you to set a random value on the config entry as well as a random entry for the env variable.

    https://github.com/spryker/http/blob/07b4e5361acbf190158fe2d6b92990030736c32b/src/Spryker/Shared/Http/HttpConfig.php#L39

    The error message below it still mentions the "old" way of doing it tho, which is just confusing but thats probably just a leftover.

    If you completely remove the $config[HttpConstants::URI_SIGNER_SECRET_KEY] it wont even work.

    This is also mentioned explicitly in the aforementioned secret email

  • U01TZ93MPSQ
    U01TZ93MPSQ Posts: 40 🧑🏻‍🚀 - Cadet
    edited April 2022
    Options

    But I agree with @UM9F81RCP. Why is the $default the environment variable and the config value is the one usually used, I would prefer not having this "secret" checked into git...

  • U01F7P3D9NH
    U01F7P3D9NH Posts: 60 🧑🏻‍🚀 - Cadet
    Options

    @U01TZ93MPSQ With 1.7.1. I don´t have to add it. In PaaS/AWS you can store secrets in a Parameter store which is referenced to the enviroment secrets in the containers

  • U01TZ93MPSQ
    U01TZ93MPSQ Posts: 40 🧑🏻‍🚀 - Cadet
    edited April 2022
    Options

    Ah I see where the confusion comes from then. We are on-Prem