cockroachdb / cockroach

CockroachDB — the cloud native, distributed SQL database designed for high availability, effortless scale, and control over data placement.
https://www.cockroachlabs.com
Other
29.88k stars 3.77k forks source link

Security check on certificate file permissions needs refinement #61975

Closed bryanhuntesl closed 3 years ago

bryanhuntesl commented 3 years ago

Describe the problem

The security check on certificate permissions needs refinement, it seems like best practice for ssh keys has been adapted but without considering certain use cases.

Client cert file is stored on NFS share.

File ownership is user=cockroachdb group=cockroachdb.

I want to have an unprivileged user have access to that file.

I add the user to the cockroachdb group and change the file permissions to 0640 (user rw, group r, other nothing).

I attempt to use the file to authenticate to the server.

The client throws an error, indicating that these security permissions are unacceptable.

To Reproduce

$ sudo usermod -a -G cockroachdb $(id -nu)
$ sudo chmod 640 /mnt/nfs_crdb/certs/node.key
$ sudo ls -la /mnt/nfs_crdb/certs/node.key
-rw-r-----. 1 cockroachdb cockroachdb 1679 Mar 13 17:59 /mnt/nfs_crdb/certs/node.key
$ cockroach node ls --certs-dir=/mnt/nfs_crdb/certs
W210313 18:15:13.875001 1 security/certificate_loader.go:356  error finding key for /mnt/nfs_crdb/certs/client.root.crt: could not read key file /mnt/nfs_crdb/certs/client.root.key: open /mnt/nfs_crdb/certs/client.root.key: permission denied
W210313 18:15:13.875358 1 security/certificate_loader.go:356  error finding key for /mnt/nfs_crdb/certs/node.crt: key file /mnt/nfs_crdb/certs/node.key has permissions -rw-r-----, exceeds -rwx------
ERROR: cannot load certificates.
Check your certificate settings, set --certs-dir, or use --insecure for insecure clusters.

problem using security settings: key file /mnt/nfs_crdb/certs/node.key has permissions -rw-r-----, exceeds -rwx------
Failed running "node ls"

Expected behaviour Allow file permissions of 0400, 0600, 0640, ~0660~ for certificate file.

Environment:

$ cockroach version
Build Tag:        v20.2.5
Build Time:       2021/02/16 12:52:58
Distribution:     CCL
Platform:         linux amd64 (x86_64-unknown-linux-gnu)
Go Version:       go1.13.14
C Compiler:       gcc 6.3.0
Build Commit ID:  162c5ac4968cf31c0ed54cd29aa8aeccd66247bb
Build Type:       release
 - Server OS:  Centos 9

Additional context

Inconvenience, yet another thing to add to a deployment script, need to create a copy of the directory/change ownership. Yes, I can set the environmental var COCKROACH_SKIP_KEY_PERMISSION_CHECK after reading the source code but that feels defensive.

blathers-crl[bot] commented 3 years ago

Hello, I am Blathers. I am here to help you get the issue triaged.

Hoot - a bug! Though bugs are the bane of my existence, rest assured the wretched thing will get the best of care here.

I was unable to automatically find someone to ping.

If we have not gotten back to your issue within a few business days, you can try the following:

:owl: Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

bryanhuntesl commented 3 years ago

Override :

https://github.com/cockroachdb/cockroach/blob/effe2826f6de7715f8f65e00c6e79db762791bee/pkg/security/certificate_loader.go#L36

Permissions stuff ( personally I wouldn't allow 0700 either, it's not supposed to be an executable file)

https://github.com/cockroachdb/cockroach/blob/effe2826f6de7715f8f65e00c6e79db762791bee/pkg/security/certificate_loader.go#L98-L104

rafiss commented 3 years ago

Hi! This isn't my area of expertise, but I'll forward it along to our server/security team.

cc @knz @aaron-crl

knz commented 3 years ago

@aaron-crl what do you think?

bryanhuntesl commented 3 years ago

Can set gid in Kubernetes when mounting volume but not UID. Which, because Posix.

https://serverfault.com/questions/906083/how-to-mount-volume-with-specific-uid-in-kubernetes-pod

Solution, mount files in with ridiculously permissive permissions, copy to /tmp, correct ownership and let cql happily load them because permissions and ownership are “correct”.

I might be missing something but this seems sub-optimal.

knz commented 3 years ago

For Kubernetes, I think the best approach is to use a k8s secret store instead of a volume mount (where the UID can be set to match I believe).

aaron-crl commented 3 years ago

If k8s secret store can handle the permissions appropriately then I'd recommend that.

Postgres does support a group permission option for system where the certificates are managed by the root user. I'd be game for emulating that, namely:

On Unix systems, the permissions on server.key must disallow any access to world or group; achieve this by the command chmod 0600 server.key. Alternatively, the file can be owned by root and have group read access (that is, 0640 permissions). That setup is intended for installations where certificate and key files are managed by the operating system. The user under which the PostgreSQL server runs should then be made a member of the group that has access to those certificate and key files.

https://www.postgresql.org/docs/13/ssl-tcp.html

I'd be disinclined to broadly weaken the permissions to allow for both user and group to read the certificate as that undermines the intent of the check (which is present to ensure the certificate is linked a singular identity).

catj-cockroach commented 3 years ago

I'd agree with @knz that using a Secret would be preferable! Unfortunately there's no way to override the UID for volume mounts (there's an ongoing issue about this). @aaron-crl do you mind if I implement your suggestion?

aaron-crl commented 3 years ago

No objections on my end though I'd defer to @knz who knows the history here better than I do.

catj-cockroach commented 3 years ago

I've opened a PR to enable the use of root owned private keys, provided that the group with read permission on the key matches the group of the user running CockroachDB: https://github.com/cockroachdb/cockroach/pull/68182

I've set the allowed permissions for this case to 0x640, because if root owns the private key, I'd argue it should be the only user modifying it (which would be the case for volumes in kubernetes).

catj-cockroach commented 3 years ago

Hey @bryanhuntesl, we just merged #68182! If (using the latest version on the master branch) you configure your certificate keys to mount with the same group as the user that runs cockroachdb, it'll now be able to read the certificate key if the permissions are set to rw-r-----!

bryanhuntesl commented 3 years ago

Perfect and much appreciated thanks

On Wed, 4 Aug 2021 at 17:59, Cat J @.***> wrote:

Hey @bryanhuntesl https://github.com/bryanhuntesl, we just merged #68182 https://github.com/cockroachdb/cockroach/pull/68182! If you configure your certificate keys to mount with the same group as the user that runs cockroachdb, it'll now be able to read the certificate key if the permissions are set to rw-r-----!

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/cockroachdb/cockroach/issues/61975#issuecomment-892819476, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHUCR5TLWGZEJXUXWLQFWV3T3FWVNANCNFSM4ZECI7MQ .

-- ............................................... (PGP) 0x87E3B94D7B2BEEEF (Keybase) @.*** (Github) bryanhuntesl ...............................................

-- Our upcoming conferences:

RabbitMQ Summit: https://rabbitmqsummit.com/ 13-14 July 2021 ElixirConf EU: https://www2.elixirconf.eu/elixir-conf-2021/es 8-10 September 2021 Code Beam SF: https://www2.codesync.global/code-beam-sf-2021/es 4-5 November 2021

Erlang Solutions cares about your data and privacy; please find all details about the basis for communicating with you and the way we process your data in our Privacy Policy https://www.erlang-solutions.com/privacy-policy.html. You can update your email preferences or opt-out from receiving Marketing emails here https://www2.erlang-solutions.com/email-preference?epc_hash=JtO6C7Q2rJwCdZxBx3Ad8jI2D4TJum7XcUWcgfjZ8YY.