commit c1935c6bdb302b719cd82102eb2a2ea498690380
parent a1bd70722b274622328ac8f992807148782694e5
Author: Christian Grothoff <christian@grothoff.org>
Date: Sun, 11 Oct 2026 15:03:52 +0200
refuse to use key files that had too broad permissions and are thus insecure
Diffstat:
5 files changed, 111 insertions(+), 177 deletions(-)
diff --git a/src/util/secmod_common.c b/src/util/secmod_common.c
@@ -227,6 +227,79 @@ TES_normalize_key_directory (const struct GNUNET_CONFIGURATION_Handle *cfg,
}
+int
+TES_open_key_file (const char *filename,
+ struct stat *sbuf)
+{
+ int fd;
+
+ /* O_NOFOLLOW refuses symbolic links without a separate lstat(), and
+ O_NONBLOCK keeps us from hanging on a FIFO, which fstat() refuses. */
+ fd = open (filename,
+ O_RDONLY | O_CLOEXEC | O_NOFOLLOW | O_NONBLOCK);
+ if (-1 == fd)
+ {
+ /* Linux reports a symbolic link with ELOOP, some BSDs with EMLINK */
+ if ( (ELOOP == errno) ||
+ (EMLINK == errno) )
+ GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
+ "File `%s' is a symbolic link, which is not allowed for private keys; refusing to use it\n",
+ filename);
+ else
+ GNUNET_log_strerror_file (GNUNET_ERROR_TYPE_WARNING,
+ "open",
+ filename);
+ return -1;
+ }
+ if (0 != fstat (fd,
+ sbuf))
+ {
+ GNUNET_log_strerror_file (GNUNET_ERROR_TYPE_WARNING,
+ "fstat",
+ filename);
+ GNUNET_break (0 == close (fd));
+ return -1;
+ }
+ if (! S_ISREG (sbuf->st_mode))
+ {
+ GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
+ "File `%s' is not a regular file, which is not allowed for private keys; refusing to use it\n",
+ filename);
+ GNUNET_break (0 == close (fd));
+ return -1;
+ }
+ if (geteuid () != sbuf->st_uid)
+ {
+ GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
+ "Private key file `%s' is owned by uid %u, not by our uid %u; refusing to use it\n",
+ filename,
+ (unsigned int) sbuf->st_uid,
+ (unsigned int) geteuid ());
+ GNUNET_break (0 == close (fd));
+ return -1;
+ }
+ if (1 != sbuf->st_nlink)
+ {
+ GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
+ "Private key file `%s' has %llu hard links instead of one; refusing to use it\n",
+ filename,
+ (unsigned long long) sbuf->st_nlink);
+ GNUNET_break (0 == close (fd));
+ return -1;
+ }
+ if (0 != (sbuf->st_mode & (S_IRWXG | S_IRWXO)))
+ {
+ GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
+ "Private key file `%s' is accessible by group or others (mode %04o); refusing to use it, restrict it to its owner (chmod 400)\n",
+ filename,
+ (unsigned int) (sbuf->st_mode & 07777));
+ GNUNET_break (0 == close (fd));
+ return -1;
+ }
+ return fd;
+}
+
+
/**
* Head of DLL of clients connected to us.
*/
diff --git a/src/util/secmod_common.h b/src/util/secmod_common.h
@@ -57,6 +57,23 @@ TES_normalize_key_directory (const struct GNUNET_CONFIGURATION_Handle *cfg,
/**
+ * Open the private key file @a filename for reading and check that it may
+ * be used: it must be a regular file (not a symbolic link), be owned by our
+ * effective user, have exactly one hard link and grant no permissions to
+ * group or others. All checks are made with fstat() on the opened file
+ * descriptor. A refused file is logged at ERROR level with the reason and
+ * left untouched.
+ *
+ * @param filename name of the key file
+ * @param[out] sbuf set to the status of the opened file
+ * @return file descriptor open for reading, -1 if the file must not be used
+ */
+int
+TES_open_key_file (const char *filename,
+ struct stat *sbuf);
+
+
+/**
* Create the listen socket for a secmod daemon.
*
* This function is not thread-safe, as it changes and
diff --git a/src/util/secmod_cs.c b/src/util/secmod_cs.c
@@ -2030,67 +2030,10 @@ import_key (void *cls,
int fd;
struct stat sbuf;
- {
- struct stat lsbuf;
-
- if (0 != lstat (filename,
- &lsbuf))
- {
- GNUNET_log_strerror_file (GNUNET_ERROR_TYPE_WARNING,
- "lstat",
- filename);
- return GNUNET_OK;
- }
- if (! S_ISREG (lsbuf.st_mode))
- {
- GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
- "File `%s' is not a regular file, which is not allowed for private keys!\n",
- filename);
- return GNUNET_OK;
- }
- }
-
- fd = open (filename,
- O_RDONLY | O_CLOEXEC);
+ fd = TES_open_key_file (filename,
+ &sbuf);
if (-1 == fd)
- {
- GNUNET_log_strerror_file (GNUNET_ERROR_TYPE_WARNING,
- "open",
- filename);
- return GNUNET_OK;
- }
- if (0 != fstat (fd,
- &sbuf))
- {
- GNUNET_log_strerror_file (GNUNET_ERROR_TYPE_WARNING,
- "stat",
- filename);
- GNUNET_break (0 == close (fd));
- return GNUNET_OK;
- }
- if (! S_ISREG (sbuf.st_mode))
- {
- GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
- "File `%s' is not a regular file, which is not allowed for private keys!\n",
- filename);
- GNUNET_break (0 == close (fd));
return GNUNET_OK;
- }
- if (0 != (sbuf.st_mode & (S_IWUSR | S_IRWXG | S_IRWXO)))
- {
- /* permission are NOT tight, try to patch them up! */
- if (0 !=
- fchmod (fd,
- S_IRUSR))
- {
- GNUNET_log_strerror_file (GNUNET_ERROR_TYPE_WARNING,
- "fchmod",
- filename);
- /* refuse to use key if file has wrong permissions */
- GNUNET_break (0 == close (fd));
- return GNUNET_OK;
- }
- }
fh = GNUNET_DISK_get_handle_from_int_fd (fd);
if (NULL == fh)
{
diff --git a/src/util/secmod_eddsa.c b/src/util/secmod_eddsa.c
@@ -881,67 +881,10 @@ import_key (void *cls,
struct stat sbuf;
(void) cls;
- {
- struct stat lsbuf;
-
- if (0 != lstat (filename,
- &lsbuf))
- {
- GNUNET_log_strerror_file (GNUNET_ERROR_TYPE_WARNING,
- "lstat",
- filename);
- return GNUNET_OK;
- }
- if (! S_ISREG (lsbuf.st_mode))
- {
- GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
- "File `%s' is not a regular file, which is not allowed for private keys!\n",
- filename);
- return GNUNET_OK;
- }
- }
-
- fd = open (filename,
- O_RDONLY | O_CLOEXEC);
+ fd = TES_open_key_file (filename,
+ &sbuf);
if (-1 == fd)
- {
- GNUNET_log_strerror_file (GNUNET_ERROR_TYPE_WARNING,
- "open",
- filename);
return GNUNET_OK;
- }
- if (0 != fstat (fd,
- &sbuf))
- {
- GNUNET_log_strerror_file (GNUNET_ERROR_TYPE_WARNING,
- "stat",
- filename);
- GNUNET_break (0 == close (fd));
- return GNUNET_OK;
- }
- if (! S_ISREG (sbuf.st_mode))
- {
- GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
- "File `%s' is not a regular file, which is not allowed for private keys!\n",
- filename);
- GNUNET_break (0 == close (fd));
- return GNUNET_OK;
- }
- if (0 != (sbuf.st_mode & (S_IWUSR | S_IRWXG | S_IRWXO)))
- {
- /* permission are NOT tight, try to patch them up! */
- if (0 !=
- fchmod (fd,
- S_IRUSR))
- {
- GNUNET_log_strerror_file (GNUNET_ERROR_TYPE_WARNING,
- "fchmod",
- filename);
- /* refuse to use key if file has wrong permissions */
- GNUNET_break (0 == close (fd));
- return GNUNET_OK;
- }
- }
fh = GNUNET_DISK_get_handle_from_int_fd (fd);
if (NULL == fh)
{
diff --git a/src/util/secmod_rsa.c b/src/util/secmod_rsa.c
@@ -1711,6 +1711,21 @@ parse_key (struct Denomination *denom,
GNUNET_free (nf);
return;
}
+ /* refuse keys whose modulus does not have the configured size */
+ if (denom->rsa_keysize !=
+ GNUNET_CRYPTO_rsa_public_key_len (pub))
+ {
+ GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
+ "File `%s' holds a %u-bit RSA key, but RSA_KEYSIZE of section `%s' is %u; refusing to use it\n",
+ (NULL == nf) ? filename : nf,
+ GNUNET_CRYPTO_rsa_public_key_len (pub),
+ denom->section,
+ (unsigned int) denom->rsa_keysize);
+ GNUNET_CRYPTO_rsa_public_key_free (pub);
+ GNUNET_CRYPTO_rsa_private_key_free (priv);
+ GNUNET_free (nf);
+ return;
+ }
dk = GNUNET_new (struct DenominationKey);
dk->denom_priv = priv;
dk->denom = denom;
@@ -1780,67 +1795,10 @@ import_key (void *cls,
int fd;
struct stat sbuf;
- {
- struct stat lsbuf;
-
- if (0 != lstat (filename,
- &lsbuf))
- {
- GNUNET_log_strerror_file (GNUNET_ERROR_TYPE_WARNING,
- "lstat",
- filename);
- return GNUNET_OK;
- }
- if (! S_ISREG (lsbuf.st_mode))
- {
- GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
- "File `%s' is not a regular file, which is not allowed for private keys!\n",
- filename);
- return GNUNET_OK;
- }
- }
-
- fd = open (filename,
- O_RDONLY | O_CLOEXEC);
+ fd = TES_open_key_file (filename,
+ &sbuf);
if (-1 == fd)
- {
- GNUNET_log_strerror_file (GNUNET_ERROR_TYPE_WARNING,
- "open",
- filename);
- return GNUNET_OK;
- }
- if (0 != fstat (fd,
- &sbuf))
- {
- GNUNET_log_strerror_file (GNUNET_ERROR_TYPE_WARNING,
- "stat",
- filename);
- GNUNET_break (0 == close (fd));
- return GNUNET_OK;
- }
- if (! S_ISREG (sbuf.st_mode))
- {
- GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
- "File `%s' is not a regular file, which is not allowed for private keys!\n",
- filename);
- GNUNET_break (0 == close (fd));
return GNUNET_OK;
- }
- if (0 != (sbuf.st_mode & (S_IWUSR | S_IRWXG | S_IRWXO)))
- {
- /* permission are NOT tight, try to patch them up! */
- if (0 !=
- fchmod (fd,
- S_IRUSR))
- {
- GNUNET_log_strerror_file (GNUNET_ERROR_TYPE_WARNING,
- "fchmod",
- filename);
- /* refuse to use key if file has wrong permissions */
- GNUNET_break (0 == close (fd));
- return GNUNET_OK;
- }
- }
fh = GNUNET_DISK_get_handle_from_int_fd (fd);
if (NULL == fh)
{
