Read secret files through unbuffered streams

apps/wolfsshd/auth.c

@@ -623,7 +623,7 @@ void wolfSSHD_AuthInit(void)
 
 #ifndef WOLFSSHD_UNIT_TEST
     /* /etc/shadow is commonly root:shadow 0640; don't reject group-readable. */
-    if (wolfSSHD_OpenSecureFile(WSSHD_SHADOW_FILE, 0 /* ownerUid: root */,
+    if (wolfSSHD_OpenSecretFile(WSSHD_SHADOW_FILE, 0 /* ownerUid: root */,
             0 /* rejectReadable */, 0 /* relaxPerms */, NULL, &f)
             == WS_SUCCESS && f != NULL) {
         ScanShadowFile(f);
@@ -1387,7 +1387,7 @@ int wolfSSHD_OpenSecureFile(const char* path, WUID_T ownerUid,
     }
 
     /* The target is a regular file, so restore blocking semantics for the
-     * buffered reads the caller will perform. */
+     * reads the caller will perform. */
     if (ret == WS_SUCCESS) {
         flags = fcntl(fd, F_GETFL);
         if (flags != -1) {
@@ -1431,6 +1431,22 @@ int wolfSSHD_OpenSecureFile(const char* path, WUID_T ownerUid,
 #endif
 }
 
+/* wolfSSHD_OpenSecureFile() for a secret: the stream is unbuffered, so the
+ * host key or shadow records are not left in a libc buffer that is freed
+ * unwiped. Public files keep the buffered stream for their line scans. */
+int wolfSSHD_OpenSecretFile(const char* path, WUID_T ownerUid,
+        int rejectReadable, int relaxPerms, void* heap, WFILE** out)
+{
+    int ret;
+
+    ret = wolfSSHD_OpenSecureFile(path, ownerUid, rejectReadable, relaxPerms,
+            heap, out);
+    if (ret == WS_SUCCESS) {
+        WFSETNOBUF(NULL, *out);
+    }
+    return ret;
+}
+
 /* Scan keys file. Fails closed on no match.
  * strictModes requires uid ownership. isCert flags DER vs wire-format. */
 static int SearchKeysFile(const char* keysFilePath, const byte* key,

apps/wolfsshd/auth.h

@@ -111,6 +111,10 @@ int wolfSSHD_GetHomeDirectory(WOLFSSHD_AUTH* auth, WOLFSSH* ssh, WCHAR* out, int
  * See the definition in auth.c for the meaning of each argument. */
 int wolfSSHD_OpenSecureFile(const char* path, WUID_T ownerUid,
     int rejectReadable, int relaxPerms, void* heap, WFILE** out);
+/* As wolfSSHD_OpenSecureFile(), but the stream is unbuffered. For secrets:
+ * the host private key and the shadow file. */
+int wolfSSHD_OpenSecretFile(const char* path, WUID_T ownerUid,
+    int rejectReadable, int relaxPerms, void* heap, WFILE** out);
 
 /* classifies a loaded host private key buffer as OpenSSH or ASN1/DER.
  * *keyDer is a WMALLOC'd (heap, DYNTYPE_SSHD) buffer to WS_FORCEZERO +

apps/wolfsshd/test/test_configuration.c

@@ -34,6 +34,7 @@
 
 #ifndef _WIN32
     #include <unistd.h>
+    #include <fcntl.h>
     #include <sys/stat.h>
     #include <sys/resource.h>
     #include <signal.h>
@@ -6401,6 +6402,103 @@ static int test_OpenSecureFile(void)
     return ret;
 }
 
+/* Read one byte through a secure-open stream and report how far the
+ * descriptor advanced. A buffered stream reads ahead into a libc buffer that
+ * fclose frees unwiped; an unbuffered one reads only the byte asked for. */
+static int smReadAhead(const char* path, WUID_T ownerUid, int rejectReadable,
+        int secret, long* pos)
+{
+    WFILE* f = WBADFILE;
+    int ret;
+
+    *pos = -1;
+    if (secret)
+        ret = wolfSSHD_OpenSecretFile(path, ownerUid, rejectReadable, 0, NULL,
+                &f);
+    else
+        ret = wolfSSHD_OpenSecureFile(path, ownerUid, rejectReadable, 0, NULL,
+                &f);
+    if (ret == WS_SUCCESS) {
+        if (fgetc(f) == EOF) {
+            ret = WS_FATAL_ERROR;
+        }
+        else {
+            *pos = (long)lseek(fileno(f), 0, SEEK_CUR);
+        }
+        (void)WFCLOSE(NULL, f);
+    }
+    return ret;
+}
+
+static int test_OpenSecretFileUnbuffered(void)
+{
+    int ret = WS_SUCCESS;
+    char base[] = "/tmp/wolfsshd_nbXXXXXX";
+    char secret[64] = "";
+    /* written with write(), not stdio, so the setup leaves no copy of its
+     * own in a libc buffer */
+    const char data[] = "root:$6$salt$hash:19000:0:99999:7:::\n"
+                        "user:$6$salt$hash:19000:0:99999:7:::\n";
+    WUID_T uid = getuid();
+    int fd;
+    long pos;
+    int rejectReadable;
+
+    if (mkdtemp(base) == NULL) {
+        Log("    mkdtemp failed.\n");
+        ret = WS_FATAL_ERROR;
+    }
+    if (ret == WS_SUCCESS) {
+        snprintf(secret, sizeof(secret), "%s/secret", base);
+        fd = open(secret, O_WRONLY | O_CREAT | O_EXCL, 0600);
+        if (fd < 0) {
+            ret = WS_FATAL_ERROR;
+        }
+        else {
+            if (write(fd, data, sizeof(data) - 1) != (ssize_t)sizeof(data) - 1)
+                ret = WS_FATAL_ERROR;
+            close(fd);
+        }
+    }
+    if (ret == WS_SUCCESS)
+        ret = smChmod(secret, 0600);
+
+    /* the host key opens with rejectReadable set, /etc/shadow without */
+    for (rejectReadable = 0; ret == WS_SUCCESS && rejectReadable <= 1;
+            rejectReadable++) {
+        ret = smReadAhead(secret, uid, rejectReadable, 1, &pos);
+        Log("    Testing scenario: secret open is unbuffered "
+            "(rejectReadable %d, read %ld for 1).", rejectReadable, pos);
+        if (ret != WS_SUCCESS || pos != 1) {
+            Log(" FAILED.\n");
+            ret = WS_FATAL_ERROR;
+        }
+        else {
+            Log(" PASSED.\n");
+        }
+    }
+
+    /* authorized_keys and the user CA keys are public and scanned a line at
+     * a time, so their stream keeps its read-ahead buffer */
+    if (ret == WS_SUCCESS) {
+        ret = smReadAhead(secret, uid, 0, 0, &pos);
+        Log("    Testing scenario: secure open is buffered "
+            "(read %ld for 1).", pos);
+        if (ret != WS_SUCCESS || pos != (long)sizeof(data) - 1) {
+            Log(" FAILED.\n");
+            ret = WS_FATAL_ERROR;
+        }
+        else {
+            Log(" PASSED.\n");
+        }
+    }
+
+    unlink(secret);
+    rmdir(base);
+
+    return ret;
+}
+
 /* write a config file whose PidFile is 'pidTarget' */
 static int pidConfWrite(const char* confPath, const char* pidTarget)
 {
@@ -7792,6 +7890,7 @@ const TEST_CASE testCases[] = {
     TEST_DECL(test_ConfigFree),
 #ifndef _WIN32
     TEST_DECL(test_OpenSecureFile),
+    TEST_DECL(test_OpenSecretFileUnbuffered),
     TEST_DECL(test_ConfigSavePID),
 #endif
     TEST_DECL(test_DetectPrivKeyFormat),

apps/wolfsshd/wolfsshd.c

@@ -410,14 +410,22 @@ static byte* getBufferFromFile(const char* fileName, word32* bufSz, void* heap,
          * StrictModes. The owner is the daemon's effective user (or root), and
          * the host private key (SECRET) is also refused if group/world
          * readable, unless WOLFSSHD_HOSTKEY_RELAX_PERMS. */
-        if (wolfSSHD_OpenSecureFile(fileName,
+        int ret;
 #ifndef _WIN32
-                geteuid(),
+        WUID_T owner = geteuid();
 #else
-                0,
+        WUID_T owner = 0;
 #endif
-                loadClass == WOLFSSHD_LOAD_SECRET /* rejectReadable */,
-                relaxPerms, heap, &file) != WS_SUCCESS) {
+
+        if (loadClass == WOLFSSHD_LOAD_SECRET) {
+            ret = wolfSSHD_OpenSecretFile(fileName, owner,
+                    1 /* rejectReadable */, relaxPerms, heap, &file);
+        }
+        else {
+            ret = wolfSSHD_OpenSecureFile(fileName, owner,
+                    0 /* rejectReadable */, relaxPerms, heap, &file);
+        }
+        if (ret != WS_SUCCESS) {
             return NULL;
         }
     }

src/ssh.c

@@ -3047,6 +3047,9 @@ static int ReadFileIntoBuffer(const char* name, byte** out, word32* outSz,
     ret = WFOPEN(NULL, &file, name, "rb");
     if (ret != 0 || file == WBADFILE) return WS_BAD_FILE_E;
 #endif
+    /* Read straight into 'in', leaving no copy in a libc buffer that is
+     * freed unwiped. */
+    WFSETNOBUF(NULL, file);
 
     if (!WFSEEK_SUCCESS(WFSEEK(NULL, file, 0, WSEEK_END))) {
         WFCLOSE(NULL, file);

wolfssh/port.h

@@ -463,6 +463,8 @@ extern "C" {
     #define WFSTAT(fs,fd,b)     fstat((fd),(b))
     #define WREWIND(fs,s)       do { fseek((s),0,SEEK_SET); \
                                      clearerr((s)); } while (0)
+    /* Unbuffered, so libc keeps no copy of a secret file's contents. */
+    #define WFSETNOBUF(fs,f)    ((void)setvbuf((f), NULL, _IONBF, 0))
     #define WSEEK_END           SEEK_END
     #define WBADFILE            NULL
     #ifdef WOLFSSL_VXWORKS
@@ -1776,6 +1778,12 @@ extern "C" {
     #define WFSEEK_SUCCESS(r)   ((r) == 0)
 #endif
 
+/* Catch-all for ports with no stdio buffer to turn off. Must be called on a
+ * freshly opened file, before any other operation on it. */
+#ifndef WFSETNOBUF
+    #define WFSETNOBUF(fs,f)    do { } while (0)
+#endif
+
 /* wIsSymlink lives in the always-compiled port.c, but its filesystem
  * dependencies (WSTAT_T/WLSTAT/S_ISLNK on POSIX, WS_GetFileAttributesExA on
  * Windows) and its only callers exist solely in the SFTP and SCP code, so