diff --git a/apps/wolfsshd/auth.c b/apps/wolfsshd/auth.c index 2b9611e56..00f0ed5e5 100644 --- a/apps/wolfsshd/auth.c +++ b/apps/wolfsshd/auth.c @@ -423,6 +423,7 @@ static int CheckPasswordHashUnix(const char* input, char* stored) (const byte*)stored, storedSz) != 0) { ret = WSSHD_AUTH_FAILURE; } + WS_FORCEZERO(hashedInput, hashedInputSz); } } diff --git a/apps/wolfsshd/configuration.c b/apps/wolfsshd/configuration.c index c03c95a07..e355a99de 100644 --- a/apps/wolfsshd/configuration.c +++ b/apps/wolfsshd/configuration.c @@ -151,7 +151,7 @@ static long GetConfigInt(const char* in, int inSz, int isTime, void* heap) WMEMCPY(num, in, sz); num[sz] = '\0'; ret = atol(num); - if (ret == 0 && WSTRCMP(in, "0") != 0) { + if (ret == 0 && WSTRCMP(num, "0") != 0) { ret = WS_BAD_ARGUMENT; } else if (ret > 0) { diff --git a/apps/wolfsshd/test/test_configuration.c b/apps/wolfsshd/test/test_configuration.c index 1fc371a5d..5ec82bae7 100644 --- a/apps/wolfsshd/test/test_configuration.c +++ b/apps/wolfsshd/test/test_configuration.c @@ -262,6 +262,10 @@ static int test_ParseConfigLine(void) {"Invalid login grace time", "LoginGraceTime wolfsshd", 1}, {"Bare multiplier m (no digit)", "LoginGraceTime m", 1}, {"Bare multiplier h (no digit)", "LoginGraceTime h", 1}, + {"Valid login grace time zero", "LoginGraceTime 0", 0}, + {"Valid login grace time zero minutes", "LoginGraceTime 0m", 0}, + {"Valid login grace time zero hours", "LoginGraceTime 0h", 0}, + {"Invalid zero padded login grace time", "LoginGraceTime 00", 1}, /* Permit empty password tests. */ {"Permit empty password no", "PermitEmptyPasswords no", 0}, diff --git a/examples/client/common.c b/examples/client/common.c index 4fa7c6d86..b9df3416e 100644 --- a/examples/client/common.c +++ b/examples/client/common.c @@ -52,6 +52,7 @@ static byte userPublicKeyBuf[512]; static byte* userPublicKey = userPublicKeyBuf; +static byte userPublicKeyAlloc = 0; static const byte* userPublicKeyType = NULL; static byte userPassword[256]; static const byte* userPrivateKeyType = NULL; @@ -284,7 +285,7 @@ static int load_der_file(const char* filename, byte** out, word32* outSz, } WREWIND(NULL, file); - in = (byte*)WMALLOC(inSz, heap, 0); + in = (byte*)WMALLOC(inSz, heap, DYNTYPE_PRIVKEY); if (in == NULL) { WFCLOSE(NULL, file); return -1; @@ -293,7 +294,7 @@ static int load_der_file(const char* filename, byte** out, word32* outSz, ret = (int)WFREAD(NULL, in, 1, inSz, file); if (ret <= 0 || ret != inSz) { ret = -1; - WFREE(in, heap, 0); + WFREE(in, heap, DYNTYPE_PRIVKEY); in = 0; inSz = 0; } @@ -757,6 +758,15 @@ int ClientUseCert(const char* certName, void* heap) userPublicKeyType = publicKeyType; userPublicKeyTypeSz = (word32)WSTRLEN((const char*)publicKeyType); pubKeyLoaded = 1; + userPublicKeyAlloc = 1; + } + else { + /* Defensive: load_der_file() clears its output pointer on the + * short-read failure, so put the static buffer back. */ + userPublicKey = userPublicKeyBuf; + userPublicKeySz = 0; + userPublicKeyType = NULL; + userPublicKeyAlloc = 0; } #else (void)heap; @@ -860,7 +870,7 @@ static int readKeyBlob(const char* filename, WOLFTPM2_KEYBLOB* key) } static int wolfSSH_TPM_InitKey(WOLFTPM2_DEV* dev, const char* name, - WOLFTPM2_KEY* pTpmKey, const char* tpmKeyAuth) + WOLFTPM2_KEY* pTpmKey, const char* tpmKeyAuth, void* heap) { int rc = 0; WOLFTPM2_KEY endorse; @@ -943,11 +953,14 @@ static int wolfSSH_TPM_InitKey(WOLFTPM2_DEV* dev, const char* name, /* Read public key from buffer and convert key to OpenSSH format */ if (rc == 0) { + /* Allocate from the caller's heap, ClientFreeBuffers() frees it + * with the same one. */ rc = wolfSSH_ReadPublicKey_buffer(userPublicKey, userPublicKeySz, WOLFSSH_FORMAT_ASN1, &p, &userPublicKeySz, &userPublicKeyType, - &userPublicKeyTypeSz, NULL); + &userPublicKeyTypeSz, heap); if (rc == 0) { userPublicKey = p; + userPublicKeyAlloc = 1; } else { WLOG(WS_LOG_DEBUG, "Reading public key failed, rc: %d", rc); } @@ -1030,7 +1043,8 @@ int ClientSetPrivateKey(const char* privKeyName, int userEcc, */ WMEMSET(&tpmDev, 0, sizeof(tpmDev)); WMEMSET(&tpmKey, 0, sizeof(tpmKey)); - ret = wolfSSH_TPM_InitKey(&tpmDev, privKeyName, &tpmKey, tpmKeyAuth); + ret = wolfSSH_TPM_InitKey(&tpmDev, privKeyName, &tpmKey, tpmKeyAuth, + heap); #elif !defined(NO_FILESYSTEM) userPrivateKey = NULL; /* create new buffer based on parsed input */ userPrivateKeyAlloc = 1; @@ -1089,6 +1103,13 @@ int ClientUsePubKey(const char* pubKeyName, int userEcc, void* heap) #endif /* NO_FILESYSTEM */ if (ret == 0) { pubKeyLoaded = 1; + userPublicKeyAlloc = 1; + } + else { + userPublicKey = userPublicKeyBuf; + userPublicKeySz = 0; + userPublicKeyType = NULL; + userPublicKeyAlloc = 0; } } @@ -1112,7 +1133,7 @@ int ClientLoadCA(WOLFSSH_CTX* ctx, const char* caCert) fprintf(stderr, "Couldn't parse in CA certificate."); ret = WS_PARSE_E; } - WFREE(der, NULL, 0); + WFREE(der, ctx->heap, DYNTYPE_PRIVKEY); } #else (void)ctx; @@ -1132,11 +1153,16 @@ void ClientFreeBuffers(const char* pubKeyName, const char* privKeyName, #ifdef WOLFSSH_TPM wolfSSH_TPM_Cleanup(&tpmDev, &tpmKey); #endif - if (pubKeyName != NULL && userPublicKey != NULL && - userPublicKey != userPublicKeyBuf) { + /* The public key buffer is tracked by userPublicKeyAlloc, not by + * pubKeyName: ClientUseCert() allocates one without a public key file + * name being given. */ + (void)pubKeyName; + + if (userPublicKeyAlloc && userPublicKey != NULL) { WFREE(userPublicKey, heap, DYNTYPE_PRIVKEY); userPublicKey = userPublicKeyBuf; userPublicKeySz = 0; + userPublicKeyAlloc = 0; } if (privKeyName != NULL && userPrivateKey != NULL) { diff --git a/examples/echoserver/echoserver.c b/examples/echoserver/echoserver.c index 92ea43bbf..4bb178bcf 100644 --- a/examples/echoserver/echoserver.c +++ b/examples/echoserver/echoserver.c @@ -2352,41 +2352,68 @@ static int LoadPubKeyList(StrList* strList, int format, PwMapList* mapList) fileName++; load_file(fileName, NULL, &bufSz); - buf = (byte*)WMALLOC(bufSz, NULL, 0); - bufSz = load_file(fileName, buf, &bufSz); - if (bufSz > 0) { + if (bufSz == 0) { + fprintf(stderr, "File error: %s\n", fileName); + } + else if ((buf = (byte*)WMALLOC(bufSz, NULL, 0)) == NULL) { + fprintf(stderr, "Memory error: %s\n", fileName); + } + else if ((bufSz = load_file(fileName, buf, &bufSz)) == 0) { + fprintf(stderr, "File error: %s\n", fileName); + } + else { + byte* out = NULL; + word32 outSz = 0; + int ok = 1; + if (format == WOLFSSH_FORMAT_SSH) { const byte* type = NULL; - byte* out = NULL; - word32 typeSz, outSz; + word32 typeSz = 0; - wolfSSH_ReadKey_buffer(buf, bufSz, WOLFSSH_FORMAT_SSH, - &out, &outSz, &type, &typeSz, NULL); + if (wolfSSH_ReadKey_buffer(buf, bufSz, WOLFSSH_FORMAT_SSH, + &out, &outSz, &type, &typeSz, NULL) + != WS_SUCCESS || out == NULL) { + fprintf(stderr, "ReadKey error: %s\n", fileName); + ok = 0; + } (void)type; (void)typeSz; - - WFREE(buf, NULL, 0); - buf = out; - bufSz = outSz; } else if (format == WOLFSSH_FORMAT_PEM) { - byte* out = NULL; - word32 outSz; - out = (byte*)WMALLOC(bufSz, NULL, 0); - outSz = wc_CertPemToDer(buf, bufSz, out, bufSz, CERT_TYPE); + if (out == NULL) { + fprintf(stderr, "Memory error: %s\n", fileName); + ok = 0; + } + else { + int rc = wc_CertPemToDer(buf, bufSz, out, bufSz, + CERT_TYPE); - WFREE(buf, NULL, 0); - buf = out; - bufSz = outSz; + if (rc <= 0) { + fprintf(stderr, "PEM error: %s\n", fileName); + ok = 0; + } + else { + outSz = (word32)rc; + } + } } - PwMapNew(mapList, WOLFSSH_USERAUTH_PUBLICKEY, - (byte*)names, (word32)WSTRLEN(names), buf, bufSz); - } - else { - fprintf(stderr, "File error: %s\n", names); + if (ok) { + /* Converted key replaces the raw file contents. */ + if (out != NULL) { + WFREE(buf, NULL, 0); + buf = out; + bufSz = outSz; + out = NULL; + } + + PwMapNew(mapList, WOLFSSH_USERAUTH_PUBLICKEY, + (byte*)names, (word32)WSTRLEN(names), buf, bufSz); + } + + WFREE(out, NULL, 0); } } else { diff --git a/examples/portfwd/portfwd.c b/examples/portfwd/portfwd.c index 405100452..1d2aaa445 100644 --- a/examples/portfwd/portfwd.c +++ b/examples/portfwd/portfwd.c @@ -690,17 +690,23 @@ THREAD_RETURN WOLFSSH_THREAD portfwd_worker(void* args) rxFds = templateFds; errFds = templateFds; + /* A readable appFd with a full buffer would spin select(). Mask the + * read set only, so errors are still noticed. */ + if (appFdSet && appBufferUsed == appBufferSz) + FD_CLR(appFd, &rxFds); + to.tv_sec = 1; to.tv_usec = 0; ret = select(nFds, &rxFds, NULL, &errFds, &to); + if (ret < 0) + err_sys("select failed\n"); if (ret == 0) { + /* Fall through rather than continue, a short send leaves a tail + * owed another attempt. select() cleared the sets. */ ret = wolfSSH_SendIgnore(ssh, NULL, 0); if (ret != WS_SUCCESS) err_sys("Couldn't send an ignore message."); - continue; } - else if (ret < 0) - err_sys("select failed\n"); if ((appFdSet && FD_ISSET(appFd, &errFds)) || FD_ISSET(sshFd, &errFds) || @@ -708,7 +714,10 @@ THREAD_RETURN WOLFSSH_THREAD portfwd_worker(void* args) err_sys("some socket had an error"); } - if (appFdSet && FD_ISSET(appFd, &rxFds)) { + /* Skip the read when the buffer is full, a zero length recv() returns + * 0 and would be mistaken for the peer closing. */ + if (appFdSet && appBufferUsed < appBufferSz && + FD_ISSET(appFd, &rxFds)) { int rxd; rxd = (int)recv(appFd, appBuffer + appBufferUsed, appBufferSz - appBufferUsed, 0); @@ -789,8 +798,12 @@ THREAD_RETURN WOLFSSH_THREAD portfwd_worker(void* args) } if (appBufferUsed > 0) { ret = wolfSSH_ChannelSend(fwdChannel, appBuffer, appBufferUsed); - if (ret > 0) + if (ret > 0) { + if ((word32)ret != appBufferUsed) { + WMEMMOVE(appBuffer, appBuffer + ret, appBufferUsed - ret); + } appBufferUsed -= ret; + } else if (ret == WS_CHANNEL_NOT_CONF || ret == WS_CHAN_RXD) { #ifdef SHELL_DEBUG printf("Waiting for channel open confirmation.\n"); diff --git a/src/agent.c b/src/agent.c index 79a9390bd..0d030704b 100644 --- a/src/agent.c +++ b/src/agent.c @@ -1701,6 +1701,11 @@ int wolfSSH_AGENT_worker(WOLFSSH* ssh) if (ssh == NULL) ret = WS_SSH_NULL_E; + if (ret == WS_SUCCESS) { + if (ssh->agent == NULL) + ret = WS_AGENT_NULL_E; + } + if (ret == WS_SUCCESS) { SendSuccess(NULL); DoMessage(NULL, NULL, 0, NULL); diff --git a/src/certman.c b/src/certman.c index 66345f48a..2674ca2f7 100644 --- a/src/certman.c +++ b/src/certman.c @@ -93,6 +93,7 @@ static WOLFSSH_CERTMAN* _CertMan_init(WOLFSSH_CERTMAN* cm, void* heap) ret = cm; if (ret != NULL) { WMEMSET(ret, 0, sizeof(WOLFSSH_CERTMAN)); + ret->heap = heap; ret->cm = wolfSSL_CertManagerNew_ex(heap); if (ret->cm == NULL) { ret = NULL; @@ -507,20 +508,24 @@ static int CheckProfile(DecodedCert* cert, int profile) const byte* date; int dateSz; byte dateFormat; - struct tm t; + struct tm t = { 0 }; dateFormat = cert->afterDate[0]; /* i.e ASN_UTC_TIME */ dateSz = cert->afterDate[1]; date = &cert->afterDate[2]; - wc_GetDateAsCalendarTime(date, dateSz, dateFormat, &t); - if (t.tm_year <= 149 && dateFormat != ASN_UTC_TIME) { + if (wc_GetDateAsCalendarTime(date, dateSz, dateFormat, &t) != 0) { + WLOG(WS_LOG_CERTMAN, "unable to get date"); + valid = 0; + } + + if (valid && t.tm_year <= 149 && dateFormat != ASN_UTC_TIME) { WLOG(WS_LOG_CERTMAN, "date format was not utc for year %d", t.tm_year); valid = 0; } - if (t.tm_year > 149 && dateFormat != ASN_GENERALIZED_TIME) { + if (valid && t.tm_year > 149 && dateFormat != ASN_GENERALIZED_TIME) { WLOG(WS_LOG_CERTMAN, "date format was not general for year %d", t.tm_year); valid = 0; diff --git a/src/internal.c b/src/internal.c index 0fe3ad9f6..f67c9b5a8 100644 --- a/src/internal.c +++ b/src/internal.c @@ -1650,7 +1650,9 @@ void SshResourceFree(WOLFSSH* ssh, void* heap) ShrinkBuffer(&ssh->inputBuffer, 1); ShrinkBuffer(&ssh->outputBuffer, 1); - WS_FORCEZERO(ssh->k, ssh->kSz); + /* Use the sizeof k, as kSz can change. */ + WS_FORCEZERO(ssh->k, sizeof(ssh->k)); + ssh->kSz = 0; HandshakeInfoFree(ssh->handshake, heap); WS_FORCEZERO(&ssh->keys, sizeof(Keys)); WS_FORCEZERO(&ssh->peerKeys, sizeof(Keys)); @@ -11985,14 +11987,20 @@ static INLINE int VerifyMac(WOLFSSH* ssh, const byte* in, word32 inSz, #ifndef WOLFSSH_NO_AEAD +/* Increment the explicit portion of the AEAD nonce. The carry runs the full + * width with no early exit, so the timing doesn't depend on the counter, + * which starts out as KDF output. */ static INLINE void AeadIncrementExpIv(byte* iv) { int i; + unsigned int carry = 1; iv += AEAD_IMP_IV_SZ; for (i = AEAD_EXP_IV_SZ-1; i >= 0; i--) { - if (++iv[i]) return; + unsigned int sum = (unsigned int)iv[i] + carry; + iv[i] = (byte)sum; + carry = (sum >> 8) & 1u; } } diff --git a/src/keygen.c b/src/keygen.c index 5acfc268b..240d25907 100644 --- a/src/keygen.c +++ b/src/keygen.c @@ -77,11 +77,13 @@ int wolfSSH_MakeRsaKey(byte* out, word32 outSz, word32 size, word32 e) if (ret == WS_SUCCESS) { RsaKey key; + int keyInit = 0; if (wc_InitRsaKey(&key, NULL) != 0) ret = WS_CRYPTO_FAILED; if (ret == WS_SUCCESS) { + keyInit = 1; if (wc_MakeRsaKey(&key, size, e, &rng) != 0) { WLOG(WS_LOG_DEBUG, "RSA key generation failed"); ret = WS_CRYPTO_FAILED; @@ -100,10 +102,12 @@ int wolfSSH_MakeRsaKey(byte* out, word32 outSz, word32 size, word32 e) ret = keySz; } - if (wc_FreeRsaKey(&key) != 0) { - WLOG(WS_LOG_DEBUG, "RSA key free failed"); - if (ret >= 0) - ret = WS_CRYPTO_FAILED; + if (keyInit) { + if (wc_FreeRsaKey(&key) != 0) { + WLOG(WS_LOG_DEBUG, "RSA key free failed"); + if (ret >= 0) + ret = WS_CRYPTO_FAILED; + } } if (wc_FreeRng(&rng) != 0) { @@ -141,11 +145,13 @@ int wolfSSH_MakeEcdsaKey(byte* out, word32 outSz, word32 size) if (ret == WS_SUCCESS) { ecc_key key; + int keyInit = 0; if (wc_ecc_init(&key) != 0) ret = WS_CRYPTO_FAILED; if (ret == WS_SUCCESS) { + keyInit = 1; ret = wc_ecc_make_key(&rng, size/8, &key); if (ret != 0) { WLOG(WS_LOG_DEBUG, "ECDSA key generation failed"); @@ -167,10 +173,12 @@ int wolfSSH_MakeEcdsaKey(byte* out, word32 outSz, word32 size) ret = keySz; } - if (wc_ecc_free(&key) != 0) { - WLOG(WS_LOG_DEBUG, "ECDSA key free failed"); - if (ret >= 0) - ret = WS_CRYPTO_FAILED; + if (keyInit) { + if (wc_ecc_free(&key) != 0) { + WLOG(WS_LOG_DEBUG, "ECDSA key free failed"); + if (ret >= 0) + ret = WS_CRYPTO_FAILED; + } } if (wc_FreeRng(&rng) != 0) { @@ -208,11 +216,13 @@ int wolfSSH_MakeEd25519Key(byte* out, word32 outSz, word32 size) if (ret == WS_SUCCESS) { ed25519_key key; + int keyInit = 0; if (wc_ed25519_init(&key) != 0) ret = WS_CRYPTO_FAILED; if (ret == WS_SUCCESS) { + keyInit = 1; ret = wc_ed25519_make_key(&rng, size/8, &key); if (ret != 0) { WLOG(WS_LOG_DEBUG, "ED25519 key generation failed"); @@ -234,7 +244,9 @@ int wolfSSH_MakeEd25519Key(byte* out, word32 outSz, word32 size) ret = keySz; } - wc_ed25519_free(&key); + if (keyInit) { + wc_ed25519_free(&key); + } if (wc_FreeRng(&rng) != 0) { WLOG(WS_LOG_DEBUG, "Couldn't free RNG"); diff --git a/src/ssh.c b/src/ssh.c index fa7bfb60f..44d17b816 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -1830,7 +1830,7 @@ static int DoSshPubKey(const byte* in, word32 inSz, byte** out, c = (char*)WMALLOC(inSz + 1, heap, DYNTYPE_STRING); if (c != NULL) { WMEMCPY(c, in, inSz); - c[inSz-1] = 0; + c[inSz] = 0; type = WSTRTOK(c, " \n", &last); key = WSTRTOK(NULL, " \n", &last); } @@ -2113,7 +2113,7 @@ static int DoPemKey(const byte* in, word32 inSz, byte** out, } if (ret == WS_SUCCESS) { - ret = IdentifyAsn1Key(newKey, newKeySz, 1, heap, NULL); + ret = IdentifyAsn1Key(newKey, newKeySz, isPrivate, heap, NULL); } if (ret > 0) { diff --git a/src/wolfsftp.c b/src/wolfsftp.c index 57804eb8f..6a5648727 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -7168,6 +7168,10 @@ static WS_SFTPNAME* wolfSSH_SFTP_DoName(WOLFSSH* ssh) return NULL; } + /* ret still holds the read length; clear it so a count of 0 + * doesn't hit the error path below. */ + ret = WS_SUCCESS; + while (count > 0) { word32 sz; const byte* str; diff --git a/tests/api.c b/tests/api.c index 6b48184ef..815e9beae 100644 --- a/tests/api.c +++ b/tests/api.c @@ -1154,6 +1154,105 @@ static void test_wolfSSH_ReadKey(void) } +#ifndef WOLFSSH_NO_RSA +/* SubjectPublicKeyInfo for the same RSA key as serverKeyRsaDer. */ +static const char serverKeyRsaPubPem[] = + "-----BEGIN PUBLIC KEY-----\n" + "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA2l2tJRR2FVnzQP08uGIw\n" + "s23A+ezsi4MenkKcykFq04rhUjTgDRNiftQPrlxNBPGN+sWtd6paBcrv+I2r/4op\n" + "CUwEwvUZy+0fsbQp08NsqSPfo6DlCN6tjHH5NIhs7Tvwb6UPrFn/azPxcPuMpLNF\n" + "Io2dd3rlKV+EFNmZ6urOLVHz41j6WwIPybUqvLJe08Iwuzyxw+9Y81CUKIvEZUr3\n" + "ANmX2WtNjZWhimIGtFARIoO06irn0KggR0//Rq7FE+E4i/hUrzpNL/gf14SQ2JMF\n" + "BsJ9kNvjnNDEZVoDrQCsWqLN2j+JWDdTvytGeqyJQStaLuh2517jKYWjY+rmhmB8\n" + "LQIDAQAB\n" + "-----END PUBLIC KEY-----\n"; +/* The same SubjectPublicKeyInfo in DER, what the PEM above decodes to. */ +static const char serverKeyRsaPubDer[] = + "30820122300d06092a864886f70d01010105000382010f003082010a02820101" + "00da5dad2514761559f340fd3cb86230b36dc0f9ecec8b831e9e429cca416ad3" + "8ae15234e00d13627ed40fae5c4d04f18dfac5ad77aa5a05caeff88dabff8a29" + "094c04c2f519cbed1fb1b429d3c36ca923dfa3a0e508dead8c71f934886ced3b" + "f06fa50fac59ff6b33f170fb8ca4b345228d9d777ae5295f8414d999eaeace2d" + "51f3e358fa5b020fc9b52abcb25ed3c230bb3cb1c3ef58f35094288bc4654af7" + "00d997d96b4d8d95a18a6206b450112283b4ea2ae7d0a820474fff46aec513e1" + "388bf854af3a4d2ff81fd78490d8930506c27d90dbe39cd0c4655a03ad00ac5a" + "a2cdda3f89583753bf2b467aac89412b5a2ee876e75ee32985a363eae686607c" + "2d0203010001"; +#endif + + +static void test_wolfSSH_ReadPublicKey_pem(void) +{ +#ifndef WOLFSSH_NO_RSA + /* DoPemKey() used to pass a literal 1 for isPrivate when identifying the + * decoded key, so a public PEM converted by wc_PubKeyPemToDer() was then + * run through the private-key decoders and never identified. */ + byte* key = NULL; + word32 keySz = 0; + const byte* keyType = NULL; + word32 keyTypeSz = 0; + word32 pemSz = (word32)WSTRLEN(serverKeyRsaPubPem); + int ret; + + ret = wolfSSH_ReadPublicKey_buffer((const byte*)serverKeyRsaPubPem, pemSz, + WOLFSSH_FORMAT_PEM, &key, &keySz, &keyType, &keyTypeSz, NULL); + +#ifdef WOLFSSH_TPM + /* The public PEM branch of DoPemKey() is only compiled in with TPM + * support, so this is the only build where the read can succeed. */ + AssertIntEQ(ret, WS_SUCCESS); + AssertNotNull(key); + AssertIntGT(keySz, 0); + AssertIntLT(keySz, pemSz); /* DER is smaller than the PEM it came from */ + AssertStrEQ(keyType, "ssh-rsa"); + AssertIntEQ(keyTypeSz, (word32)WSTRLEN("ssh-rsa")); + + /* The output is the decoded SubjectPublicKeyInfo. Re-reading it as ASN.1 + * confirms a public key came back, not a mis-tagged private one. */ + { + byte* sshKey = NULL; + word32 sshKeySz = 0; + const byte* sshKeyType = NULL; + word32 sshKeyTypeSz = 0; + + AssertIntEQ(wolfSSH_ReadPublicKey_buffer(key, keySz, + WOLFSSH_FORMAT_ASN1, &sshKey, &sshKeySz, + &sshKeyType, &sshKeyTypeSz, NULL), WS_SUCCESS); + AssertNotNull(sshKey); + AssertStrEQ(sshKeyType, "ssh-rsa"); + WFREE(sshKey, NULL, DYNTYPE_PRIVKEY); + } + + WFREE(key, NULL, DYNTYPE_PRIVKEY); +#else + /* Without TPM support the branch is compiled out and the read must fail + * cleanly, leaving the outputs untouched. */ + AssertIntEQ(ret, WS_PARSE_E); + AssertNull(key); + AssertIntEQ(keySz, 0); + AssertNull(keyType); + AssertIntEQ(keyTypeSz, 0); +#endif /* WOLFSSH_TPM */ + + /* The public PEM branch above only builds with TPM support, so pin the + * behavior it depends on here where every build runs it: the decoded + * SubjectPublicKeyInfo identifies as ssh-rsa only when isPrivate is 0. + * Hardcoding 1, as DoPemKey() once did, cannot identify it. */ + { + byte* der = NULL; + word32 derSz = 0; + + AssertIntEQ(ConvertHexToBin(serverKeyRsaPubDer, &der, &derSz, + NULL, NULL, NULL, NULL, NULL, NULL, + NULL, NULL, NULL), 0); + AssertIntEQ(IdentifyAsn1Key(der, derSz, 0, NULL, NULL), ID_SSH_RSA); + AssertIntLT(IdentifyAsn1Key(der, derSz, 1, NULL, NULL), 0); + WFREE(der, NULL, 0); + } +#endif /* WOLFSSH_NO_RSA */ +} + + static void test_wolfSSH_ReadKey_badPad(void) { #ifndef WOLFSSH_NO_ECDSA_SHA2_NISTP256 @@ -1284,8 +1383,62 @@ static void test_wolfSSH_ReadKey_noTrailingNewline(void) AssertIntEQ(WMEMCMP(keyTrim, keyRef, keyRefSz), 0); AssertStrEQ(keyType, "ssh-rsa"); - WFREE(keyRef, NULL, DYNTYPE_FILE); - WFREE(keyTrim, NULL, DYNTYPE_FILE); + WFREE(keyRef, NULL, DYNTYPE_PRIVKEY); + WFREE(keyTrim, NULL, DYNTYPE_PRIVKEY); +#endif +} + + +static void test_wolfSSH_ReadKey_sshNoComment(void) +{ +#ifndef WOLFSSH_NO_RSA + /* An SSH format public key whose buffer ends on the last base64 + * character, with no comment and no trailing newline, must decode to the + * same bytes as the canonical form. DoSshPubKey()'s old "c[inSz-1] = 0" + * wrote the null terminator over that last character. */ + byte* keyRef = NULL; + byte* keyTrim = NULL; + word32 keyRefSz = 0; + word32 keyTrimSz = 0; + const byte* keyType = NULL; + word32 keyTypeSz = 0; + const char* blob; + const char* end; + word32 fullSz = (word32)WSTRLEN(id_rsa_pub); + word32 trimSz; + int ret; + + /* Canonical parse, the input has a comment and a trailing newline. */ + ret = wolfSSH_ReadKey_buffer((const byte*)id_rsa_pub, fullSz, + WOLFSSH_FORMAT_SSH, &keyRef, &keyRefSz, + &keyType, &keyTypeSz, NULL); + AssertIntEQ(ret, WS_SUCCESS); + AssertNotNull(keyRef); + AssertIntGT(keyRefSz, 0); + AssertStrEQ(keyType, "ssh-rsa"); + + /* Length up to the end of the base64 blob, dropping the comment. */ + blob = WSTRCHR(id_rsa_pub, ' '); /* end of the "ssh-rsa" type */ + AssertNotNull(blob); + end = WSTRCHR(blob + 1, ' '); /* end of the base64 blob */ + AssertNotNull(end); + trimSz = (word32)(end - id_rsa_pub); + AssertIntLT(trimSz, fullSz); + + keyType = NULL; + keyTypeSz = 0; + ret = wolfSSH_ReadKey_buffer((const byte*)id_rsa_pub, trimSz, + WOLFSSH_FORMAT_SSH, &keyTrim, &keyTrimSz, + &keyType, &keyTypeSz, NULL); + AssertIntEQ(ret, WS_SUCCESS); + AssertNotNull(keyTrim); + /* Must match the canonical decode exactly, not be truncated by a byte. */ + AssertIntEQ(keyTrimSz, keyRefSz); + AssertIntEQ(WMEMCMP(keyTrim, keyRef, keyRefSz), 0); + AssertStrEQ(keyType, "ssh-rsa"); + + WFREE(keyRef, NULL, DYNTYPE_PRIVKEY); + WFREE(keyTrim, NULL, DYNTYPE_PRIVKEY); #endif } @@ -5198,9 +5351,11 @@ int wolfSSH_ApiTest(int argc, char** argv) test_wolfSSH_CTX_SetWindowPacketSize(); test_wolfSSH_CertMan(); test_wolfSSH_ReadKey(); + test_wolfSSH_ReadPublicKey_pem(); test_wolfSSH_ReadKey_badPad(); test_wolfSSH_ReadKey_shortBuffer(); test_wolfSSH_ReadKey_noTrailingNewline(); + test_wolfSSH_ReadKey_sshNoComment(); test_wolfSSH_QueryAlgoList(); test_wolfSSH_SetMaxAuthAttempts(); test_wolfSSH_AlgoListKeyInSync();