From a99c31c6209080630924b0363c2c93021a2d6cfd Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 30 Oct 2026 10:34:24 -0700 Subject: [PATCH] Read secret files through unbuffered streams ReadFileIntoBuffer and the wolfsshd host-key and /etc/shadow readers turn off stdio buffering right after open, so those secrets are read straight into wolfSSH's own buffers instead of being staged in a libc FILE buffer that fclose frees without wiping. Public-key lists stay buffered. - port.h: add WFSETNOBUF, setvbuf(_IONBF) for stdio ports, a no-op elsewhere - wolfsshd: add wolfSSHD_OpenSecretFile, the unbuffered secure open - test_configuration: the secret open reads no further than asked, the secure open reads ahead Issue: F-14689, F-14690 --- apps/wolfsshd/auth.c | 20 ++++- apps/wolfsshd/auth.h | 4 + apps/wolfsshd/test/test_configuration.c | 99 +++++++++++++++++++++++++ apps/wolfsshd/wolfsshd.c | 18 +++-- src/ssh.c | 3 + wolfssh/port.h | 8 ++ 6 files changed, 145 insertions(+), 7 deletions(-) diff --git a/apps/wolfsshd/auth.c b/apps/wolfsshd/auth.c index 217d0aa46..81bbcac6b 100644 --- a/apps/wolfsshd/auth.c +++ b/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, diff --git a/apps/wolfsshd/auth.h b/apps/wolfsshd/auth.h index 7f6a80401..7a3c69d50 100644 --- a/apps/wolfsshd/auth.h +++ b/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 + diff --git a/apps/wolfsshd/test/test_configuration.c b/apps/wolfsshd/test/test_configuration.c index 827ce2547..4053fa7d9 100644 --- a/apps/wolfsshd/test/test_configuration.c +++ b/apps/wolfsshd/test/test_configuration.c @@ -34,6 +34,7 @@ #ifndef _WIN32 #include + #include #include #include #include @@ -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), diff --git a/apps/wolfsshd/wolfsshd.c b/apps/wolfsshd/wolfsshd.c index 003851260..932e00481 100644 --- a/apps/wolfsshd/wolfsshd.c +++ b/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; } } diff --git a/src/ssh.c b/src/ssh.c index 09bf297fe..9f7cee37f 100644 --- a/src/ssh.c +++ b/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); diff --git a/wolfssh/port.h b/wolfssh/port.h index 4e1464141..9da7e0065 100644 --- a/wolfssh/port.h +++ b/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