Skip to content

Commit b8e2fd3

Browse files
aidangarskedgarske
authored andcommitted
Address review: TPM publickey auth fallback, reject truncated CA, guard negative CI test, silence maybe-uninitialized
1 parent d818d03 commit b8e2fd3

5 files changed

Lines changed: 21 additions & 5 deletions

File tree

.github/workflows/tpm-ssh.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,8 +186,14 @@ jobs:
186186
cat tpmcert_client_neg.txt
187187
exit 1
188188
fi
189+
echo "----- server -----"; cat tpmcert_server_neg.txt
189190
echo "client correctly rejected the untrusted server:"
190191
cat tpmcert_client_neg.txt
192+
# Guard against a false pass: the server must have come up, and the
193+
# client must have failed inside the SSH handshake (certificate
194+
# rejection) rather than from a plain connection failure.
195+
grep -q "Listening on port" tpmcert_server_neg.txt
196+
grep -q "wolfSSH_connect failed" tpmcert_client_neg.txt
191197
192198
# Client public-key authentication with a TPM-resident key (RSA only).
193199
- name: Test TPM client public-key auth

examples/tpmcertserver/tpmcertclient.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,14 +72,18 @@ static int TpmCcLoadFile(const char* file, byte* buf, word32* bufSz)
7272
int ret = 0;
7373
FILE* f = fopen(file, "rb");
7474
size_t n;
75+
int extra;
7576

7677
if (f == NULL) {
7778
ret = -1;
7879
}
7980
else {
8081
n = fread(buf, 1, *bufSz, f);
82+
/* If the buffer filled exactly, the file may be larger than the buffer;
83+
* reject a truncated read rather than loading a partial certificate. */
84+
extra = (n == (size_t)*bufSz) ? fgetc(f) : EOF;
8185
fclose(f);
82-
if (n == 0)
86+
if (n == 0 || extra != EOF)
8387
ret = -1;
8488
else
8589
*bufSz = (word32)n;

examples/tpmcertserver/tpmcertserver.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,11 +84,11 @@ static int TpmCsMakeKeyAndCert(WOLFTPM2_DEV* dev, WOLFTPM2_KEY* key,
8484
{
8585
int rc;
8686
int devId = INVALID_DEVID;
87-
int sigType;
87+
int sigType = 0;
8888
TpmCryptoDevCtx tpmCtx;
8989
WOLFTPM2_KEY srk;
9090
TPMT_PUBLIC pub;
91-
const char* subject;
91+
const char* subject = NULL;
9292
const char* keyUsage = "serverAuth,clientAuth";
9393
TPMA_OBJECT attr;
9494

src/internal.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17688,9 +17688,12 @@ int SendUserAuthRequest(WOLFSSH* ssh, byte authType, int addSig)
1768817688
#ifdef WOLFSSH_TPM
1768917689
/* When the client has a TPM key configured, prefer publickey auth so
1769017690
* the TPM key is used even if the server also offers password or
17691-
* keyboard-interactive. Applied before any method-specific branch. */
17692-
if (ssh->ctx->tpmKey != NULL
17691+
* keyboard-interactive. Only strip the other methods on the first
17692+
* attempt; once publickey has been tried and rejected, allow fallback
17693+
* to password/keyboard on the next DoUserAuthFailure() retry. */
17694+
if (ssh->ctx->tpmKey != NULL && !ssh->tpmPubkeyTried
1769317695
&& (authType & WOLFSSH_USERAUTH_PUBLICKEY)) {
17696+
ssh->tpmPubkeyTried = 1;
1769417697
authType &= ~WOLFSSH_USERAUTH_PASSWORD;
1769517698
#ifdef WOLFSSH_KEYBOARD_INTERACTIVE
1769617699
authType &= ~WOLFSSH_USERAUTH_KEYBOARD;

wolfssh/internal.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1033,6 +1033,9 @@ struct WOLFSSH {
10331033
WS_UserAuthData_Keyboard kbAuth;
10341034
byte kbAuthAttempts;
10351035
#endif
1036+
#ifdef WOLFSSH_TPM
1037+
byte tpmPubkeyTried; /* client tried TPM publickey; allow auth fallback */
1038+
#endif
10361039
#ifdef WOLFSSH_TEST_INTERNAL
10371040
word32 testSftpSendCap; /* test hook: cap per-call SFTP buffer send */
10381041
word32 testSftpStallPending; /* test hook: force N flush-only resumes */

0 commit comments

Comments
 (0)