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

Secrets leaked by util.Secret #4007

Closed
jugglinmike opened this issue Mar 20, 2018 · 3 comments · Fixed by #4701
Closed

Secrets leaked by util.Secret #4007

jugglinmike opened this issue Mar 20, 2018 · 3 comments · Fixed by #4701

Comments

@jugglinmike
Copy link
Contributor

The util.Secret API is designed to allow users to reference sensitive information without exposing it in log files. In Buildbot versoin 1.1.0, the data is incorrectly emitted to the log files if it has not previously been rendered via the util.Interpolate API.

I've included the contents of a master.cfg file which demonstrates this behavior and a screenshot of a build created using that file.

$ buildbot --version
Buildbot version: 1.1.0
Twisted version: 17.9.0

(Installed via pip)

import os
from buildbot.plugins import *

secrets_dir = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'secrets')

try:
    os.mkdir(secrets_dir)
except OSError:
    pass

with open('%s/my_secret' % secrets_dir, 'w') as handle:
    handle.write('this is a secret')

os.chmod('%s/my_secret' % secrets_dir, 0600)

secret = util.Secret('my_secret')
interpolation = util.Interpolate('%(secret:my_secret)s')
demo_factory = util.BuildFactory([
    steps.ShellCommand(name='Debug ShellCommand / util.Secret #1',
                       command=['echo', secret]),
    steps.ShellCommand(name='Debug ShellCommand / util.Secret #2',
                       command=['echo', secret]),
    steps.ShellCommand(name='Debug ShellCommand / util.Interpolate',
                       command=['echo', interpolation]),
    steps.ShellCommand(name='Debug ShellCommand / util.Secret #3',
                       command=['echo', secret]),
])

c = BuildmasterConfig = {}
c['schedulers'] = [
  schedulers.ForceScheduler(name='demo', builderNames=['demoBuilder'])
]
c['builders'] = [
    util.BuilderConfig(name='demoBuilder',
                       workernames=['moe'],
                       factory=demo_factory)
]
c['secretsProviders'] = [secrets.SecretInAFile(dirname=secrets_dir)]
c['workers'] = [worker.LocalWorker('moe')]
c['protocols'] = {'pb': {'port': 9989}}
c['services'] = []
c['title'] = 'Demo'
c['titleURL'] = 'Demo'
c['buildbotURL'] = 'http://localhost/'
c['www'] = {'port': 80}
c['db'] = {'db_url': 'sqlite:///state.sqlite'}

screenshot-2018-3-20 buildbot builder demobuilder build 8

@tardyp
Copy link
Member

tardyp commented Mar 21, 2018

Hi good findings.

Would you like to make a patch for this issue?

It is about adding the line:

    properties.useSecret(secret_detail.value, self.secret_name)

from:
https://github.com/buildbot/buildbot/blob/master/master/buildbot/process/properties.py#L500

in line 524:
https://github.com/buildbot/buildbot/blob/master/master/buildbot/process/properties.py#L524

the fix the integration test to also test that Secret is not leaking values:

https://github.com/buildbot/buildbot/blob/master/master/buildbot/test/integration/interop/test_integration_secrets.py#L90

@endrift
Copy link

endrift commented Feb 28, 2019

Hi, I just had to roll a password because of this. Please fix it, considering you seem to already know how to.

@jugglinmike
Copy link
Contributor Author

Thanks!

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

Successfully merging a pull request may close this issue.

3 participants