diff options
| -rw-r--r-- | .github/workflows/build.yml | 9 | ||||
| -rw-r--r-- | svr-authpubkey.c | 47 | ||||
| -rw-r--r-- | test/test_svrauth.py | 30 |
3 files changed, 72 insertions, 14 deletions
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index bf785b9..ba207ec 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -86,6 +86,8 @@ jobs: # for fuzzing CXX: clang++ RANLIB: ${{ matrix.ranlib || 'ranlib' }} + # some pytests depend on special setup from this file. see authorized_keys below. + DBTEST_IN_ACTION: true steps: - name: deps @@ -128,7 +130,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/svr-authpubkey.c b/svr-authpubkey.c index 70e7f70..912114a 100644 --- a/svr-authpubkey.c +++ b/svr-authpubkey.c @@ -261,6 +261,7 @@ 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) { buffer *options_buf = NULL; + char *info_str = NULL; unsigned int pos, len, infopos, infolen; int ret = DROPBEAR_FAILURE; @@ -339,16 +340,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; - } - /* findout the length of the public key info */ + 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++) { - if (buf_getbyte(line) == ' ') break; + 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); @@ -359,26 +380,24 @@ static int checkpubkey_line(buffer* line, int line_num, const char* filename, /* free pubkey_info if it is filled */ if (ses.authstate.pubkey_info) { m_free(ses.authstate.pubkey_info); - ses.authstate.pubkey_info = NULL; } + if (ret == DROPBEAR_SUCCESS) { if (options_buf) { ret = svr_add_pubkey_options(options_buf, line_num, filename); } - /* save the (optional) public key information */ - if (infolen) { - ses.authstate.pubkey_info = m_malloc(infolen + 1); - if (ses.authstate.pubkey_info) { - strncpy(ses.authstate.pubkey_info,(const char *) buf_getptr(line, infopos), infolen); - ses.authstate.pubkey_info[infolen]='\0'; - } - } + /* take the (optional) public key information */ + ses.authstate.pubkey_info = info_str; + info_str = NULL; } out: if (options_buf) { buf_free(options_buf); } + if (info_str) { + m_free(info_str); + } return ret; } diff --git a/test/test_svrauth.py b/test/test_svrauth.py new file mode 100644 index 0000000..0ded6d4 --- /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_PUBKEYINFO 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_PUBKEYINFO 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_PUBKEYINFO 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" |