Merge branch 'fix/audit-creds' into fix/audit-cycle

src/shared/credentials.c

@@ -80,12 +80,16 @@ static bool is_comment_char(char c) {
  * path and then fstat the resulting fd (rather than stat()ing the path first
  * and reopening it), so the permission decision is made on the same inode that
  * is read and cannot be raced by swapping the path between check and open.
- * The path may be a process-substitution pipe (`<(...)` -> /dev/fd/N), so
- * regular files and FIFOs are accepted when the ownership/mode checks pass.
+ * O_NOFOLLOW refuses a symlinked path outright (ELOOP fails closed) instead of
+ * following it before the owner/mode gate can run.  O_NONBLOCK keeps a FIFO
+ * from blocking the open/read forever: an empty or writer-less FIFO yields
+ * EOF/EAGAIN rather than hanging in fgets.  Only regular files and FIFOs pass
+ * the ownership/mode checks; O_NONBLOCK is cleared for regular files, where it
+ * is a no-op anyway, so their stdio read path is byte-for-byte unchanged.
  *
  * Returns a FILE* the caller must fclose, or NULL with `err` filled. */
 static FILE* secret_file_open(const char* path, char* err, size_t err_size) {
-  int fd = open(path, O_RDONLY | O_CLOEXEC);
+  int fd = open(path, O_RDONLY | O_NOFOLLOW | O_NONBLOCK | O_CLOEXEC);
   if (fd < 0) {
     set_error(err, err_size, "cannot open secret file '%s': %s", path, strerror(errno));
     return NULL;
@@ -105,6 +109,15 @@ static FILE* secret_file_open(const char* path, char* err, size_t err_size) {
     close(fd);
     return NULL;
   }
+  /* O_NONBLOCK is only meaningful for the FIFO allowance.  Restore blocking
+   * mode on a regular file so its read path is exactly as before; a no-op on
+   * most systems, but explicit.  Failures here are ignored: O_NONBLOCK on a
+   * regular file does not affect reads either way. */
+  if (S_ISREG(st.st_mode)) {
+    int flags = fcntl(fd, F_GETFL);
+    if (flags >= 0)
+      (void)fcntl(fd, F_SETFL, flags & ~O_NONBLOCK);
+  }
   FILE* fp = fdopen(fd, "r");
   if (!fp) {
     set_error(err, err_size, "cannot read secret file '%s': %s", path, strerror(errno));

src/shared/file.c

@@ -25,8 +25,6 @@
 #include "utils.h"
 #include "protocol.h"
 #include "xattr.h"
-#include <fcntl.h>
-#include <unistd.h>
 
 /* Files larger than this are not loaded whole for transfer (the sender streams
  * them); a whole-file digest is computed from the path instead.  Kept in sync

tests/test_credentials.c

@@ -719,6 +719,50 @@ static void test_credentials_read_secret_file_bad() {
   EXPECT_EQ_INT(credentials_read_secret_file(missing, NULL, NULL, err, sizeof(err)), -1);
 }
 
+/* A symlink planted at a password-file path is refused (O_NOFOLLOW) instead of
+ * being followed before the owner/mode gate, even when it resolves to a valid
+ * owner-only regular file. */
+static void test_credentials_read_secret_file_symlink_rejected() {
+  char err[512];
+  char* target = make_tmp_file("alice:correct horse battery staple\n");
+  EXPECT_NOT_NULL(target);
+
+  char link[256];
+  snprintf(link, sizeof(link), "/tmp/fs_cred_pwlink_%d_%d", (int)getpid(), g_file_counter++);
+  unlink(link);
+  EXPECT_EQ_INT(symlink(target, link), 0);
+
+  char* user = (char*)1;
+  char* password = (char*)1;
+  EXPECT_EQ_INT(credentials_read_secret_file(link, &user, &password, err, sizeof(err)), -1);
+  EXPECT_NULL(user);
+  EXPECT_NULL(password);
+  EXPECT_TRUE(err[0] != '\0');
+
+  unlink(link); /* remove the symlink itself, not its target */
+  rm_temp(target);
+  free(target);
+}
+
+/* A named FIFO with no writer must not hang in fgets (O_NONBLOCK): the read
+ * fails cleanly with "no user:password line" instead of blocking forever. */
+static void test_credentials_read_secret_file_fifo_no_hang() {
+  char err[512];
+  char fifo[256];
+  snprintf(fifo, sizeof(fifo), "/tmp/fs_cred_pwfifo_%d_%d", (int)getpid(), g_file_counter++);
+  unlink(fifo);
+  EXPECT_EQ_INT(mkfifo(fifo, 0600), 0);
+
+  char* user = (char*)1;
+  char* password = (char*)1;
+  EXPECT_EQ_INT(credentials_read_secret_file(fifo, &user, &password, err, sizeof(err)), -1);
+  EXPECT_NULL(user);
+  EXPECT_NULL(password);
+  EXPECT_TRUE(strstr(err, "no 'user:password'") != NULL || err[0] != '\0');
+
+  unlink(fifo);
+}
+
 static void test_credentials_hash_file() {
   char* plaintext = make_tmp_file("# comment\n\n alice :" KAT_PASSWORD "\nbob:bob-s3cret\n");
   EXPECT_NOT_NULL(plaintext);
@@ -1103,6 +1147,8 @@ void test_credentials(void) {
   test_credentials_early_input_merge();
   test_credentials_read_secret_file();
   test_credentials_read_secret_file_bad();
+  test_credentials_read_secret_file_symlink_rejected();
+  test_credentials_read_secret_file_fifo_no_hang();
   test_credentials_hash_file();
   test_credentials_rejects_group_or_other_accessible();
   test_credentials_dummy_key_persisted();