1
0
Fork 0
mirror of https://github.com/moby/moby.git synced 2022-11-09 12:21:53 -05:00
Commit graph

8 commits

Author SHA1 Message Date
Sebastiaan van Stijn
f8a0f26843 DebugRequestMiddleware: Remove path handling
Path-specific rules were removed, so this is no longer used.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 530e63c1a61b105a6f7fc143c5acb9b5cd87f958)
Signed-off-by: Tibor Vass <tibor@docker.com>
2019-07-09 21:42:12 +00:00
Sebastiaan van Stijn
73db8c77bf DebugRequestMiddleware: unconditionally scrub data field
Commit 77b8465d7e added a secret update
endpoint to allow updating labels on existing secrets. However, when
implementing the endpoint, the DebugRequestMiddleware was not updated
to scrub the Data field (as is being done when creating a secret).

When updating a secret (to set labels), the Data field should be either
`nil` (not set), or contain the same value as the existing secret. In
situations where the Data field is set, and the `dockerd` daemon is
running with debugging enabled / log-level debug, the base64-encoded
value of the secret is printed to the daemon logs.

The docker cli does not have a `docker secret update` command, but
when using `docker stack deploy`, the docker cli sends the secret
data both when _creating_ a stack, and when _updating_ a stack, thus
leaking the secret data if the daemon runs with debug enabled:

1. Start the daemon in debug-mode

        dockerd --debug

2. Initialize swarm

        docker swarm init

3. Create a file containing a secret

        echo secret > my_secret.txt

4. Create a docker-compose file using that secret

        cat > docker-compose.yml <<'EOF'
        version: "3.3"
        services:
          web:
            image: nginx:alpine
            secrets:
              - my_secret
        secrets:
          my_secret:
            file: ./my_secret.txt
        EOF

5. Deploy the stack

        docker stack deploy -c docker-compose.yml test

6. Verify that the secret is scrubbed in the daemon logs

        DEBU[2019-07-01T22:36:08.170617400Z] Calling POST /v1.30/secrets/create
        DEBU[2019-07-01T22:36:08.171364900Z] form data: {"Data":"*****","Labels":{"com.docker.stack.namespace":"test"},"Name":"test_my_secret"}

7. Re-deploy the stack to trigger an "update"

        docker stack deploy -c docker-compose.yml test

8. Notice that this time, the Data field is not scrubbed, and the base64-encoded secret is logged

        DEBU[2019-07-01T22:37:35.828819400Z] Calling POST /v1.30/secrets/w3hgvwpzl8yooq5ctnyp71v52/update?version=34
        DEBU[2019-07-01T22:37:35.829993700Z] form data: {"Data":"c2VjcmV0Cg==","Labels":{"com.docker.stack.namespace":"test"},"Name":"test_my_secret"}

This patch modifies `maskSecretKeys` to unconditionally scrub `Data` fields.
Currently, only the `secrets` and `configs` endpoints use a field with this
name, and no other POST API endpoints use a data field, so scrubbing this
field unconditionally will only scrub requests for those endpoints.

If a new endpoint is added in future where this field should not be scrubbed,
we can re-introduce more fine-grained (path-specific) handling.

This patch introduces some change in behavior:

- In addition to secrets, requests to create or update _configs_ will
  now have their `Data` field scrubbed. Generally, the actual data should
  not be interesting for debugging, so likely will not be problematic.
  In addition, scrubbing this data for configs may actually be desirable,
  because (even though they are not explicitely designed for this purpose)
  configs may contain sensitive data (credentials inside a configuration
  file, e.g.).
- Requests that send key/value pairs as a "map" and that contain a
  key named "data", will see the value of that field scrubbed. This
  means that (e.g.) setting a `label` named `data` on a config, will
  scrub/mask the value of that label.
- Note that this is already the case for any label named `jointoken`,
  `password`, `secret`, `signingcakey`, or `unlockkey`.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit c7ce4be93ae8edd2da62a588e01c67313a4aba0c)
Signed-off-by: Tibor Vass <tibor@docker.com>
2019-07-09 21:42:12 +00:00
Sebastiaan van Stijn
ebb542b3f8 TestMaskSecretKeys: use subtests
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 32d70c7e21631224674cd60021d3ec908c2d888c)
Signed-off-by: Tibor Vass <tibor@docker.com>
2019-07-09 21:42:12 +00:00
Sebastiaan van Stijn
18dac2cf32 TestMaskSecretKeys: add more test-cases
Add tests for

- case-insensitive matching of fields
- recursive masking

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit db5f811216e70bcb4a10e477c1558d6c68f618c5)
Signed-off-by: Tibor Vass <tibor@docker.com>
2019-07-09 21:42:12 +00:00
Vincent Demeester
3845728524
Update tests to use gotest.tools 👼
Signed-off-by: Vincent Demeester <vincent@sbr.pm>
2018-06-13 09:04:30 +02:00
Daniel Nephin
6be0f70983 Automated migration using
gty-migrate-from-testify --ignore-build-tags

Signed-off-by: Daniel Nephin <dnephin@docker.com>
2018-03-16 11:03:43 -04:00
Daniel Nephin
4f0d95fa6e Add canonical import comment
Signed-off-by: Daniel Nephin <dnephin@docker.com>
2018-02-05 16:51:57 -05:00
Aaron Lehmann
3fbc352cbb middleware: Redact secret data on "secret create"
With debug logging turned on, we currently log the base64-encoded secret
payload.

Change the middleware code to redact this. Since the field is called
"Data", it requires some context-sensitivity. The URI path is examined
to see which route is being invoked.

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
2017-06-29 16:04:47 -07:00