diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index bf785b9..d474866 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -65,6 +65,8 @@ jobs: # 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 @@ jobs: # 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 @@ jobs: # 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 @@ jobs: - 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 --git a/auth.h b/auth.h index 9279619..2063cad 100644 --- a/auth.h +++ b/auth.h @@ -125,6 +125,7 @@ struct AuthState { char *pw_passwd; #if DROPBEAR_SVR_PUBKEY_OPTIONS_BUILT struct PubKeyOptions* pubkey_options; + char *pubkey_info; #endif }; diff --git a/cli-auth.c b/cli-auth.c index 5179230..49b5ed8 100644 --- a/cli-auth.c +++ b/cli-auth.c @@ -85,31 +85,32 @@ void recv_msg_userauth_banner() { 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 --git a/cli-runopts.c b/cli-runopts.c index fdedf72..cbd15b3 100644 --- a/cli-runopts.c +++ b/cli-runopts.c @@ -62,6 +62,7 @@ static void printhelp() { "-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 @@ void cli_getopts(int argc, char ** argv) { 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 @@ void cli_getopts(int argc, char ** argv) { } 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 @@ multihop_passthrough_args() { 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 --git a/runopts.h b/runopts.h index 5a59e6f..4271d1f 100644 --- a/runopts.h +++ b/runopts.h @@ -92,7 +92,6 @@ typedef struct svr_runopts { /* whether to print the MOTD */ int domotd; #endif - int norootlogin; #ifdef HAVE_GETGROUPLIST @@ -155,6 +154,7 @@ typedef struct cli_runopts { int always_accept_key; int no_hostkey_check; int no_cmd; + int quiet; int backgrounded; int is_subsystem; #if DROPBEAR_CLI_PUBKEY_AUTH diff --git a/svr-authpubkey.c b/svr-authpubkey.c index a33cc39..e58751b 100644 --- a/svr-authpubkey.c +++ b/svr-authpubkey.c @@ -257,11 +257,15 @@ static void send_msg_userauth_pk_ok(const char* sigalgo, unsigned int sigalgolen } +/* 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 @@ static int checkpubkey_line(buffer* line, int line_num, const char* filename, 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 @@ static int checkpubkey_line(buffer* line, int line_num, const char* filename, 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 @@ static int checkpubkey(const char* keyalgo, unsigned int keyalgolen, } 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 @@ static int checkfileperm(char * filename) { 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 --git a/svr-authpubkeyoptions.c b/svr-authpubkeyoptions.c index 7ddf680..447f4b7 100644 --- a/svr-authpubkeyoptions.c +++ b/svr-authpubkeyoptions.c @@ -115,6 +115,9 @@ void svr_pubkey_options_cleanup() { } 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 --git a/svr-chansession.c b/svr-chansession.c index 02cb035..71e0e46 100644 --- a/svr-chansession.c +++ b/svr-chansession.c @@ -1030,6 +1030,9 @@ static void execchild(const void *user_data) { 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 --git a/test/test_dropbear.py b/test/test_dropbear.py index 5e7fcc6..77d1774 100644 --- a/test/test_dropbear.py +++ b/test/test_dropbear.py @@ -72,6 +72,10 @@ def own_venv_command(): 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 --git a/test/test_svrauth.py b/test/test_svrauth.py new file mode 100644 index 0000000..88f13fa --- /dev/null +++ b/test/test_svrauth.py @@ -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"