# HG changeset patch # User Matt Johnston # Date 1647937067 -28800 # Node ID 45e552ee439139e6d5fb5a0dd7e598a420a4d9b9 # Parent fc4c9ef618565e12cc559292c3e304951bba6480# Parent a7b66ea1863206b044ab21418d8be0cfa12e5923 merge diff -r fc4c9ef61856 -r 45e552ee4391 .github/workflows/build.yml --- a/.github/workflows/build.yml Tue Mar 22 16:17:05 2022 +0800 +++ b/.github/workflows/build.yml Tue Mar 22 16:17:47 2022 +0800 @@ -65,6 +65,8 @@ # configure_flags: --enable-fuzz --disable-harden --enable-bundled-libtom --enable-werror # ldflags: -fsanitize=address # extracflags: -fsanitize=address + # # -fsanitize=address prevents aslr, don't test it + # pytest_addopts: -k "not aslr" # fuzz: True # cc: clang @@ -74,6 +76,7 @@ # ldflags: -fsanitize=undefined # # don't fail with alignment due to https://github.com/libtom/libtomcrypt/issues/549 # extracflags: -fsanitize=undefined -fno-sanitize-recover=undefined -fsanitize-recover=alignment + # pytest_addopts: -k "not aslr" # fuzz: True # cc: clang @@ -86,6 +89,10 @@ # for fuzzing CXX: clang++ RANLIB: ${{ matrix.ranlib || 'ranlib' }} + # pytest in "make check" recognises this for extra arguments + PYTEST_ADDOPTS: ${{ matrix.pytest_addopts }} + # some pytests depend on special setup from this file. see authorized_keys below. + DBTEST_IN_ACTION: true steps: - name: deps @@ -128,7 +135,14 @@ - name: keys run: | mkdir -p ~/.ssh + # remove old files so we can rerun in-place with "act -r" during test development + rm -vf ~/.ssh/id_dropbear* ~/inst/bin/dropbearkey -t ecdsa -f ~/.ssh/id_dropbear | grep ^ecdsa > ~/.ssh/authorized_keys + + # to test setting SSH_PUBKEYINFO, replace the trailing comment + ~/inst/bin/dropbearkey -t ecdsa -f ~/.ssh/id_dropbear_key2 | grep ^ecdsa | sed 's/[^ ]*$/key2 extra/' >> ~/.ssh/authorized_keys + ~/inst/bin/dropbearkey -t ecdsa -f ~/.ssh/id_dropbear_key3 | grep ^ecdsa | sed 's/[^ ]*$/key3%char/' >> ~/.ssh/authorized_keys + ~/inst/bin/dropbearkey -t ecdsa -f ~/.ssh/id_dropbear_key4 | grep ^ecdsa | sed 's/[^ ]*$/key4,char/' >> ~/.ssh/authorized_keys chmod 700 ~ ~/.ssh ~/.ssh/authorized_keys ls -ld ~ ~/.ssh ~/.ssh/authorized_keys diff -r fc4c9ef61856 -r 45e552ee4391 auth.h --- a/auth.h Tue Mar 22 16:17:05 2022 +0800 +++ b/auth.h Tue Mar 22 16:17:47 2022 +0800 @@ -125,6 +125,7 @@ char *pw_passwd; #if DROPBEAR_SVR_PUBKEY_OPTIONS_BUILT struct PubKeyOptions* pubkey_options; + char *pubkey_info; #endif }; diff -r fc4c9ef61856 -r 45e552ee4391 cli-auth.c --- a/cli-auth.c Tue Mar 22 16:17:05 2022 +0800 +++ b/cli-auth.c Tue Mar 22 16:17:47 2022 +0800 @@ -85,31 +85,32 @@ banner = buf_getstring(ses.payload, &bannerlen); buf_eatstring(ses.payload); /* The language string */ - if (bannerlen > MAX_BANNER_SIZE) { - TRACE(("recv_msg_userauth_banner: bannerlen too long: %d", bannerlen)) - truncated = 1; - } else { - cleantext(banner); + if (cli_opts.quiet == 0) { + if (bannerlen > MAX_BANNER_SIZE) { + TRACE(("recv_msg_userauth_banner: bannerlen too long: %d", bannerlen)) + truncated = 1; + } else { + cleantext(banner); - /* Limit to 24 lines */ - linecount = 1; - for (i = 0; i < bannerlen; i++) { - if (banner[i] == '\n') { - if (linecount >= MAX_BANNER_LINES) { - banner[i] = '\0'; - truncated = 1; - break; + /* Limit to 24 lines */ + linecount = 1; + for (i = 0; i < bannerlen; i++) { + if (banner[i] == '\n') { + if (linecount >= MAX_BANNER_LINES) { + banner[i] = '\0'; + truncated = 1; + break; + } + linecount++; } - linecount++; } + fprintf(stderr, "%s\n", banner); } - fprintf(stderr, "%s\n", banner); + + if (truncated) { + fprintf(stderr, "[Banner from the server is too long]\n"); + } } - - if (truncated) { - fprintf(stderr, "[Banner from the server is too long]\n"); - } - m_free(banner); TRACE(("leave recv_msg_userauth_banner")) } diff -r fc4c9ef61856 -r 45e552ee4391 cli-runopts.c --- a/cli-runopts.c Tue Mar 22 16:17:05 2022 +0800 +++ b/cli-runopts.c Tue Mar 22 16:17:47 2022 +0800 @@ -62,6 +62,7 @@ "-T Don't allocate a pty\n" "-N Don't run a remote command\n" "-f Run in background after auth\n" + "-q quiet, don't show remote banner\n" "-y Always accept remote host key if unknown\n" "-y -y Don't perform any remote host key checking (caution)\n" "-s Request a subsystem (use by external sftp)\n" @@ -141,6 +142,7 @@ cli_opts.username = NULL; cli_opts.cmd = NULL; cli_opts.no_cmd = 0; + cli_opts.quiet = 0; cli_opts.backgrounded = 0; cli_opts.wantpty = 9; /* 9 means "it hasn't been touched", gets set later */ cli_opts.always_accept_key = 0; @@ -214,6 +216,9 @@ } cli_opts.always_accept_key = 1; break; + case 'q': /* quiet */ + cli_opts.quiet = 1; + break; case 'p': /* remoteport */ next = (char**)&cli_opts.remoteport; break; @@ -540,6 +545,12 @@ ret = m_malloc(len); total = 0; + if (cli_opts.quiet) + { + int written = snprintf(ret+total, len-total, "-q "); + total += written; + } + if (cli_opts.no_hostkey_check) { int written = snprintf(ret+total, len-total, "-y -y "); diff -r fc4c9ef61856 -r 45e552ee4391 runopts.h --- a/runopts.h Tue Mar 22 16:17:05 2022 +0800 +++ b/runopts.h Tue Mar 22 16:17:47 2022 +0800 @@ -92,7 +92,6 @@ /* whether to print the MOTD */ int domotd; #endif - int norootlogin; #ifdef HAVE_GETGROUPLIST @@ -155,6 +154,7 @@ int always_accept_key; int no_hostkey_check; int no_cmd; + int quiet; int backgrounded; int is_subsystem; #if DROPBEAR_CLI_PUBKEY_AUTH diff -r fc4c9ef61856 -r 45e552ee4391 svr-authpubkey.c --- a/svr-authpubkey.c Tue Mar 22 16:17:05 2022 +0800 +++ b/svr-authpubkey.c Tue Mar 22 16:17:47 2022 +0800 @@ -257,11 +257,15 @@ } +/* Content for SSH_PUBKEYINFO is optionally returned malloced in ret_info (will be + freed if already set */ static int checkpubkey_line(buffer* line, int line_num, const char* filename, const char* algo, unsigned int algolen, - const unsigned char* keyblob, unsigned int keybloblen) { + const unsigned char* keyblob, unsigned int keybloblen, + char ** ret_info) { buffer *options_buf = NULL; - unsigned int pos, len; + char *info_str = NULL; + unsigned int pos, len, infopos, infolen; int ret = DROPBEAR_FAILURE; if (line->len < MIN_AUTHKEYS_LINE || line->len > MAX_AUTHKEYS_LINE) { @@ -339,11 +343,36 @@ goto out; } - /* truncate the line at the space after the base64 data */ + /* find the length of base64 data */ pos = line->pos; for (len = 0; line->pos < line->len; len++) { - if (buf_getbyte(line) == ' ') break; - } + if (buf_getbyte(line) == ' ') { + break; + } + } + + /* find out the length of the public key info, stop at the first space */ + infopos = line->pos; + for (infolen = 0; line->pos < line->len; infolen++) { + const char c = buf_getbyte(line); + if (c == ' ') { + break; + } + /* We have an allowlist - authorized_keys lines can't be fully trusted, + some shell scripts may do unsafe things with env var values */ + if (!(isalnum(c) || strchr(".,_-+@", c))) { + TRACE(("Not setting SSH_PUBKEYINFO, special characters")) + infolen = 0; + break; + } + } + if (infolen > 0) { + info_str = m_malloc(infolen + 1); + buf_setpos(line, infopos); + strncpy(info_str, buf_getptr(line, infolen), infolen); + } + + /* truncate to base64 data length */ buf_setpos(line, pos); buf_setlen(line, line->pos + len); @@ -351,14 +380,30 @@ ret = cmp_base64_key(keyblob, keybloblen, (const unsigned char *) algo, algolen, line, NULL); - if (ret == DROPBEAR_SUCCESS && options_buf) { - ret = svr_add_pubkey_options(options_buf, line_num, filename); + /* free pubkey_info if it is filled */ + if (ret_info && *ret_info) { + m_free(*ret_info); + *ret_info = NULL; + } + + if (ret == DROPBEAR_SUCCESS) { + if (options_buf) { + ret = svr_add_pubkey_options(options_buf, line_num, filename); + } + if (ret_info) { + /* take the (optional) public key information */ + *ret_info = info_str; + info_str = NULL; + } } out: if (options_buf) { buf_free(options_buf); } + if (info_str) { + m_free(info_str); + } return ret; } @@ -431,7 +476,8 @@ } line_num++; - ret = checkpubkey_line(line, line_num, filename, keyalgo, keyalgolen, keyblob, keybloblen); + ret = checkpubkey_line(line, line_num, filename, keyalgo, keyalgolen, + keyblob, keybloblen, &ses.authstate.pubkey_info); if (ret == DROPBEAR_SUCCESS) { break; } @@ -548,7 +594,7 @@ int fuzz_checkpubkey_line(buffer* line, int line_num, char* filename, const char* algo, unsigned int algolen, const unsigned char* keyblob, unsigned int keybloblen) { - return checkpubkey_line(line, line_num, filename, algo, algolen, keyblob, keybloblen); + return checkpubkey_line(line, line_num, filename, algo, algolen, keyblob, keybloblen, NULL); } #endif diff -r fc4c9ef61856 -r 45e552ee4391 svr-authpubkeyoptions.c --- a/svr-authpubkeyoptions.c Tue Mar 22 16:17:05 2022 +0800 +++ b/svr-authpubkeyoptions.c Tue Mar 22 16:17:47 2022 +0800 @@ -115,6 +115,9 @@ } m_free(ses.authstate.pubkey_options); } + if (ses.authstate.pubkey_info) { + m_free(ses.authstate.pubkey_info); + } } /* helper for svr_add_pubkey_options. returns DROPBEAR_SUCCESS if the option is matched, diff -r fc4c9ef61856 -r 45e552ee4391 svr-chansession.c --- a/svr-chansession.c Tue Mar 22 16:17:05 2022 +0800 +++ b/svr-chansession.c Tue Mar 22 16:17:47 2022 +0800 @@ -1030,6 +1030,9 @@ if (chansess->original_command) { addnewvar("SSH_ORIGINAL_COMMAND", chansess->original_command); } + if (ses.authstate.pubkey_info != NULL) { + addnewvar("SSH_PUBKEYINFO", ses.authstate.pubkey_info); + } /* change directory */ if (chdir(ses.authstate.pw_dir) < 0) { diff -r fc4c9ef61856 -r 45e552ee4391 test/test_dropbear.py --- a/test/test_dropbear.py Tue Mar 22 16:17:05 2022 +0800 +++ b/test/test_dropbear.py Tue Mar 22 16:17:47 2022 +0800 @@ -72,6 +72,10 @@ return f"source {venv}/bin/activate" class HandleTcp(socketserver.ThreadingMixIn, socketserver.TCPServer): + + # override TCPServer's default, avoids TIME_WAIT + allow_reuse_addr = True + """ Listens for a single incoming request, sends a response if given, and returns the inbound data. Reponse can be a queue object, in which case each item in the queue will diff -r fc4c9ef61856 -r 45e552ee4391 test/test_svrauth.py --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/test_svrauth.py Tue Mar 22 16:17:47 2022 +0800 @@ -0,0 +1,30 @@ +from test_dropbear import * +import signal +import queue +import socket +import os +from pathlib import Path + +# Tests for server side authentication + +# Requires keyfile and authorized_keys set up in github action build.yml +@pytest.mark.skipif('DBTEST_IN_ACTION' not in os.environ, reason="DBTEST_IN_ACTION not set") +def test_pubkeyinfo(request, dropbear): + kf = str(Path.home() / ".ssh/id_dropbear_key2") + r = dbclient(request, "-i", kf, "echo -n $SSH_PUBKEYINFO", capture_output=True) + # stop at first space + assert r.stdout.decode() == "key2" + +@pytest.mark.skipif('DBTEST_IN_ACTION' not in os.environ, reason="DBTEST_IN_ACTION not set") +def test_pubkeyinfo_special(request, dropbear): + kf = str(Path.home() / ".ssh/id_dropbear_key3") + r = dbclient(request, "-i", kf, "echo -n $SSH_PUBKEYINFO", capture_output=True) + # comment contains special characters so the SSH_PUBKEYINFO should not be set + assert r.stdout.decode() == "" + +@pytest.mark.skipif('DBTEST_IN_ACTION' not in os.environ, reason="DBTEST_IN_ACTION not set") +def test_pubkeyinfo_okchar(request, dropbear): + kf = str(Path.home() / ".ssh/id_dropbear_key4") + r = dbclient(request, "-i", kf, "echo -n $SSH_PUBKEYINFO", capture_output=True) + # comment contains special characters so the SSH_PUBKEYINFO should not be set + assert r.stdout.decode() == "key4,char"