Skip to content

Commit a28b3ac

Browse files
mscdexmscdex
committed
test: fix OpenSSH key auth checking
Some versions of OpenSSH send a signature immediately, while others send the "key check" first.
1 parent 77391c2 commit a28b3ac

File tree

1 file changed

+36
-8
lines changed

1 file changed

+36
-8
lines changed

‎test/test-integration-openssh.js‎

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ for (const file of readdirSync(FIXTURES_DIR, { withFileTypes: true })) {
9292

9393
server.on('connection', mustCall((conn) => {
9494
let authAttempt = 0;
95-
conn.on('authentication', mustCall((ctx) => {
95+
conn.on('authentication', mustCallAtLeast((ctx) => {
9696
assert(ctx.username === username,
9797
`Wrong username: ${ctx.username}`);
9898
switch (++authAttempt) {
@@ -101,7 +101,9 @@ for (const file of readdirSync(FIXTURES_DIR, { withFileTypes: true })) {
101101
`Wrong auth method: ${ctx.method}`);
102102
return ctx.reject();
103103
case 2:
104-
assert(ctx.signature, 'Missing publickey signature');
104+
case 3:
105+
if (authAttempt === 3)
106+
assert(ctx.signature, 'Missing publickey signature');
105107
assert(ctx.method === 'publickey',
106108
`Wrong auth method: ${ctx.method}`);
107109
assert(ctx.key.algo === clientKey.key.type,
@@ -110,10 +112,15 @@ for (const file of readdirSync(FIXTURES_DIR, { withFileTypes: true })) {
110112
ctx.key.data,
111113
'Public key mismatch');
112114
break;
115+
default:
116+
assert(false, 'Unexpected number of auth attempts');
113117
}
114118
if (ctx.signature) {
115119
assert(clientKey.key.verify(ctx.blob, ctx.signature) === true,
116120
'Could not verify publickey signature');
121+
// We should not expect any further auth attempts after we verify a
122+
// signature
123+
authAttempt = Infinity;
117124
}
118125
ctx.accept();
119126
}, 2)).on('ready', mustCall(() => {
@@ -159,7 +166,7 @@ for (const file of readdirSync(FIXTURES_DIR, { withFileTypes: true })) {
159166

160167
server.on('connection', mustCall((conn) => {
161168
let authAttempt = 0;
162-
conn.on('authentication', mustCall((ctx) => {
169+
conn.on('authentication', mustCallAtLeast((ctx) => {
163170
assert(ctx.username === username,
164171
`Wrong username: ${ctx.username}`);
165172
switch (++authAttempt) {
@@ -168,7 +175,9 @@ for (const file of readdirSync(FIXTURES_DIR, { withFileTypes: true })) {
168175
`Wrong auth method: ${ctx.method}`);
169176
return ctx.reject();
170177
case 2:
171-
assert(ctx.signature, 'Missing publickey signature');
178+
case 3:
179+
if (authAttempt === 3)
180+
assert(ctx.signature, 'Missing publickey signature');
172181
assert(ctx.method === 'publickey',
173182
`Wrong auth method: ${ctx.method}`);
174183
assert(ctx.key.algo === clientKey.key.type,
@@ -177,10 +186,15 @@ for (const file of readdirSync(FIXTURES_DIR, { withFileTypes: true })) {
177186
ctx.key.data,
178187
'Public key mismatch');
179188
break;
189+
default:
190+
assert(false, 'Unexpected number of auth attempts');
180191
}
181192
if (ctx.signature) {
182193
assert(clientKey.key.verify(ctx.blob, ctx.signature) === true,
183194
'Could not verify publickey signature');
195+
// We should not expect any further auth attempts after we verify a
196+
// signature
197+
authAttempt = Infinity;
184198
}
185199
ctx.accept();
186200
}, 2)).on('ready', mustCall(() => {
@@ -219,7 +233,7 @@ for (const file of readdirSync(FIXTURES_DIR, { withFileTypes: true })) {
219233
childProc.stdin.write('ping');
220234
})).on('connection', mustCall((conn) => {
221235
let authAttempt = 0;
222-
conn.on('authentication', mustCall((ctx) => {
236+
conn.on('authentication', mustCallAtLeast((ctx) => {
223237
assert(ctx.username === username,
224238
`Wrong username: ${ctx.username}`);
225239
switch (++authAttempt) {
@@ -228,7 +242,9 @@ for (const file of readdirSync(FIXTURES_DIR, { withFileTypes: true })) {
228242
`Wrong auth method: ${ctx.method}`);
229243
return ctx.reject();
230244
case 2:
231-
assert(ctx.signature, 'Missing publickey signature');
245+
case 3:
246+
if (authAttempt === 3)
247+
assert(ctx.signature, 'Missing publickey signature');
232248
assert(ctx.method === 'publickey',
233249
`Wrong auth method: ${ctx.method}`);
234250
assert(ctx.key.algo === clientKey.key.type,
@@ -237,10 +253,15 @@ for (const file of readdirSync(FIXTURES_DIR, { withFileTypes: true })) {
237253
ctx.key.data,
238254
'Public key mismatch');
239255
break;
256+
default:
257+
assert(false, 'Unexpected number of auth attempts');
240258
}
241259
if (ctx.signature) {
242260
assert(clientKey.key.verify(ctx.blob, ctx.signature) === true,
243261
'Could not verify publickey signature');
262+
// We should not expect any further auth attempts after we verify a
263+
// signature
264+
authAttempt = Infinity;
244265
}
245266
ctx.accept();
246267
}, 2)).on('ready', mustCall(() => {
@@ -278,7 +299,7 @@ for (const file of readdirSync(FIXTURES_DIR, { withFileTypes: true })) {
278299

279300
server.on('connection', mustCall((conn) => {
280301
let authAttempt = 0;
281-
conn.on('authentication', mustCall((ctx) => {
302+
conn.on('authentication', mustCallAtLeast((ctx) => {
282303
assert(ctx.username === username,
283304
`Wrong username: ${ctx.username}`);
284305
switch (++authAttempt) {
@@ -287,7 +308,9 @@ for (const file of readdirSync(FIXTURES_DIR, { withFileTypes: true })) {
287308
`Wrong auth method: ${ctx.method}`);
288309
return ctx.reject();
289310
case 2:
290-
assert(ctx.signature, 'Missing publickey signature');
311+
case 3:
312+
if (authAttempt === 3)
313+
assert(ctx.signature, 'Missing publickey signature');
291314
assert(ctx.method === 'publickey',
292315
`Wrong auth method: ${ctx.method}`);
293316
assert(ctx.key.algo === clientKey.key.type,
@@ -296,10 +319,15 @@ for (const file of readdirSync(FIXTURES_DIR, { withFileTypes: true })) {
296319
ctx.key.data,
297320
'Public key mismatch');
298321
break;
322+
default:
323+
assert(false, 'Unexpected number of auth attempts');
299324
}
300325
if (ctx.signature) {
301326
assert(clientKey.key.verify(ctx.blob, ctx.signature) === true,
302327
'Could not verify publickey signature');
328+
// We should not expect any further auth attempts after we verify a
329+
// signature
330+
authAttempt = Infinity;
303331
}
304332
ctx.accept();
305333
}, 2)).on('ready', mustCall(() => {

0 commit comments

Comments
 (0)