diff --git a/apps/wolfsshd/configuration.c b/apps/wolfsshd/configuration.c index 771bd1a28..4e0cf197b 100644 --- a/apps/wolfsshd/configuration.c +++ b/apps/wolfsshd/configuration.c @@ -1375,6 +1375,21 @@ static int CheckNotInMatch(const WOLFSSHD_CONFIG* conf, const char* option) return ret; } +/* Returns the keyword a config option tag came from, for log messages. */ +static const char* OptionName(int opt) +{ + int idx; + + for (idx = 0; idx < NUM_OPTIONS; ++idx) { + if (options[idx].tag == opt) { + return options[idx].name; + } + } + + return ""; +} + + /* returns WS_SUCCESS on success */ /* NOLINTNEXTLINE(misc-no-recursion): bounded by WOLFSSHD_MAX_INCLUDE_DEPTH */ static int HandleConfigOption(WOLFSSHD_CONFIG** conf, int opt, @@ -1404,28 +1419,21 @@ static int HandleConfigOption(WOLFSSHD_CONFIG** conf, int opt, case OPT_PERMIT_EMPTY_PW: ret = HandlePermitEmptyPw(*conf, value); break; + /* @TODO Recognized for sshd_config compatibility, but nothing reads + * them. An unknown keyword is fatal, so accepting these in silence + * reads as support for a setting that is not enforced. Warn and carry + * on: rejecting them would turn every config copied from OpenSSH into + * a startup failure. */ case OPT_SUBSYSTEM: - /* TODO */ - ret = WS_SUCCESS; - break; case OPT_CHALLENGE_RESPONSE_AUTH: - /* TODO */ - ret = WS_SUCCESS; - break; case OPT_USE_PAM: - /* TODO */ - ret = WS_SUCCESS; - break; case OPT_X11_FORWARDING: - /* TODO */ - ret = WS_SUCCESS; - break; case OPT_PRINT_MOTD: - /* TODO */ - ret = WS_SUCCESS; - break; case OPT_ACCEPT_ENV: - /* TODO */ + case OPT_USE_DNS: + wolfSSH_Log(WS_LOG_WARN, + "[SSHD] %s is recognized but not implemented, it has no " + "effect", OptionName(opt)); ret = WS_SUCCESS; break; case OPT_PROTOCOL: @@ -1479,10 +1487,6 @@ static int HandleConfigOption(WOLFSSHD_CONFIG** conf, int opt, case OPT_PERMIT_ROOT: ret = HandlePermitRoot(*conf, value); break; - case OPT_USE_DNS: - /* TODO */ - ret = WS_SUCCESS; - break; case OPT_INCLUDE: ret = HandleInclude(*conf, value, depth); break; diff --git a/apps/wolfsshd/wolfsshd.c b/apps/wolfsshd/wolfsshd.c index 6c3ba5623..73ecd7aff 100644 --- a/apps/wolfsshd/wolfsshd.c +++ b/apps/wolfsshd/wolfsshd.c @@ -109,6 +109,11 @@ #define WOLFSSHD_TIMEOUT 1 #endif +/* The umask the daemon holds, and so the one every session inherits. */ +#ifndef WOLFSSHD_DEFAULT_UMASK + #define WOLFSSHD_DEFAULT_UMASK 022 +#endif + #ifdef EXAMPLE_BUFFER_SZ #warning use WOLFSSHD_SHELL_BUFFER_SZ instead of EXAMPLE_BUFFER_SZ #define WOLFSSHD_SHELL_BUFFER_SZ EXAMPLE_BUFFER_SZ @@ -4556,7 +4561,11 @@ static int StartSSHD(int argc, char** argv) exit(EXIT_SUCCESS); } - umask(0); + /* Not umask(0): every per-connection child inherits this, and + * nothing sets one later, so a cleared mask reaches the user's + * shell and the SCP receive path. Files created there came out + * 0666 and directories 0777. */ + umask(WOLFSSHD_DEFAULT_UMASK); if (chdir("/") < 0) { ret = WS_FATAL_ERROR; } diff --git a/src/wolfsftp.c b/src/wolfsftp.c index e7cca5487..699d1e886 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -1142,9 +1142,9 @@ static int SFTP_AttributesSz(WOLFSSH* ssh, WS_SFTP_FILEATRB* atr) /* check if extended attributes are present */ if (atr->flags & WOLFSSH_FILEATRB_EXT) { + /* @TODO handle extended attributes. Only the count is sized, and + * SFTP_SetAttributes writes it as zero to match. */ sz += UINT32_SZ; - - /* @TODO handle extended attributes */ } return sz; @@ -1191,14 +1191,43 @@ static int SFTP_SetAttributes(WOLFSSH* ssh, byte* buf, word32 bufSz, /* check if extended attributes are present */ if (atr->flags & WOLFSSH_FILEATRB_EXT) { - /* @TODO handle attribute extensions */ - c32toa(atr->extCount, buf + idx); + /* @TODO handle attribute extensions. Until they are written, the + * count goes out as zero: atr->extCount would promise records that + * follow, and the peer's decoder reads past the end looking for + * them. */ + c32toa(0, buf + idx); idx += UINT32_SZ; } return WS_SUCCESS; } +#ifdef WOLFSSH_TEST_INTERNAL +/* Encode atr with the real encoder for unit testing. Returns the number of + * bytes written, or a negative error. */ +int wolfSSH_TestSftpSetAttributes(byte* buf, word32 bufSz, + WS_SFTP_FILEATRB* atr) +{ + int sz; + + if (buf == NULL || atr == NULL) { + return WS_BAD_ARGUMENT; + } + + sz = SFTP_AttributesSz(NULL, atr); + if (sz < 0 || (word32)sz > bufSz) { + return WS_BUFFER_E; + } + + if (SFTP_SetAttributes(NULL, buf, bufSz, atr) != WS_SUCCESS) { + return WS_FATAL_ERROR; + } + + return sz; +} +#endif + + static INLINE int SFTP_GetSz(byte* buf, word32* sz, word32 lowerBound, word32 upperBound) { @@ -7094,6 +7123,20 @@ int SFTP_ParseAttributes_buffer(WOLFSSH* ssh, WS_SFTP_FILEATRB* atr, byte* buf, } +#ifdef WOLFSSH_TEST_INTERNAL +/* Decode attributes with the real parser for unit testing. */ +int wolfSSH_TestSftpParseAttributes(byte* buf, word32 bufSz, + WS_SFTP_FILEATRB* atr, word32* idx) +{ + if (buf == NULL || atr == NULL || idx == NULL) { + return WS_BAD_ARGUMENT; + } + + return SFTP_ParseAttributes_buffer(NULL, atr, buf, idx, bufSz); +} +#endif + + #if 0 /* parse out file attributes from I/O stream * diff --git a/tests/api.c b/tests/api.c index 51e291d08..916180037 100644 --- a/tests/api.c +++ b/tests/api.c @@ -7578,6 +7578,120 @@ static void test_wolfSSH_SetAlgoList(void) } +/* Is name an exact entry in a comma separated algorithm list? Substring + * matching would confuse hmac-sha1 with hmac-sha1-96. */ +static int AlgoListHas(const char* list, const char* name) +{ + const char* p = list; + word32 nameSz = (word32)WSTRLEN(name); + + if (list == NULL) { + return 0; + } + + while (*p != '\0') { + const char* end = WSTRCHR(p, ','); + word32 sz = (end != NULL) ? (word32)(end - p) : (word32)WSTRLEN(p); + + if (sz == nameSz && WSTRNCMP(p, name, nameSz) == 0) { + return 1; + } + if (end == NULL) { + break; + } + p = end + 1; + } + + return 0; +} + + +/* fenrir 13961: the canned default lists leave SHA-1 and AES-CBC out unless + * the build opts back in. Nothing asserted on their contents, so an inverted + * guard or a stray list edit could put a weak algorithm back in the default + * proposal without failing a test. Check what the defaults actually hold. */ +static void test_wolfSSH_DefaultAlgoListsExcludeWeak(void) +{ + WOLFSSH_CTX* ctx; + const char* kex; + const char* key; + const char* cipher; + const char* mac; + + /* A client context: the server derives its key lists from the host keys + * it has loaded, so they are still null on a fresh one. */ + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_CLIENT, NULL); + AssertNotNull(ctx); + + kex = wolfSSH_CTX_GetAlgoListKex(ctx); + key = wolfSSH_CTX_GetAlgoListKey(ctx); + cipher = wolfSSH_CTX_GetAlgoListCipher(ctx); + mac = wolfSSH_CTX_GetAlgoListMac(ctx); + AssertNotNull(kex); + AssertNotNull(key); + AssertNotNull(cipher); + AssertNotNull(mac); + + /* The lists are not empty: a guard that removed everything would other- + * wise pass every absence check below. */ +#ifndef WOLFSSH_NO_HMAC_SHA2_256 + AssertIntEQ(AlgoListHas(mac, "hmac-sha2-256"), 1); +#endif +#ifndef WOLFSSH_NO_AES_CTR + AssertIntEQ(AlgoListHas(cipher, "aes256-ctr"), 1); +#endif +#ifndef WOLFSSH_NO_AES_GCM + AssertIntEQ(AlgoListHas(cipher, "aes256-gcm@openssh.com"), 1); +#endif +#ifndef WOLFSSH_NO_DH_GROUP14_SHA256 + AssertIntEQ(AlgoListHas(kex, "diffie-hellman-group14-sha256"), 1); +#endif + + /* SHA-1 KEX, host key and MAC: in only under the opt-in macro. */ +#ifdef WOLFSSH_NO_SHA1_SOFT_DISABLE + #ifndef WOLFSSH_NO_DH_GROUP14_SHA1 + AssertIntEQ(AlgoListHas(kex, "diffie-hellman-group14-sha1"), 1); + #endif + #ifndef WOLFSSH_NO_SSH_RSA_SHA1 + AssertIntEQ(AlgoListHas(key, "ssh-rsa"), 1); + #endif + #ifndef WOLFSSH_NO_HMAC_SHA1 + AssertIntEQ(AlgoListHas(mac, "hmac-sha1"), 1); + #endif +#else + AssertIntEQ(AlgoListHas(kex, "diffie-hellman-group14-sha1"), 0); + AssertIntEQ(AlgoListHas(kex, "diffie-hellman-group1-sha1"), 0); + AssertIntEQ(AlgoListHas(key, "ssh-rsa"), 0); + AssertIntEQ(AlgoListHas(key, "x509v3-ssh-rsa"), 0); + AssertIntEQ(AlgoListHas(mac, "hmac-sha1"), 0); + AssertIntEQ(AlgoListHas(mac, "hmac-sha1-96"), 0); +#endif + + /* AES-CBC: the same, under its own macro. */ +#if defined(WOLFSSH_NO_AES_CBC_SOFT_DISABLE) && !defined(WOLFSSH_NO_AES_CBC) + AssertIntEQ(AlgoListHas(cipher, "aes256-cbc"), 1); +#else + AssertIntEQ(AlgoListHas(cipher, "aes256-cbc"), 0); + AssertIntEQ(AlgoListHas(cipher, "aes192-cbc"), 0); + AssertIntEQ(AlgoListHas(cipher, "aes128-cbc"), 0); +#endif + + /* A fresh session inherits the context's policy, so the same holds. */ + { + WOLFSSH* ssh = wolfSSH_new(ctx); + + AssertNotNull(ssh); + AssertPtrEq(wolfSSH_GetAlgoListKex(ssh), kex); + AssertPtrEq(wolfSSH_GetAlgoListKey(ssh), key); + AssertPtrEq(wolfSSH_GetAlgoListCipher(ssh), cipher); + AssertPtrEq(wolfSSH_GetAlgoListMac(ssh), mac); + wolfSSH_free(ssh); + } + + wolfSSH_CTX_free(ctx); +} + + /* Exercise CheckAlgoList()'s rejection paths through the public setters. */ static void test_wolfSSH_CheckAlgoList(void) { @@ -8249,6 +8363,7 @@ int wolfSSH_ApiTest(int argc, char** argv) test_wolfSSH_SetMaxAuthAttempts(); test_wolfSSH_AlgoListKeyInSync(); test_wolfSSH_SetAlgoList(); + test_wolfSSH_DefaultAlgoListsExcludeWeak(); test_wolfSSH_CheckAlgoList(); #ifdef WOLFSSH_FWD test_wolfSSH_FwdRemote_badArgs(); diff --git a/tests/regress.c b/tests/regress.c index 9b135211b..b0f3d063d 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -11879,6 +11879,84 @@ static void TestOct2DecRejectsInvalidNonLeadingDigit(void) wolfSSH_CTX_free(ctx); } + +/* fenrir 2479: the attribute encoder and decoder must agree. Extensions are + * unimplemented, so an encode of WOLFSSH_FILEATRB_EXT writes a zero count and + * the peer's decoder consumes the block instead of reading past it looking for + * records that were never written. */ +static void TestSftpAttributesRoundTrip(void) +{ + static const struct { + word32 flags; + word32 sz; + } cases[] = { + { 0, 4 }, + { WOLFSSH_FILEATRB_PERM, 8 }, + { WOLFSSH_FILEATRB_SIZE | WOLFSSH_FILEATRB_UIDGID + | WOLFSSH_FILEATRB_PERM | WOLFSSH_FILEATRB_TIME, 32 }, + { WOLFSSH_FILEATRB_EXT, 8 }, + { WOLFSSH_FILEATRB_PERM | WOLFSSH_FILEATRB_EXT, 12 }, + { WOLFSSH_FILEATRB_SIZE | WOLFSSH_FILEATRB_UIDGID + | WOLFSSH_FILEATRB_PERM | WOLFSSH_FILEATRB_TIME + | WOLFSSH_FILEATRB_EXT, 36 }, + }; + word32 i; + + for (i = 0; i < (word32)(sizeof(cases) / sizeof(cases[0])); i++) { + WS_SFTP_FILEATRB in, out; + byte buf[64]; + word32 idx = 0; + int encSz; + + WMEMSET(&in, 0, sizeof(in)); + WMEMSET(&out, 0, sizeof(out)); + WMEMSET(buf, 0xEE, sizeof(buf)); + + in.flags = cases[i].flags; + in.sz[0] = 0x44332211; + in.sz[1] = 0x88776655; + in.uid = 1000; + in.gid = 1001; + in.per = 0640; + in.atime = 0x5A5A5A5A; + in.mtime = 0x6B6B6B6B; + /* a caller that set the flag may well have set a count too */ + in.extCount = 3; + + encSz = wolfSSH_TestSftpSetAttributes(buf, (word32)sizeof(buf), &in); + AssertIntEQ(encSz, (int)cases[i].sz); + + /* the encoder writes exactly the size it advertises */ + AssertTrue(encSz < (int)sizeof(buf)); + AssertIntEQ(buf[encSz], 0xEE); + + /* the decoder consumes exactly that, over a buffer bounded to it */ + AssertIntEQ(wolfSSH_TestSftpParseAttributes(buf, (word32)encSz, &out, + &idx), WS_SUCCESS); + AssertIntEQ((int)idx, encSz); + + AssertIntEQ((int)out.flags, (int)in.flags); + if (in.flags & WOLFSSH_FILEATRB_SIZE) { + AssertIntEQ((int)out.sz[0], (int)in.sz[0]); + AssertIntEQ((int)out.sz[1], (int)in.sz[1]); + } + if (in.flags & WOLFSSH_FILEATRB_UIDGID) { + AssertIntEQ((int)out.uid, (int)in.uid); + AssertIntEQ((int)out.gid, (int)in.gid); + } + if (in.flags & WOLFSSH_FILEATRB_PERM) { + AssertIntEQ((int)out.per, (int)in.per); + } + if (in.flags & WOLFSSH_FILEATRB_TIME) { + AssertIntEQ((int)out.atime, (int)in.atime); + AssertIntEQ((int)out.mtime, (int)in.mtime); + } + /* no extension records are written, so none come back */ + AssertIntEQ((int)out.extCount, 0); + AssertNull(out.exts); + } +} + #endif /* WOLFSSH_SFTP */ #if !(defined(WOLFSSH_NO_RSA) && defined(WOLFSSH_NO_ECDSA_SHA2_NISTP256)) @@ -16447,6 +16525,8 @@ int main(int argc, char** argv) #ifdef WOLFSSH_SFTP TestOct2DecRejectsInvalidNonLeadingDigit(); TestSftpBufferSendPendingOutput(); + /* the attribute encoder and decoder agree, extensions included */ + TestSftpAttributesRoundTrip(); #if !defined(NO_WOLFSSH_SERVER) && !defined(USE_WINDOWS_API) && \ !defined(NO_FILESYSTEM) /* fenrir 4232/4343/4346/4349: forged SFTP file handles must be rejected */ diff --git a/wolfssh/wolfsftp.h b/wolfssh/wolfsftp.h index 676912aa6..9b3c9aefd 100644 --- a/wolfssh/wolfsftp.h +++ b/wolfssh/wolfsftp.h @@ -354,6 +354,10 @@ WOLFSSH_LOCAL void wolfSSH_SFTP_ShowSizes(void); WOLFSSH_API int wolfSSH_TestSftpDoName(WOLFSSH* ssh); WOLFSSH_API int wolfSSH_TestSftpGetHandle(WOLFSSH* ssh, byte* handle, word32* handleSz); + WOLFSSH_API int wolfSSH_TestSftpSetAttributes(byte* buf, word32 bufSz, + WS_SFTP_FILEATRB* atr); + WOLFSSH_API int wolfSSH_TestSftpParseAttributes(byte* buf, word32 bufSz, + WS_SFTP_FILEATRB* atr, word32* idx); WOLFSSH_API int wolfSSH_TestSftpSendCap(WOLFSSH* ssh, word32 cap); WOLFSSH_API int wolfSSH_TestSftpStallPending(WOLFSSH* ssh, word32 count); #if !defined(NO_WOLFSSH_SERVER) && !defined(NO_FILESYSTEM)