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

Secret values on object in an array are not masked inside a pack config. #4139

Closed
NikosVlagoidis opened this issue May 22, 2018 · 5 comments
Closed
Milestone

Comments

@NikosVlagoidis
Copy link
Contributor

NikosVlagoidis commented May 22, 2018

ISSUE TYPE
  • Bug Report
STACKSTORM VERSION
st2 2.8dev (0f1d9dc), on Python 2.7.6
OS / ENVIRONMENT / INSTALL METHOD

docker instalation

SUMMARY

When I mark a secret value on a pack config inside an array I get the values in plain text even if Ii don't make the request with parameter show_secret=true.

STEPS TO REPRODUCE

Pack config

---
  instance:
    type: "array"
    required: false
    items:
      type: "object"
      properties:
          alias:
              description: "an alias for this instance"
              type: "string"
              required: true
              secret: false
          base_url:
              description: "Base URL for a service"
              type: "string"
              secret: false
              required: true
          secret:
              description: "The api secret key"
              type: "string"
              secret: true
              required: true

when I make a request on : https://{{host}}/api/v1/configs/<pack_name>

{
    "values": {
        "instance": [
            {
                "alias": "alias1",
                "secret": "secret",
                "base_url": "10.10.10.1"
            },
            {
                "alias": "alias2",
                "secret": "secret23",
                "base_url": "10.10.10.2"
            }
        ]
    },
    "id": "5afadbb2b2c724049ee7e03c",
    "pack": "<pack_name>"
}
EXPECTED RESULTS
{
    "values": {
        "instance": [
            {
                "alias": "alias1",
                "secret": "****",
                "base_url": "10.10.10.1"
            },
            {
                "alias": "alias2",
                "secret": "*****",
                "base_url": "10.10.10.2"
            }
        ]
    },
    "id": "5afadbb2b2c724049ee7e03c",
    "pack": "<pack_name>"
}
ACTUAL RESULTS
{
    "values": {
        "instance": [
            {
                "alias": "alias1",
                "secret": "secret",
                "base_url": "10.10.10.1"
            },
            {
                "alias": "alias2",
                "secret": "secret23",
                "base_url": "10.10.10.2"
            }
        ]
    },
    "id": "5afadbb2b2c724049ee7e03c",
    "pack": "<pack_name>"
}
@nmaludy
Copy link
Member

nmaludy commented May 22, 2018

Probably related to #4122

Wonder if the secret masking process for the CLI / API needs the same modifications?

@Kami
Copy link
Member

Kami commented May 22, 2018

@NikosVlagoidis Thanks for reporting this.

@nmaludy Yeah, it's most likely related to that change, but not to that code directly (that code is used in places where decrypted values are expected, API follows a different code path - just retrieves object from the database and masks secrets).

We probably need to update BaseAPI class / PackConfigAPI class to correctly mask secrets for nested objects / lists.

Edit: st2common.models.db.pack.PackDB.mask_secrets is the method which needs to be updated.

That's also a good example which shows why it's important to think about all the various edge cases for tests when adding a new feature / similar and not just for simple "positive" test cases :)

@nmaludy Will you have time to look into it? Or want me to do it?

@Kami Kami added this to the 2.8.0 milestone May 22, 2018
@nmaludy
Copy link
Member

nmaludy commented May 22, 2018

@Kami i'm checking it out now, looks like the affected functions are get_secret_parameters() and mask_secret_parameters() here https://github.com/StackStorm/st2/blob/master/st2common/st2common/util/secrets.py#L28-L61

@arm4b arm4b added the bug label May 22, 2018
@Kami
Copy link
Member

Kami commented May 22, 2018

@armab Thanks for assigning the "bug" label - I was actually thinking of doing it myself, but IIRC, since nested objects were not supported in stable versions prior to v2.8dev, it's not a bug in a stable release, just a bug in current dev version so I decided to leave it out for now.

Anyway, yeah, it's still a bug even though just in a dev release so it doesn't hurt to have that label.

nmaludy added a commit to nmaludy/st2 that referenced this issue May 22, 2018
nmaludy added a commit to nmaludy/st2 that referenced this issue May 24, 2018
@arm4b arm4b added the security label May 24, 2018
Kami added a commit that referenced this issue May 24, 2018
Fix for #4139 - Secrets not being masked in pack config
@LindsayHill
Copy link
Contributor

Looks like this is fixed in #4140/#4236

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants