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

Concourse Seems to Expose Secrets on Web Nodes #2415

Open
ryanaross opened this Issue Jul 25, 2018 · 7 comments

Comments

5 participants
@ryanaross
Copy link

ryanaross commented Jul 25, 2018

Bug Report

  • Concourse version: 3.13.0
  • Deployment type (BOSH/Docker/binary): BOSH / concourse-release

Hello Concourse devs,

Great product, but am having an issue with plaintext communication between web and workers.

Here is a sample of output from running
tcpdump -i eth0 'port 7777' -X
On a 'web' instance:

17:15:25.387058 IP c3b9-XXXX-asdgd-9f64-4b70-bfd4-903bd604509f.web.default.concourse.bosh.34000 > 1663-e497-4b98-XXXXX-8db2-e60b3997cc55.worker.default.concourse.bosh.7777: Flags [P.], seq 252:713, ack 196, win 229, options [nop,nop,TS val 1632542092 ecr 164360566], length 460
	0x0000:  4500 ... E.....@.@.......
	0x0010:  0a83 ...  ..."...a.Pg.....
	0x0020:  8018 ... ....3%........i.
	0x0030:  00fb ...  ..+.{"process_id
	0x0040:  223a ... ":"xxxxxxx-xxxx
	0x0050:  2d34 ... -xxxx-xxxx-xxxxx
	0x0060:  3035 ...  xxxxx","source
	0x0070:  223a ...  ":0,"data":"{\"s
	0x0080:  6f75 ... ource\":{\"acces
	0x0090:  735f ... s_key_id\":\"X\",
	0x00b0:  5c22 ... \"bucket\":\"con
	0x00c0:  636f ... course-backups\"
	0x00d0:  2c5c ...  ,\"disable_ssl\"
	0x00e0:  3a66 ... :false,\"endpoin
	0x00f0:  745c  ... t\":\"xxxxxxxxxx
	0x0100:  3030 ...  xxxxx.xxxxxxxx.
	0x0110:  636f ... xxxx\",\"regexp\"
	0x0120:  3a5c ... :\"xxxxxxxxxxxxx
	0x0130:  6c6f  ... xxxxxxxxxxxxxxxx
	0x0140:  7379 ... sys.io/(.*).tgz\
	0x0150:  222c ... ",\"secret_acces
	0x0160:  735f ... s_key\":\"XXXXXX
	0x0170:  3030 ... XXXXXXXXXXXXXXXX
	0x0180:  3837 ... XXXXXXXXXXXXXXXX
	0x0190:  4c43 ... XXXXXXXX\"},\"ve
	0x01a0:  7273 ... rsion\":{\"path\
	0x01b0:  223a ... ":\"xxxxxxxxxxxx
	0x01c0:  726c ... xxxxxxxxxxxxxxxx
	0x01d0:  6273 ... xxxxxxx/concours
	0x01e0:  652d ... e-atc-db-dump-20
	0x01f0:  3138 ... 180724.tgz\"}}"}
	0x0200:  0a                           

As you can see, there are secrets (parameters sent to concourse task) exposed in plaintext which I have x'd out.

Here is the BOSH manifest:

---
instance_groups:
- azs:
  - z1
  instances: 1
  jobs:
  - name: postgres
    properties:
      databases:
        databases:
        - name: atc
        port: 5432
        roles:
        - name: atc
          password: "((/bosh/concourse/atc-db-password))"
    release: postgres
  name: db
  networks:
  - name: default
  persistent_disk_type: large
  stemcell: default
  vm_type: large
- azs:
  - z1
  instances: 2
  jobs:
  - name: atc
    properties:
      basic_auth_password: "((atc_basic_auth.password))"
      basic_auth_username: "((atc_basic_auth.username))"
      credhub:
        client_id: concourse_credhub_id
        client_secret: "((client_secret))"
        tls:
          ca_cert:
            certificate: "((xxxx.certificate))"
          insecure_skip_verify: false
        url: https://xxxxxxx:8844
      encryption_key: "((encryption-key))"
      external_url: https://xxxxxxxxx:443
      postgresql:
        database: atc
        host: xxxxxxxxxx
        role:
          name: atc
          password: "((atc-db-password))"
        sslmode: disable
      tls_bind_port: 443
      tls_cert: "((xxxxxxx.certificate))"
      tls_key: "((xxxxxxxx.private_key))"
      token_signing_key: "((token_signing_key))"
    release: concourse
  - name: tsa
    properties:
      authorized_keys:
      - "((worker_key.public_key))"
      host_key: "((tsa_host_key))"
      token_signing_key: "((token_signing_key))"
    release: concourse
  name: web
  networks:
  - name: default
  stemcell: default
  vm_type: large
- azs:
  - z1
  instances: 2
  jobs:
  - consumes:
      baggageclaim:
        from: worker-baggageclaim
    name: worker
    properties:
      drain_timeout: 10m
      tsa:
        worker_key: "((worker_key))"
    release: concourse
  - name: baggageclaim
    properties:
      log_level: debug
    provides:
      baggageclaim:
        as: worker-baggageclaim
    release: concourse
  - name: garden
    properties:
      garden:
        listen_address: 0.0.0.0:7777
        listen_network: tcp
    release: garden-runc
  name: worker
  name: worker
  networks:
  - name: default
  stemcell: default
  vm_type: large
name: concourse
releases:
- name: concourse
  version: 3.13.0
- name: garden-runc
  version: 1.15.1
- name: postgres
  version: '28'
stemcells:
- alias: default
  os: ubuntu-trusty
  version: '3586.26'
update:
  canaries: 1
  canary_watch_time: 30000-1200000
  max_in_flight: 1
  serial: true
  update_watch_time: 10000-1200000

@vito vito added the security label Jul 31, 2018

@vito vito added this to Icebox in Operations via automation Jul 31, 2018

@vito vito moved this from Icebox to Backlog in Operations Jul 31, 2018

@vito

This comment has been minimized.

Copy link
Member

vito commented Jul 31, 2018

Yeah, I've been wanting to fix this for a while. :/ Right now Garden can't be configured to listen via TLS, since it's usually configured to listen on a local Unix domain socket instead. We're the only ones that really use it this way.

There's also the concern of there being no auth to Garden's API, so even with TLS it wouldn't be enough to really comfortably "lock it down" within the network (unless we do mutual TLS, which ain't a bad idea).

So in the long run it might be best to simply move away from direct worker registration, require workers to register via the TSA, and have the TSA's tunneled listeners listen via TLS with some reasonable auth.

@jraffin

This comment has been minimized.

Copy link

jraffin commented Sep 27, 2018

Hi @vito, I like the "workers to register via the TSA, and have the TSA's tunneled listeners listen via TLS with some reasonable auth." approach.

@jama22 jama22 added the size/large label Oct 1, 2018

@xtreme-sameer-vohra xtreme-sameer-vohra moved this from Backlog to In Flight in Operations Oct 1, 2018

@xtreme-sameer-vohra xtreme-sameer-vohra moved this from In Flight to Backlog in Operations Oct 1, 2018

@jama22

This comment has been minimized.

Copy link
Member

jama22 commented Oct 22, 2018

How about:

  • Configure concourse web with --worker-gateway-ca-cert and --worker-gateway-ca-key
  • On start, concourse web could use the key to generate a cert for itself, because otherwise we'd need to know the IP ahead of time and we don't want to rely on static IPs for the worker pool
@jama22

This comment has been minimized.

Copy link
Member

jama22 commented Nov 12, 2018

As per @vito 's recent refactor of the TSA in #2774, everything uses forward mode, so we can ignore some of my previous point of switching everything to forwarding.

@cirocosta

This comment has been minimized.

Copy link
Member

cirocosta commented Jan 21, 2019

Hey @jama22,

Should we consider this done as of #2748 ? Or do we have anything missing? There (in the PR) the direct registration method got removed.

Asking this because the issue is still in the operations backlog.

Thanks!

@vito

This comment has been minimized.

Copy link
Member

vito commented Jan 21, 2019

@cirocosta This is still not fixed because the ATC -> TSA forwarded addresses is still done over plaintext.

@cirocosta

This comment has been minimized.

Copy link
Member

cirocosta commented Jan 21, 2019

Oh I see, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.