INN commit: branches/2.5/nnrpd (tls.c)

INN Commit rra at isc.org
Sun Jul 17 18:23:31 UTC 2011


    Date: Sunday, July 17, 2011 @ 11:23:31
  Author: iulius
Revision: 9261

better check of the rights on the private key for TLS connections

Use stat() instead of lstat().  The permissions of the symlink are
irrelevant; we only care about the permissions of the underlying file.

Use getegid() instead of getgid().

The news group is fairly trusted already by INN.  Ensure that if the mode
is 440 or 640, the group owner is the news group (to prevent the failure
case of having news:users as the owner and group).

In the error message, a wrong path to the key was put (cert_file instead
of key_file).

Thanks to Florian Schlichting for having reported the issue, and to Russ
Allbery for his help on the fix.

Modified:
  branches/2.5/nnrpd/tls.c

-------+
 tls.c |   18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

Modified: tls.c
===================================================================
--- tls.c	2011-07-17 18:23:10 UTC (rev 9260)
+++ tls.c	2011-07-17 18:23:31 UTC (rev 9261)
@@ -378,16 +378,22 @@
 	if (key_file == NULL)
 	    key_file = cert_file;
 
-	/* Check ownership and permissions of key file. */
-	if (lstat(key_file, &buf) == -1) {
+	/* Check ownership and permissions of key file.
+         * Look at the real file (stat) and not a possible symlink (lstat). */
+	if (stat(key_file, &buf) == -1) {
 	    syslog(L_ERROR, "unable to stat private key '%s'", key_file);
 	    return (0);
 	}
-	if (!S_ISREG(buf.st_mode) || (buf.st_mode & 0077) != 0 ||
-	    buf.st_uid != getuid()) {
+
+        /* 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. */
+	if (!S_ISREG(buf.st_mode) || (buf.st_mode & 0137) != 0
+            || ((buf.st_mode & 0040) != 0 && buf.st_gid != getegid())) {
 	    syslog(L_ERROR, "bad ownership or permissions on private key"
-                   " '%s': private key must be mode 600 and owned by "
-                   "uid %d", cert_file, getuid());
+                   " '%s':  private key must be mode 640 at most, and readable"
+                   " by the news group only", key_file);
 	    return (0);
 	}
 




More information about the inn-committers mailing list