Mercurial > dropbear

changeset 1890:45e552ee4391

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
merge
author Matt Johnston <matt@ucc.asn.au>
date Tue, 22 Mar 2022 16:17:47 +0800
parents fc4c9ef61856 (current diff) a7b66ea18632 (diff)
children ab9c5467970d
files
diffstat 10 files changed, 143 insertions(+), 30 deletions(-) [+]
line wrap: on
line diff
--- 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
 
--- 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
 };
 
--- 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"))
 }
--- 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 ");
--- 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
--- 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
 
--- 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,
--- 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) {
--- 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
--- /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
[email protected]('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"
+
[email protected]('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() == ""
+
[email protected]('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"