TLS certificate permission checks

[Julien ÉLIE]

Hi Russ,

> In another group I read, someone was setting up a TLS certificate for 
> use
> with nnrpd using Let's Encrypt, and they ran into a ton of trouble 
> because
> of the very tight permission checks in nnrpd before it's willing to use
> the certificate.  (The root problem was that the key was rejected 
> because
> it was owned by a different group than news, even though it otherwise 
> had
> the correct permissions.)
> 
> I think we may be a bit too aggressive about this.  We're trying to
> protect people against mistakes that could leak the key to other users 
> on
> the same host, but it's increasingly uncommon for a news server to run 
> on
> the same box as untrusted people, so I'm not sure how much this 
> matters.
> And it causes some friction when people are setting up automatic
> certificate renewal.
> 
> What would folks think about replacing the current checks in 
> nnrpd/tls.c
> with just:
> 
>     !S_ISREG(buf.st_mode) || (buf.st_mode & 0007) != 0
> 
> which just makes sure that it's a regular file and not world-readable?

Shouldn't we also check that the key is readable?

I think it was the point of the initial commit in 2002:
   https://inn.eyrie.org/trac/changeset/6037/trunk/nnrpd/tls.c
where the check was:

    !S_ISREG(buf.st_mode) || (buf.st_mode & 0077) != 0 || buf.st_uid != 
getuid()


Otherwise, maybe the error appearing in the logs is not clear enough,
if it does not say that there is a read access issue.



I agree that the new checks in 2011 were probably too restrictive
for the use case you mention in your mail:
   https://inn.eyrie.org/trac/changeset/9219/trunk/nnrpd/tls.c

    /* Check that the key file is a real file, not readable by
     * everyone.  If the mode is 440 or 640, make sure the group owner
     * is the news group (to prevent the failure case of having 
news:users
     * as the owner and group. */

      !S_ISREG(buf.st_mode) || (buf.st_mode & 0137) != 0
        || ((buf.st_mode & 0040) != 0 && buf.st_gid != getegid())

-- 
Julien