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

archive/domain/privkeyN.pem is set to 0644 instead of 0600 (or 0440) #1473

Closed
ZaiLynch opened this Issue Nov 12, 2015 · 51 comments

Comments

Projects
None yet
@ZaiLynch
Copy link

ZaiLynch commented Nov 12, 2015

The private keys in /etc/letsencrypt/archive/domain/privkey.pem are currently set to 0644.
I guess it's better to have only root to be able to read the private key, hence set them to 0600.

@pde

This comment has been minimized.

Copy link
Member

pde commented Nov 12, 2015

Hrm. What permissions do you see on the directory /etc/letsencrypt/archive? Can you actually read a privkey as an unprivileged user?

@pde

This comment has been minimized.

Copy link
Member

pde commented Nov 12, 2015

Probably the keys should be 0640 and set the group that runs the webserver, if it isn't running as root? And the directory should be 0750, assuming such a group exists?

@pde

This comment has been minimized.

Copy link
Member

pde commented Nov 12, 2015

This is going to be hard to get right from letsencrypt-auto. But we should have control of these variables concisely exported for OS packagers. @hlieberman @fmarier.

@pde

This comment has been minimized.

Copy link
Member

pde commented Nov 12, 2015

This is part of #420.

@ZaiLynch

This comment has been minimized.

Copy link

ZaiLynch commented Nov 13, 2015

Hm, you're right.
/etc/letsencrypt/archive is set to 700, so a regular user shouldn't be able to reach the keys...

@pde pde changed the title archive/domain/privkeyN.pem is set to 0644 instead of 0600 archive/domain/privkeyN.pem is set to 0644 instead of 0600 (or 0640) Nov 18, 2015

@pde

This comment has been minimized.

Copy link
Member

pde commented Nov 18, 2015

One point made on community.letsencrypt.org is that users may move the certs or keys out of /etc/letsencrypt. Of course they should be symlinking in rather than moving / copying keys out, but it will happen. So bumping this ticket up to nice for 1.0.

@pde pde added the security-low label Nov 18, 2015

@pde pde added this to the Nice for 1.0 milestone Nov 18, 2015

@pde

This comment has been minimized.

Copy link
Member

pde commented Nov 18, 2015

Actually, the point about not moving keys makes me think that in fact the permissions on everything in the directory should be 0440, not 0640.

@pde pde changed the title archive/domain/privkeyN.pem is set to 0644 instead of 0600 (or 0640) archive/domain/privkeyN.pem is set to 0644 instead of 0600 (or 0440) Nov 18, 2015

@pde pde added the good first issue label Dec 3, 2015

@lbeltrame

This comment has been minimized.

Copy link
Contributor

lbeltrame commented Dec 3, 2015

A related matter would be to honor the group permissions set by the directory the certs reside in. I use a "ssl" group that's the only one allowed to touch the certs aside root, for example.

colincross added a commit to colincross/letsencrypt that referenced this issue Dec 4, 2015

Set permissions of all four key files to 0400
Set the permissions of cert.pem, privkey.pem, chain.pem and
fullchain.pem to the recommended 0400, readable only by root.

Fixes certbot#1473

colincross added a commit to colincross/letsencrypt that referenced this issue Dec 4, 2015

Set permissions of all four key files to 0400
Set the permissions of cert.pem, privkey.pem, chain.pem and
fullchain.pem to the recommended 0400, readable only by root.

Fixes certbot#1473

colincross added a commit to colincross/letsencrypt that referenced this issue Dec 4, 2015

Set permissions of all four key files to 0600
Set the permissions of cert.pem, privkey.pem, chain.pem and
fullchain.pem to 0600, read-write only by root.

Fixes certbot#1473

colincross pushed a commit to colincross/letsencrypt that referenced this issue Dec 4, 2015

Colin Cross
Set permissions of all four key files to 0600
Set the permissions of cert.pem, privkey.pem, chain.pem and
fullchain.pem to 0600, read-write only by root.

Fixes certbot#1473

colincross pushed a commit to colincross/letsencrypt that referenced this issue Dec 4, 2015

Colin Cross
Set permissions of all four key files to 0400
Set the permissions of cert.pem, privkey.pem, chain.pem and
fullchain.pem to 0400, readable only by root.

Fixes certbot#1473

colincross pushed a commit to colincross/letsencrypt that referenced this issue Dec 4, 2015

Colin Cross
Set permissions of all four key files to 0400
Set the permissions of cert.pem, privkey.pem, chain.pem and
fullchain.pem to 0400, readable only by root. Also create the
live and archive directories with 0700 permissions.

Fixes certbot#1473

colincross pushed a commit to colincross/letsencrypt that referenced this issue Dec 4, 2015

Colin Cross
Set permissions of all four key files to 0400
Set the permissions of cert.pem, privkey.pem, chain.pem and
fullchain.pem to 0400, readable only by root. Also create the
live and archive directories with 0700 permissions.

Fixes certbot#1473
@thomaszbz

This comment has been minimized.

Copy link
Contributor

thomaszbz commented Jan 18, 2016

Since ~2006, Debian-based distros come along with a system group ssl-cert which is designed for use with private key material (according to changelog).

For symlinks from /etc/ssl/..., something like this would be nice:

-rw-r--r-- 1 root root     2.1K Jan 17 17:54 cert1.pem
-rw-r--r-- 1 root root     1.1K Jan 17 17:54 chain1.pem
-rw-r--r-- 1 root root     3.2K Jan 17 17:54 fullchain1.pem
-rw-r----- 1 root ssl-cert 3.2K Jan 17 17:54 privkey1.pem

These settings should be fine for e.g. Debian 8 / Apache and many other applications.

I guess that users have different requirements (e.g. according to distro), so I think the permissions should be configurable via cli.ini and maybe via parameters (overwriting cli.ini settings).

I also want to note that wrong permissions (in the sense of user's requirements) cannot necessarily be healed by a later user interaction: Once the private key is readable after being written to disk with weak permissions, it could theoretically be stolen if the admin user changes permissions too late. Therefore, an emtpy file with correct file permissions must be written to disk before key material is written into the file.

I suggest to introduce three config values for cli.ini (including example settings):

private-key-owner = root
private-key-group = ssl-cert
private-key-permisson = 640

Optionally, this could also be done for public key material (just for completeness).

I'm not sure for other distros than Debian, if there is a ssl-cert group (or equivalent). But assumed, such a group is widely used, this could be a nice default for LE (when cli.ini is unmodified or unused).

I guess the Debian package maintainers might want to go for ssl-cert as a default anyways.

References:

@thomaszbz

This comment has been minimized.

Copy link
Contributor

thomaszbz commented Jan 27, 2016

Courier Mailserver is complaining when LE's public certificates are symlinked from /etc/ssl/certs:

courieresmtpd: STARTTLS failed: couriertls: /etc/ssl/certs/xxxxxxxx.0: Permission denied

Result: Courier only receives mail from some mailservers (I assume servers not supporting encryption). This even happens while I don't use LE certs for my mail server host name (yet). Courier has its own certificates stored in /etc/courier, but it uses public certificates stored in /etc/ssl/certs.

The reason for that is that /etc/letsencrypt/live and /etc/letsencrypt/archive are only readable to the root user, which is a good idea as long as private key material is not protected by itself.

Current state:

drwx------ 7 root root 4.0K Jan 19 17:40 live
drwxr-xr-x 2 root root 4.0K Jan 19 14:30 live/www.example.com
lrwxrwxrwx 2 root root 4.0K Jan 19 14:30 live/www.example.com/privkey.pem (symlink)
lrwxrwxrwx 2 root root 4.0K Jan 19 14:30 live/www.example.com/fullchain.pem (symlink)
drwx------ 7 root root 4.0K Jan 19 17:40 archive (currently protects private certificates)
drwxr-xr-x 2 root root 4.0K Jan 19 14:30 archive/www.example.com
-rw-r--r-- 2 root root 4.0K Jan 19 14:30 archive/www.example.com/privkey3.pem
-rw-r--r-- 2 root root 4.0K Jan 19 14:30 archive/www.example.com/fullchain3.pem

To mitigate such issues and still be able to symlink the latest public certificate, the directories /etc/letsencrypt/live and /etc/letsencrypt/live should be world readable AND private certificates should be protected separately (in a configurable manner).

Should be:

drwxr-xr-x 7 root root 4.0K Jan 19 17:40 live
drwxr-xr-x 2 root root 4.0K Jan 19 14:30 live/www.example.com
lrwxrwxrwx 2 root root 4.0K Jan 19 14:30 live/www.example.com/privkey.pem (symlink)
lrwxrwxrwx 2 root root 4.0K Jan 19 14:30 live/www.example.com/fullchain.pem (symlink)
drwxr-xr-x 7 root root 4.0K Jan 19 17:40 archive
drwxr-xr-x 2 root root 4.0K Jan 19 14:30 archive/www.example.com
-rw-r----- 2 root root 4.0K Jan 19 14:30 archive/www.example.com/privkey3.pem (group configurable per certificate directory)
-rw-r--r-- 2 root root 4.0K Jan 19 14:30 archive/www.example.com/fullchain3.pem

With these permissions, symlinks to LE's public certificates would be world readable (which is fine for public certificates) while private certificates would be readable to only root and a configurable group, e.g.

-rw-r----- 2 root root 4.0K Jan 19 14:30 archive/www.example1.com/privkey3.pem
-rw-r----- 2 root aabb 4.0K Jan 19 14:30 archive/www.example2.com/privkey3.pem
-rw-r----- 2 root ssl-cert 4.0K Jan 19 14:30 archive/www.example3.com/privkey3.pem

@pde pde modified the milestones: 0.7.0, 1.0.0 Mar 17, 2016

@tovine

This comment has been minimized.

Copy link

tovine commented Jul 21, 2018

This issue also makes trouble for samba - our domain controller uses certbot for certificates, and the 644 permissions cause samba to refuse starting as the privkey file is too visible. Found the problem and managed to fix it by correcting the permissions, but that debugging should really not have been necessary...

I really don't see why the permissions aren't just set properly in the first place (even though the directory is "safe" with 700)?

@DimitryAndric

This comment has been minimized.

Copy link

DimitryAndric commented Aug 18, 2018

It's a bit strange that the code which originally generates the key, in https://github.com/certbot/certbot/blob/master/certbot/crypto_util.py#L65, already does the right thing (it uses open mode 0600, which is perfectly appropriate for private keys), and there is even a safe_open function which supports such modes, but it is not used when the keys are actually saved to the archive directory...

In my opinion, 0600 is the right default mode for any private key material, and you should really use that. If e.g. Debian or another distro wants to introduce separate 'ssl-cert' groups with additional permissions, then they can patch their copy of certbot while packaging. I don't think it is up to upstream to cope with all the distro-specific variations.

The following diff should make >95% of people happy (and I'm applying it locally anyway):

diff --git a/certbot/storage.py b/certbot/storage.py
index 32d6771c..88b804f4 100644
--- a/certbot/storage.py
+++ b/certbot/storage.py
@@ -1123,7 +1123,7 @@ class RenewableCert(object):
             logger.debug("Writing symlink to old private key, %s.", old_privkey)
             os.symlink(old_privkey, target["privkey"])
         else:
-            with open(target["privkey"], "wb") as f:
+            with util.safe_open(target["privkey"], "wb", chmod=0o400) as f:
                 logger.debug("Writing new private key to %s.", target["privkey"])
                 f.write(new_privkey)
@sydneyli

This comment has been minimized.

Copy link
Member

sydneyli commented Oct 30, 2018

Did a review of all of the concerns and proposed solutions in this thread. Here's a summary, interspersed with some concerns and some steps forward for the short-term.

There's a couple different things folks on this thread are concerned with:

  1. Private key permissions are too lax.
  2. Certificate/public key permissions are too restrictive.
  3. Group permissions for private key material.

...and also other things, but these are the three main ones.

1 & 2: private & public key permissions

There are a couple of complete solutions that would be nice to knock out all of these at once, like this directory permissioning recommendation from @thomaszbz. However, if we do this, or any similar proposal, we run the risk of exposing private key material if Certbot downgrades.

Currently, it's a fairly common workflow to downgrade versions of Certbot (e.g. use certbot-auto, but then switch to OS packages). Unfortunately, Certbot is careful to write privkeys as 644, BUT never ensures the live/ or archive/ directory remains 700. So, in the short-term, we shouldn't make live/ or archive/ more permissive without a more complex and well-thought-out solution, which might involve migrating the keys to a new directory that is properly permissioned. This would be good to talk about for 1.0!

For now, there's no reason we can't still knock out problem 1 in a simple and entirely safe way by dropping key permissions to 600 (basically @DimitryAndric's proposal), and punt this larger discussion to later.

3: Group permission configurability for private keys

I'd prefer some limited functionality like --key-group <gid> in the short-term, especially since the primary use case is for a private key access group. This would make it clear that such a group should be limited (good for private key hygiene, and also good for the downgrade case), unlike a --gid flag.

We're also talking about this more this week, so this post/thread might be updated with more developments in the very near future.

@Ji-eF

This comment has been minimized.

Copy link

Ji-eF commented Nov 9, 2018

radicale needs +x to 'read' certificates (?); if certs are just +r, radicale.log yelds

WARNING: Error while reading SSL certificate '/etc/letsencrypt/live/******.me/cert.pem': [Errno 13] Permission denied: '/etc/letsencrypt/live/******.me/cert.pem'
WARNING: Error while reading SSL key '/etc/letsencrypt/live/******.me/privkey.pem': [Errno 13] Permission denied: '/etc/letsencrypt/live/******.me/privkey.pem'

g+rx on pem files fixes this error

@bmw bmw added the has pr label Nov 28, 2018

@bmw bmw closed this in #6480 Nov 29, 2018

bmw added a commit that referenced this issue Nov 29, 2018

Change default privkey permissions while preserving group permissions (
…#6480)

Fixes #1473.

writes privkey.pem to 0600 by default for new lineages
on renewals where a new privkey is generated, preserves group mode and gid
Things this PR does not do:

we talked about forcing 0600 on privkeys when a Certbot upgrade is detected. Instead, this PR only creates new lineages with the more restrictive permission to prevent renewal breakages.
this doesn't solve many of the problems mentioned in #1473 that are not directly related to the title issue!

* safe_open on archive keyfiles

* keep group from current lineage

* clean up integration test

* safe_open can follow symlinks

* fix tests on windows, maybe

* Address Brad's comments

* Revert changes to safe_open
* Test chown is called when saving new key
* Reorder chown operation

* Changelog and documentation

* Fix documentation style
@sydneyli

This comment has been minimized.

Copy link
Member

sydneyli commented Nov 30, 2018

If you need a custom permission set on your private key: Starting with the next release, if you alter the GID or group mode of /etc/letsencrypt/live/<domain>/privkey.pem, Certbot will preserve that information in future renewals.

When you create a new certificate, the permission is set by default to 0600.

The primary issue (as indicated in the title) is fixed. Further discussion setting group permissions for private key material as a command-line option, an adjacent issue, should continue in this issue #2964.

@FelixSchwarz

This comment has been minimized.

Copy link
Contributor

FelixSchwarz commented Nov 30, 2018

If you need a custom permission set on your private key: Starting with the next release, if you alter the GID or group mode of /etc/letsencrypt/live/<domain>/privkey.pem, Certbot will preserve that information in future renewals.

That seems like a good solution for everyone. Just two questions:

  1. /etc/letsencrypt/live/<domain>/privkey.pem is a symlink on my systems so I can't set GID/permissions on that exact file. I assume certbot retains GID/group permission of the file referenced from the "live" directory?
  2. Is the mechanism also implemented for "other"/"world" or is this group-specific?

Edit: If I understand the commit correctly the change is behavior group-specific so permissions for "other" will NOT be retained.

It seems like the new version will always set the "world" permission bits to 0 (no permission). This might be a breaking change for some. Was this a deliberate decision?

Edit 2: Maybe my last question would be better placed in the pull request's comments (PR #6480)?

@FelixSchwarz

This comment has been minimized.

Copy link
Contributor

FelixSchwarz commented Nov 30, 2018

Thinking a bit more about this I came to the conclusion that this will be a breaking change for many users relying on the old behavior: Previously there was no way to retain file GID/permission on renewals.
So I think what many did was to change the GID of "archive"/"live" (+ subdirectories) so a non-root group could access the keys. The key itself was world-readable so no problem in that case.

Now if I understand the change correctly (and that might be a big IF) certbot will set the "world" permissions to 0. In that case the key is not accessible anymore in the scenario above.

To avoid breaking any setups I think certbot must also keep the "world" permissions on renewal.

@adferrand

This comment has been minimized.

Copy link
Collaborator

adferrand commented Nov 30, 2018

Yes I confirm, private key world permissions will be set to 0, I have migrated the integration test and the assertion about one hour ago ^^

And the group permission also, by default, on new certificates. However, if afterward the group permission or the group owner is modified, it will be retained for any private key generated during the renewal.

So there will be a problem only for non-root processes outside of this group that relies on world readable permissions on private key to read it. Is it really a case? For what I know, most daemon processes are running as root, or fork themselves to an unprivileged user after setup as root.

And after, even if it is breaking change, it is, for that matter security, a sufficient reason to take the risk and remove the flaw in my opinion.

@FelixSchwarz

This comment has been minimized.

Copy link
Contributor

FelixSchwarz commented Nov 30, 2018

So there will be a problem only for non-root processes outside of this group that relies on world readable permissions on private key to read it. Is it really a case?

One notable example which comes to mind is Exim which reads the private key at runtime (under the "exim" user).

And after, even if it is breaking change, it is, for that matter security, a sufficient reason to take the risk and remove the flaw in my opinion.

I agree that the change in itself is good but the previous setup was not insecure by itself so I don't like breaking existing setups - especially because most users will have automated renewals so something will go wrong in the middle of the night and monitoring will wake up some some poor sysadmin who needs to fix this in a frenzy.

@sydneyli

This comment has been minimized.

Copy link
Member

sydneyli commented Nov 30, 2018

@FelixSchwarz You're right, that setup would break on renewal. I'll work on something to remedy this (the simple solution being to preserve the other-read bit, or maybe a more complicated solution) today.
EDIT: By "more complicated": maybe we can limit the scope of the change to people with group-readable live/archives. The idea here being to discourage people from relying on the world-readable bit in the future, while still making sure people's current setups don't break.

@adferrand

This comment has been minimized.

Copy link
Collaborator

adferrand commented Nov 30, 2018

@sydneyli I like your "more complicated" solution, I am very uncomfortable with private key that are world readable ^^

But for the long term, we should quickly go to the separated directory for private keys approach, and fix definitively the issue.

@bmw

This comment has been minimized.

Copy link
Contributor

bmw commented Nov 30, 2018

Sydney and I talked about this a bit out of band. The reason why at least I am not too worried about copying over the other read permission is that the key permissions are irrelevant from a security perspective as long as the permissions on archive and live are unmodified.

I also think this is a clear improvement to the state of things. Currently, private keys are always made world readable. With the change Sydney is proposing, the default will be that new lineages are created with keys with 600 permissions and that will be preserved between renewals unless the user makes changes to the permissions on the keys. In that case, we'll preserve the group, group permissions, and other read bit.

I think we agree that correct long term solution requires changes to the directory structure of /etc/letsencrypt though.

@adferrand

This comment has been minimized.

Copy link
Collaborator

adferrand commented Nov 30, 2018

Indeed, I was thinking of this as a second wall in case something unexpected happens on the directory permissions level, but anyway, any root level process could just mess up all the security for a lot of reasons. So I probably overthinking ^^

@bmw

This comment has been minimized.

Copy link
Contributor

bmw commented Nov 30, 2018

That's true. Preserving the other permission removes a second layer of protection that could help users from shooting themselves in the foot. That layer doesn't exist in any released version of Certbot so there's no harm there, however, it existed in the initial proposal for the change here and now we're removing it.

I appreciate you thinking about the problem though! I think it'll be a bit tricky, but I'd be happy to chat or hear a proposal for how to really solve the permission problems with certs and keys in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment