Skip to content

353857: Filter environment variable logs

Alex Kalderimis requested to merge 353857-env-vars-api-not-filtered into master

What does this do any why?

See also: !81616 (closed), which this MR replaces.

Related issue: #353857 (closed)

This adjusts parameter filtering to mark some parameters as safe or unsafe explicitly on a per-route basis. This is required in order to correctly filter sensitive keys and values when submitted to any of the three environment variables API endpoints (instance, group, and project).

API Log examples

Bug behavior

*** /var/log/gitlab/api_json.log ***) { ... "status":200,"method":"PUT","path":"/api/v4/projects/5/variables/TEST_SSH_KEY","params":[{"key":"masked","value":null},{"key":"variable_type","value":"file"},{"key":"value","value":"-----BEGIN OPENSSH PRIVATE KEY-----" .. }

Expected behavior

*** /var/log/gitlab/api_json.log ***) { ... "status":200,"method":"PUT","path":"/api/v4/projects/5/variables/TEST_SSH_KEY","params":[{"key":"TEST_SSH_KEY","value":"[FILTERED]"},{"key":"variable_type","value":"file"} .. }

See my description in issue #353857 (closed) . As is, if the user submits an environment variable key with a name like 'TEST_SSH_KEY', the param 'key' has it's value masked, but the param 'value' is actually printed to the log. Which in the case of a personal SSH key, is a security issue.

How to set up and validate locally

  1. Start with a freshly installed GDK.
  2. Create a user with the admin role.
  3. Create a group and project for that user.
  4. Create an access token with admin api scope.
  5. Send a POST request to the Project-level variables API

curl --request POST --header "PRIVATE-TOKEN: <your_access_token>" \ "https://<gitlab.example.com>/api/v4/projects/1/variables" --form "key=TEST_SSH_KEY" --form "value=-----BEGIN OPENSSH PRIVATE KEY----- This isn't actually a private key, just some testing"

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #353857 (closed) @gitlab-com/gl-security/appsec security

Edited by Alex Kalderimis

Merge request reports