Teleport: Permissions for Teleport state directory too open

[russjones]

From https://github.com/gravitational/teleport/issues/3686.

/var/lib/teleport is automatically created by teleport with 755 permissions without good reason.

[awly]

/var/lib/teleport is created with 0700 since https://github.com/gravitational/teleport/pull/427
I confirmed this locally.

Some files under it do get created with more open permissions (in particular, sqlite databases), but that shouldn't matter.

Are there specific repro steps from the user @klizhentas ?

[webvictim]

/var/lib/teleport is automatically created by teleport with 755 permissions without good reason.

Yeah, this just isn't true. I've tested 5 of my own Teleport nodes and they all have /var/lib/teleport chmod 700.

[awly]

@travelton pointed out that we have a bunch of other ways users could create /var/lib/teleport, like the AMI scripts.

I'll fix those, and we'll confirm with the user how they created this.

[Planktonette]

To add onto this, here's the code referenced in the email https://github.com/gravitational/teleport/blob/master/lib/service/service.go#L526-L533:

```// create the data directory if it's missing
_, err = os.Stat(cfg.DataDir)
if os.IsNotExist(err) {
err := os.MkdirAll(cfg.DataDir, os.ModeDir|0700)
if err != nil {
return nil, trace.ConvertSystemError(err)
}
}

The key thing here is os.IsNotExist. Unfortunately it already exists in the deb you guys provide, and has the wrong perms set there:

dpkg -c teleport-ent_4.2.8_amd64.deb

drwxr-xr-x 0/0 0 2020-04-01 12:50 ./
drwxr-xr-x 0/0 0 2020-04-01 12:50 ./usr/
drwxr-xr-x 0/0 0 2020-04-01 12:50 ./usr/share/
drwxr-xr-x 0/0 0 2020-04-01 12:50 ./usr/share/doc/
drwxr-xr-x 0/0 0 2020-04-01 12:50 ./usr/share/doc/teleport-ent/
-rw-r--r-- 0/0 143 2020-04-01 12:50 ./usr/share/doc/teleport-ent/changelog.gz
drwxr-xr-x 0/0 0 2020-04-01 12:50 ./usr/local/
drwxr-xr-x 0/0 0 2020-04-01 12:50 ./usr/local/bin/
-rwxr-xr-x 0/0 38433304 2020-04-01 11:51 ./usr/local/bin/tctl
-rwxr-xr-x 0/0 52631951 2020-04-01 11:51 ./usr/local/bin/teleport
-rwxr-xr-x 0/0 26734520 2020-04-01 11:51 ./usr/local/bin/tsh
drwxr-xr-x 0/0 0 2020-04-01 12:50 ./var/
drwxr-xr-x 0/0 0 2020-04-01 12:50 ./var/lib/
drwxr-xr-x 0/0 0 2020-04-01 12:50 ./var/lib/teleport/
drwxr-xr-x 0/0 0 2020-04-01 12:50 ./lib/
drwxr-xr-x 0/0 0 2020-04-01 12:50 ./lib/systemd/
drwxr-xr-x 0/0 0 2020-04-01 12:50 ./lib/systemd/system/
-rw-r--r-- 0/0 271 2020-04-01 11:51 ./lib/systemd/system/teleport.service

Here's the fips rpm:

rpm -qp --dump /home/opadmin/teleport-ent-4.2.9-1-fips.x86_64.rpm

/lib/systemd/system/teleport.service 271 1588715992 51f23f3bbd178938d036847c30eca5be 0100644 root root 0 0 0 X
/usr/local/bin/tctl 39918176 1588715992 951481ac10203dc45cd8c8124de25f59 0100755 root root 0 0 0 X
/usr/local/bin/teleport 53915607 1588715993 dc503f31b1170561e9fe56ba905a974b 0100755 root root 0 0 0 X
/usr/local/bin/tsh 28252672 1588715993 670befcecd7f27c2234d0220d1db9b0c 0100755 root root 0 0 0 X
/var/lib/teleport 4096 1588715993 00000000000000000000000000000000 040755 root root 0 0 0 X
```

Directory perms for /var/lib/teleport are world readable in both. I presume this affects all of your packages.

I'm guessing this is only an issue for package based installs.

[webvictim]

Thanks @Planktonette for the detailed explanation, you're right. It seems I didn't notice because I'd already installed Teleport on all the test boxes in the past before I started installing from packages, so the Teleport binary created the directory with mode 700 and then the RPM didn't overwrite it. I'll fix the package build scripts.

[dylan-ferreira]

Some files under it do get created with more open permissions (in particular, sqlite databases), but that shouldn't matter.

I disagree here. It's trivial to enforce appropriate file permissions so why not do it? As demonstrated in this ticket, all it takes is a package deployment bug and this data world readable.

[awly]

Some files under it do get created with more open permissions (in particular, sqlite databases), but that shouldn't matter.

I disagree here. It's trivial to enforce appropriate file permissions so why not do it? As demonstrated in this ticket, all it takes is a package deployment bug and this data world readable.

Good point, we should use defense in depth where possible.

I'll try to pick off low-hanging fruit where permission defaults are easily accessible. Where it requires plumbing through many layers of (possibly vendored) libraries, return on investment might be too low.