Mercurial > dropbear

comparison svr-authpubkey.c @ 1885:5d8dbb6fdab7

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Fix SSH_PUBKEYINFO, limit characters, add tests We fix a bad_bufptr() failure from a previous commit. We now limit the allowed characters to those that will definitely be safe in a shell. Some scripts/programs may use arbitrary environment variables without escaping correctly - that could be a problem in a restricted environment. The current allowed set is a-z A-Z 0-9 .,_-+@ This also adds a test for SSH_PUBKEYINFO, by default it only runs under github actions (or "act -j build").
author Matt Johnston <matt@ucc.asn.au>
date Wed, 16 Mar 2022 17:17:23 +0800
parents f54451afc046
children a7b66ea18632
comparison
equal deleted inserted replaced
1884:75d6a9faf919 1885:5d8dbb6fdab7
259 259
260 static int checkpubkey_line(buffer* line, int line_num, const char* filename, 260 static int checkpubkey_line(buffer* line, int line_num, const char* filename,
261 const char* algo, unsigned int algolen, 261 const char* algo, unsigned int algolen,
262 const unsigned char* keyblob, unsigned int keybloblen) { 262 const unsigned char* keyblob, unsigned int keybloblen) {
263 buffer *options_buf = NULL; 263 buffer *options_buf = NULL;
264 char *info_str = NULL;
264 unsigned int pos, len, infopos, infolen; 265 unsigned int pos, len, infopos, infolen;
265 int ret = DROPBEAR_FAILURE; 266 int ret = DROPBEAR_FAILURE;
266 267
267 if (line->len < MIN_AUTHKEYS_LINE || line->len > MAX_AUTHKEYS_LINE) { 268 if (line->len < MIN_AUTHKEYS_LINE || line->len > MAX_AUTHKEYS_LINE) {
268 TRACE(("checkpubkey_line: bad line length %d", line->len)) 269 TRACE(("checkpubkey_line: bad line length %d", line->len))
337 if (buf_getbyte(line) != ' ') { 338 if (buf_getbyte(line) != ' ') {
338 TRACE(("checkpubkey_line: space character expected, isn't there")) 339 TRACE(("checkpubkey_line: space character expected, isn't there"))
339 goto out; 340 goto out;
340 } 341 }
341 342
342 /* truncate the line at the space after the base64 data */ 343 /* find the length of base64 data */
343 pos = line->pos; 344 pos = line->pos;
344 for (len = 0; line->pos < line->len; len++) { 345 for (len = 0; line->pos < line->len; len++) {
345 if (buf_getbyte(line) == ' ') break; 346 if (buf_getbyte(line) == ' ') {
346 } 347 break;
347 /* findout the length of the public key info */ 348 }
349 }
350
351 /* find out the length of the public key info, stop at the first space */
348 infopos = line->pos; 352 infopos = line->pos;
349 for (infolen = 0; line->pos < line->len; infolen++) { 353 for (infolen = 0; line->pos < line->len; infolen++) {
350 if (buf_getbyte(line) == ' ') break; 354 const char c = buf_getbyte(line);
351 } 355 if (c == ' ') {
356 break;
357 }
358 /* We have an allowlist - authorized_keys lines can't be fully trusted,
359 some shell scripts may do unsafe things with env var values */
360 if (!(isalnum(c) || strchr(".,_-+@", c))) {
361 TRACE(("Not setting SSH_PUBKEYINFO, special characters"))
362 infolen = 0;
363 break;
364 }
365 }
366 if (infolen > 0) {
367 info_str = m_malloc(infolen + 1);
368 buf_setpos(line, infopos);
369 strncpy(info_str, buf_getptr(line, infolen), infolen);
370 }
371
372 /* truncate to base64 data length */
352 buf_setpos(line, pos); 373 buf_setpos(line, pos);
353 buf_setlen(line, line->pos + len); 374 buf_setlen(line, line->pos + len);
354 375
355 TRACE(("checkpubkey_line: line pos = %d len = %d", line->pos, line->len)) 376 TRACE(("checkpubkey_line: line pos = %d len = %d", line->pos, line->len))
356 377
357 ret = cmp_base64_key(keyblob, keybloblen, (const unsigned char *) algo, algolen, line, NULL); 378 ret = cmp_base64_key(keyblob, keybloblen, (const unsigned char *) algo, algolen, line, NULL);
358 379
359 /* free pubkey_info if it is filled */ 380 /* free pubkey_info if it is filled */
360 if (ses.authstate.pubkey_info) { 381 if (ses.authstate.pubkey_info) {
361 m_free(ses.authstate.pubkey_info); 382 m_free(ses.authstate.pubkey_info);
362 ses.authstate.pubkey_info = NULL; 383 }
363 } 384
364 if (ret == DROPBEAR_SUCCESS) { 385 if (ret == DROPBEAR_SUCCESS) {
365 if (options_buf) { 386 if (options_buf) {
366 ret = svr_add_pubkey_options(options_buf, line_num, filename); 387 ret = svr_add_pubkey_options(options_buf, line_num, filename);
367 } 388 }
368 /* save the (optional) public key information */ 389 /* take the (optional) public key information */
369 if (infolen) { 390 ses.authstate.pubkey_info = info_str;
370 ses.authstate.pubkey_info = m_malloc(infolen + 1); 391 info_str = NULL;
371 if (ses.authstate.pubkey_info) {
372 strncpy(ses.authstate.pubkey_info,(const char *) buf_getptr(line, infopos), infolen);
373 ses.authstate.pubkey_info[infolen]='\0';
374 }
375 }
376 } 392 }
377 393
378 out: 394 out:
379 if (options_buf) { 395 if (options_buf) {
380 buf_free(options_buf); 396 buf_free(options_buf);
397 }
398 if (info_str) {
399 m_free(info_str);
381 } 400 }
382 return ret; 401 return ret;
383 } 402 }
384 403
385 404