From _j at outlook.com Thu Oct 1 18:05:20 2015 From: _j at outlook.com (Jan Burg) Date: Thu, 1 Oct 2015 04:05:20 -0600 Subject: Dropbear can't find USER name in Android Message-ID: Hi, I'm using dropbear to make a ssh server on Android. I can generate keys as a root user and connect to the dropbear server as a root user. When I generate keys as a non-root user and start the server, I can't connect using the non-root user's $USER name. I get an "unknown user" error. I took a closer look at the public key extracted from dropbearkey -y -f ~/.ssh/id_rsa, and it's missing the username at the bottom. This is what I get from the command above; Generating key, this may take a while... Public key portion is: ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQCmSWZpkTFG0ledIyk3TmeFGK6UwrjvzGyeGKVpt92O9XfBjkWzvrIxucK9sbxfCRCMT0G0RhGsfpRCv80kLjzeL52tW5x3jMWZNAbidVgOZNtQLrogz2lmY6bT3FGN6kh3+1vE1oqa0N59qc4XJC92jdTPkpO9nJRXM9LrFidcyK7tTkGRk5iiFG074nlRn0tysXvs6Zg/8+GhZSz7meI4PMuKbkK2or6wkGJtMDQbJgDn72N2LKzUy7SVRlnFjIxECOoT/30LA1f3pOatztSPaH/XAhEJioyUv6DGbjomlJXrutGcpci16CCtcyP5I7RU916csyASxyuZbnoERc8T @localhost Fingerprint: md5 77:db:7b:8d:d1:b8:d9:47:dc:5a:fa:0a:0b:ed:3b:02 It just says "@localhost" instead of "u0_a32 at localhost". When I generate keys as root, it determines the "root" username but seems to have problems with usernames. This isn't the first time I've seen this. Pretty much every binary I've compiled for Android has a problem getting the username. Most programs such as busybox can find the user id number ($EUID) though. The only binary I've seen successfully return the username is toolbox. It has a basic "id" applet that prints the username and group names. I'd like to figure out what the issue is, maybe determine some kind of patch or a workaround. Here is the source code for Android's toolbox: https://android.googlesource.com/platform/system/core/+/android-5.1.1_r18/toolbox/ Jan -------------- next part -------------- An HTML attachment was scrubbed... URL: http://lists.ucc.gu.uwa.edu.au/pipermail/dropbear/attachments/20151001/f88aedd9/attachment.htm From vapier at gentoo.org Sat Oct 10 07:23:32 2015 From: vapier at gentoo.org (Mike Frysinger) Date: Fri, 9 Oct 2015 19:23:32 -0400 Subject: [patch] fix build when ENABLE_CLI_INTERACT_AUTH is disabled Message-ID: <20151009232332.GA4410@vapier.lan> The session.h defines clientsession.cipher_none_after_auth only when ENABLE_CLI_INTERACT_AUTH is defined, but cli-session.c will always try to set that member. export cipher_none_after_auth all the time. -mike cli-session.c: In function 'cli_session_init': cli-session.c:171:9: error: 'struct clientsession' has no member named 'cipher_none_after_auth' cli_ses.cipher_none_after_auth = 0; --- a/session.h Tue Sep 29 22:19:11 2015 +0800 +++ b/session.h Fri Oct 09 19:21:35 2015 -0400 @@ -293,10 +293,9 @@ struct clientsession { int interact_request_received; /* flag whether we've received an info request from the server for interactive auth.*/ - +#endif int cipher_none_after_auth; /* Set to 1 if the user requested "none" auth */ -#endif sign_key *lastprivkey; int retval; /* What the command exit status was - we emulate it */ -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: Digital signature Url : http://lists.ucc.gu.uwa.edu.au/pipermail/dropbear/attachments/20151009/20dc6f9b/attachment.sig From vapier at gentoo.org Sat Oct 10 07:25:22 2015 From: vapier at gentoo.org (Mike Frysinger) Date: Fri, 9 Oct 2015 19:25:22 -0400 Subject: [patch] avoid getpass when not used Message-ID: <20151009232522.GB4410@vapier.lan> some systems (like android's bionic) do not provide getpass. you can disable ENABLE_CLI_PASSWORD_AUTH & ENABLE_CLI_INTERACT_AUTH to avoid its use (and rely on pubkey auth), but the link still fails because the support file calls getpass. do not define this func if both of those auth methods are not used. -mike cli-auth.o:cli-auth.c:function getpass_or_cancel: error: undefined reference to 'getpass' collect2: error: ld returned 1 exit status --- a/cli-auth.c Tue Sep 29 22:19:11 2015 +0800 +++ b/cli-auth.c Fri Oct 09 19:21:35 2015 -0400 @@ -324,6 +324,7 @@ int cli_auth_try() { return DROPBEAR_FAILURE; } +#if defined(ENABLE_CLI_PASSWORD_AUTH) || defined(ENABLE_CLI_INTERACT_AUTH) /* A helper for getpass() that exits if the user cancels. The returned * password is statically allocated by getpass() */ char* getpass_or_cancel(char* prompt) @@ -347,3 +348,4 @@ char* getpass_or_cancel(char* prompt) } return password; } +#endif -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: Digital signature Url : http://lists.ucc.gu.uwa.edu.au/pipermail/dropbear/attachments/20151009/d7f537da/attachment.sig From vapier at gentoo.org Sat Oct 10 07:28:05 2015 From: vapier at gentoo.org (Mike Frysinger) Date: Fri, 9 Oct 2015 19:28:05 -0400 Subject: [patch] fix default build when crypt() is unavailable Message-ID: <20151009232805.GC4410@vapier.lan> if the system doesn't support crypt.h/crypt, then ENABLE_SVR_PASSWORD_AUTH cannot work. rather than default this to on all the time, do so only when support for the header is found. -mike svr-authpasswd.o:svr-authpasswd.c:function svr_auth_password: error: undefined reference to 'crypt' collect2: error: ld returned 1 exit status --- a/options.h Tue Sep 29 22:19:11 2015 +0800 +++ b/options.h Fri Oct 09 19:21:35 2015 -0400 @@ -206,7 +206,10 @@ If you test it please contact the Dropbe * PAM challenge/response. * You can't enable both PASSWORD and PAM. */ +/* This requires crypt.h & crypt. */ +#ifdef HAVE_CRYPT_H #define ENABLE_SVR_PASSWORD_AUTH +#endif /* PAM requires ./configure --enable-pam */ /*#define ENABLE_SVR_PAM_AUTH */ #define ENABLE_SVR_PUBKEY_AUTH -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: Digital signature Url : http://lists.ucc.gu.uwa.edu.au/pipermail/dropbear/attachments/20151009/134c723a/attachment.sig From vapier at gentoo.org Sat Oct 10 07:30:25 2015 From: vapier at gentoo.org (Mike Frysinger) Date: Fri, 9 Oct 2015 19:30:25 -0400 Subject: [patch] fix default build when getpass() is unavailable Message-ID: <20151009233025.GD4410@vapier.lan> if the system doesn't support getpass, we still default on the options that require it which causes a build failure. instead, only default enable these when getpass is available. -mike cli-auth.o:cli-auth.c:function getpass_or_cancel: error: undefined reference to 'getpass' collect2: error: ld returned 1 exit status --- a/configure.ac Tue Sep 29 22:19:11 2015 +0800 +++ b/configure.ac Fri Oct 09 19:28:56 2015 -0400 @@ -632,7 +632,7 @@ fi AC_PROG_GCC_TRADITIONAL AC_FUNC_MEMCMP AC_FUNC_SELECT_ARGTYPES -AC_CHECK_FUNCS([dup2 getspnam getusershell memset putenv select socket strdup clearenv strlcpy strlcat daemon basename _getpty getaddrinfo freeaddrinfo getnameinfo fork writev]) +AC_CHECK_FUNCS([dup2 getpass getspnam getusershell memset putenv select socket strdup clearenv strlcpy strlcat daemon basename _getpty getaddrinfo freeaddrinfo getnameinfo fork writev]) AC_SEARCH_LIBS(basename, gen, AC_DEFINE(HAVE_BASENAME)) --- a/options.h Tue Sep 29 22:19:11 2015 +0800 +++ b/options.h Fri Oct 09 19:28:56 2015 -0400 @@ -217,9 +220,12 @@ If you test it please contact the Dropbe #define ENABLE_SVR_PUBKEY_OPTIONS #endif +/* This requires getpass. */ +#ifdef HAVE_FUNC_GETPASS #define ENABLE_CLI_PASSWORD_AUTH +#define ENABLE_CLI_INTERACT_AUTH +#endif #define ENABLE_CLI_PUBKEY_AUTH -#define ENABLE_CLI_INTERACT_AUTH /* A default argument for dbclient -i . Homedir is prepended unless path begins with / */ -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: Digital signature Url : http://lists.ucc.gu.uwa.edu.au/pipermail/dropbear/attachments/20151009/3967be5d/attachment.sig From guilhem at fripost.org Wed Oct 14 02:42:18 2015 From: guilhem at fripost.org (Guilhem Moulin) Date: Tue, 13 Oct 2015 20:42:18 +0200 Subject: /etc/motd is also printed on non-login shells if a TTY has been requested Message-ID: <20151013184218.GA2456@localhost.localdomain> Hi, As of 2015.68, dropbear(8) says ?By default the file /etc/motd will be printed for any login shell (unless disabled at compile-time). This can also be disabled per-user by creating a file ~/.hushlogin .? But in fact /etc/motd is printed whenever a TTY has been requested, even when a command is executed on a non-login shell (explicitly or via an authorized_key(5) restriction). For instance, using the OpenSSH client, ssh -t localhost -p 2222 true prints the Message Of The Day. This is probably not the intended behavior, as it messes up with the command's standard output hence make it hard to use redirections. Cheers, -- Guilhem. -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available Url : http://lists.ucc.gu.uwa.edu.au/pipermail/dropbear/attachments/20151013/a9f09cb7/attachment-0001.sig From guilhem at fripost.org Wed Oct 14 03:13:31 2015 From: guilhem at fripost.org (Guilhem Moulin) Date: Tue, 13 Oct 2015 21:13:31 +0200 Subject: svr_getopts should either support bundling or fail if bundling is used Message-ID: <20151013191331.GA10317@localhost.localdomain> Hi, It's fine not to implement bundling in dropbear's option parsing function (svr-runopts.c's svr_getopts), but it should at least croak if argv[i][2] != '\0'. For instance dropbear -rdropbear.key -p127.0.0.1:2222 -sjk should either fail, or be parsed as dropbear -r dropbear.key -p 127.0.0.1:2222 -s -j -k if bundling is allowed. This might have security implications, as the current parsing mechanism might make a user think that passing ?-sjk? disables port forwarding, which is not the case (the trailing ?jk? is ignored). Cheers, -- Guilhem. -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available Url : http://lists.ucc.gu.uwa.edu.au/pipermail/dropbear/attachments/20151013/06af2fd0/attachment.sig From guilhem at fripost.org Thu Oct 15 00:42:23 2015 From: guilhem at fripost.org (Guilhem Moulin) Date: Wed, 14 Oct 2015 18:42:23 +0200 Subject: [PATCH] Don't display the MOTD when an explicit command is run. In-Reply-To: <20151013191331.GA10317@localhost.localdomain> References: <20151013191331.GA10317@localhost.localdomain> Message-ID: <1444840943-13604-1-git-send-email-guilhem@fripost.org> (possibly via authorized_keys(5) restrictions), even when a pseudo-terminal has been allocated for the session. In other words, only display the MOTD when the server starts the user's default shell. --- svr-chansession.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/svr-chansession.c b/svr-chansession.c index e44299e..bfaf7f6 100644 --- a/svr-chansession.c +++ b/svr-chansession.c @@ -814,7 +814,7 @@ static int ptycommand(struct Channel *channel, struct ChanSess *chansess) { login_free_entry(li); #ifdef DO_MOTD - if (svr_opts.domotd) { + if (svr_opts.domotd && !chansess->cmd) { /* don't show the motd if ~/.hushlogin exists */ /* 12 == strlen("/.hushlogin\0") */ -- 2.6.1 From matt at ucc.asn.au Wed Oct 21 22:11:43 2015 From: matt at ucc.asn.au (Matt Johnston) Date: Wed, 21 Oct 2015 22:11:43 +0800 Subject: svr_getopts should either support bundling or fail if bundling is used In-Reply-To: <20151013191331.GA10317@localhost.localdomain> References: <20151013191331.GA10317@localhost.localdomain> Message-ID: <068C216B-5328-4AB6-86B5-DB4FC2EF5472@ucc.asn.au> Hi Guilhem, Thanks for pointing that out, I?ve made -sjk fail rather than be dropped silently. I?ve applied the other patch to avoid MOTD when there?s a command. Thanks, Matt > On Wed 14/10/2015, at 3:13 am, Guilhem Moulin wrote: > > Hi, > > It's fine not to implement bundling in dropbear's option parsing > function (svr-runopts.c's svr_getopts), but it should at least croak if > argv[i][2] != '\0'. For instance > > dropbear -rdropbear.key -p127.0.0.1:2222 -sjk > > should either fail, or be parsed as > > dropbear -r dropbear.key -p 127.0.0.1:2222 -s -j -k > > if bundling is allowed. > > > This might have security implications, as the current parsing mechanism > might make a user think that passing ?-sjk? disables port forwarding, > which is not the case (the trailing ?jk? is ignored). > > Cheers, > -- > Guilhem. From matt at ucc.asn.au Wed Oct 21 23:10:08 2015 From: matt at ucc.asn.au (Matt Johnston) Date: Wed, 21 Oct 2015 23:10:08 +0800 Subject: [patch] fix default build when getpass() is unavailable In-Reply-To: <20151009233025.GD4410@vapier.lan> References: <20151009233025.GD4410@vapier.lan> Message-ID: I?ve merged these 4 patches, thanks. Couple of changes HAVE_GETPASS not HAVE_FUNC_GETPASS, and I?m testing HAVE_CRYPT since OSX (and I guess other BSDs) don?t have crypt.h Cheers, Matt > On Sat 10/10/2015, at 7:30 am, Mike Frysinger wrote: > > if the system doesn't support getpass, we still default on the options > that require it which causes a build failure. instead, only default > enable these when getpass is available. > -mike > > cli-auth.o:cli-auth.c:function getpass_or_cancel: error: undefined reference to 'getpass' > collect2: error: ld returned 1 exit status > > --- a/configure.ac Tue Sep 29 22:19:11 2015 +0800 > +++ b/configure.ac Fri Oct 09 19:28:56 2015 -0400 > @@ -632,7 +632,7 @@ fi > AC_PROG_GCC_TRADITIONAL > AC_FUNC_MEMCMP > AC_FUNC_SELECT_ARGTYPES > -AC_CHECK_FUNCS([dup2 getspnam getusershell memset putenv select socket strdup clearenv strlcpy strlcat daemon basename _getpty getaddrinfo freeaddrinfo getnameinfo fork writev]) > +AC_CHECK_FUNCS([dup2 getpass getspnam getusershell memset putenv select socket strdup clearenv strlcpy strlcat daemon basename _getpty getaddrinfo freeaddrinfo getnameinfo fork writev]) > > AC_SEARCH_LIBS(basename, gen, AC_DEFINE(HAVE_BASENAME)) > > --- a/options.h Tue Sep 29 22:19:11 2015 +0800 > +++ b/options.h Fri Oct 09 19:28:56 2015 -0400 > @@ -217,9 +220,12 @@ If you test it please contact the Dropbe > #define ENABLE_SVR_PUBKEY_OPTIONS > #endif > > +/* This requires getpass. */ > +#ifdef HAVE_FUNC_GETPASS > #define ENABLE_CLI_PASSWORD_AUTH > +#define ENABLE_CLI_INTERACT_AUTH > +#endif > #define ENABLE_CLI_PUBKEY_AUTH > -#define ENABLE_CLI_INTERACT_AUTH > > /* A default argument for dbclient -i . > Homedir is prepended unless path begins with / */ From guilhem at fripost.org Thu Oct 22 01:21:56 2015 From: guilhem at fripost.org (Guilhem Moulin) Date: Wed, 21 Oct 2015 19:21:56 +0200 Subject: svr_getopts should either support bundling or fail if bundling is used In-Reply-To: <068C216B-5328-4AB6-86B5-DB4FC2EF5472@ucc.asn.au> References: <20151013191331.GA10317@localhost.localdomain> <068C216B-5328-4AB6-86B5-DB4FC2EF5472@ucc.asn.au> Message-ID: <20151021172155.GA5874@localhost.localdomain> Hi Matt, On Wed, 21 Oct 2015 at 22:11:43 +0800, Matt Johnston wrote: > Thanks for pointing that out, I?ve made -sjk fail rather than be > dropped silently. Thanks. However on second thought, the downside of this solution is that it might render remote systems unreachable after upgrade (at least for the users not reading changelogs or distrib NEWS files). Worse, it might not be noticed before a reboot, since upgrading typically doesn't kill existing SSH connections. (I've set ?DROPBEAR_OPTIONS=-sjk? myself, until last week where I noticed that only the first flag was considered.) Would you consider a patch to enable bundling instead? If yes I'll try to hack something up in the next couple of days. By the way, out of curiosity, is there a reason why you're not using getopt()? It's POSIX after all, and you're already using it for scp. Cheers, -- Guilhem. -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available Url : http://lists.ucc.gu.uwa.edu.au/pipermail/dropbear/attachments/20151021/5de9a600/attachment.sig From matt at ucc.asn.au Thu Oct 22 08:02:01 2015 From: matt at ucc.asn.au (Matt Johnston) Date: Thu, 22 Oct 2015 08:02:01 +0800 Subject: svr_getopts should either support bundling or fail if bundling is used In-Reply-To: <20151021172155.GA5874@localhost.localdomain> References: <20151013191331.GA10317@localhost.localdomain> <068C216B-5328-4AB6-86B5-DB4FC2EF5472@ucc.asn.au> <20151021172155.GA5874@localhost.localdomain> Message-ID: <463831C1-9E9E-48F8-BACA-15DC734AA300@ucc.asn.au> On Thu 22/10/2015, at 1:21 am, Guilhem Moulin wrote: > On Wed, 21 Oct 2015 at 22:11:43 +0800, Matt Johnston wrote: >> Thanks for pointing that out, I?ve made -sjk fail rather than be >> dropped silently. > > Thanks. However on second thought, the downside of this solution is > that it might render remote systems unreachable after upgrade (at least > for the users not reading changelogs or distrib NEWS files). Worse, it > might not be noticed before a reboot, since upgrading typically doesn't > kill existing SSH connections. Even enabling bundling could result in dropbear failing to start if there were trailing options that weren't valid. Perhaps I should just make the failure a warning instead, it'll be visible on "service dropbear restart"? 'Ignored extra trailing "jk" of "-sjk"' > By the way, out of > curiosity, is there a reason why you're not using getopt()? It's POSIX > after all, and you're already using it for scp. I think I looked into it a long time ago and it resulted in a larger static binary size. It might be worth revisiting though. Backward compatibility would still be an issue. Cheers, Matt From guilhem at fripost.org Thu Oct 22 08:24:12 2015 From: guilhem at fripost.org (Guilhem Moulin) Date: Thu, 22 Oct 2015 02:24:12 +0200 Subject: svr_getopts should either support bundling or fail if bundling is used In-Reply-To: <463831C1-9E9E-48F8-BACA-15DC734AA300@ucc.asn.au> References: <20151013191331.GA10317@localhost.localdomain> <068C216B-5328-4AB6-86B5-DB4FC2EF5472@ucc.asn.au> <20151021172155.GA5874@localhost.localdomain> <463831C1-9E9E-48F8-BACA-15DC734AA300@ucc.asn.au> Message-ID: <20151022002412.GA14007@localhost.localdomain> On Thu, 22 Oct 2015 at 08:02:01 +0800, Matt Johnston wrote: > On Thu 22/10/2015, at 1:21 am, Guilhem Moulin wrote: >> Thanks. However on second thought, the downside of this solution is >> that it might render remote systems unreachable after upgrade (at least >> for the users not reading changelogs or distrib NEWS files). Worse, it >> might not be noticed before a reboot, since upgrading typically doesn't >> kill existing SSH connections. > > Even enabling bundling could result in dropbear failing to start if > there were trailing options that weren't valid. Fair enough, but ? assuming that distributions didn't change compiling options, and that users didn't rely on dropbear ignoring trailing flags ? the failure would have to come from a typo, right? Then I think it's fine to croak, from a distro perspective at least. > Perhaps I should just make the failure a warning instead, it'll be > visible on "service dropbear restart"? 'Ignored extra trailing "jk" of > "-sjk"' Combined with a changelog entry (and a NEWS entry on the distro side), that would indeed allow smooth upgrade. However note that you can't rely on the warning being visible on the error output since it depends on the init system. But AFAIK they all log in the system log when available (hence not in the initrd). >> By the way, out of curiosity, is there a reason why you're not using >> getopt()? It's POSIX after all, and you're already using it for scp. > > I think I looked into it a long time ago and it resulted in a larger > static binary size. It might be worth revisiting though. Backward > compatibility would still be an issue. Alright, I'll give it a try when time allows. I don't understand your last sentence though: backward compatibility for what? -- Guilhem. -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available Url : http://lists.ucc.gu.uwa.edu.au/pipermail/dropbear/attachments/20151022/fe90dafe/attachment-0001.sig From guilhem at fripost.org Sun Oct 25 02:32:09 2015 From: guilhem at fripost.org (Guilhem Moulin) Date: Sat, 24 Oct 2015 20:32:09 +0200 Subject: [PATCH] Fix minor manpage formatting issues. Message-ID: <1445711529-9216-1-git-send-email-guilhem@fripost.org> --- dbclient.1 | 11 +++++------ dropbear.8 | 9 ++++----- dropbearconvert.1 | 10 +++++----- dropbearkey.1 | 7 ++----- 4 files changed, 16 insertions(+), 21 deletions(-) diff --git a/dbclient.1 b/dbclient.1 index c33f955..259c786 100644 --- a/dbclient.1 +++ b/dbclient.1 @@ -3,7 +3,7 @@ dbclient \- lightweight SSH client .SH SYNOPSIS .B dbclient -[flag arguments] [\-p +[\fIflag arguments\fR] [\-p .I port\fR] [\-i .I id\fR] [\-L .I l\fR:\fIh\fR:\fIr\fR] [\-R @@ -13,9 +13,8 @@ dbclient \- lightweight SSH client .RI [ command ] .B dbclient -[ -.I args ] -.I [user1]@host1[^port1],[user2]@host2[^port2],... +[\fIargs\fR] +[\fIuser1\fR]@\fIhost1\fR[^\fIport1\fR],[\fIuser2\fR]@\fIhost2\fR[^\fIport2\fR],... .SH DESCRIPTION .B dbclient @@ -35,7 +34,7 @@ Read the identity key from file (multiple allowed). This file is created with dropbearkey(1) or converted from OpenSSH with dropbearconvert(1). The default path ~/.ssh/id_dropbear is used .TP -.B \-L [\fIlistenaddress\fR]:\fIlistenport\fR:\fIhost\fR:\fIport\fR +.B \-L\fR [\fIlistenaddress\fR]:\fIlistenport\fR:\fIhost\fR:\fIport\fR Local port forwarding. Forward the port .I listenport @@ -44,7 +43,7 @@ on the local host through the SSH connection to port on the host .IR host . .TP -.B \-R [\fIlistenaddress\fR]:\fIlistenport\fR:\fIhost\fR:\fIport\fR +.B \-R\fR [\fIlistenaddress\fR]:\fIlistenport\fR:\fIhost\fR:\fIport\fR Remote port forwarding. Forward the port .I listenport diff --git a/dropbear.8 b/dropbear.8 index 501cecf..71200d9 100644 --- a/dropbear.8 +++ b/dropbear.8 @@ -3,11 +3,10 @@ dropbear \- lightweight SSH server .SH SYNOPSIS .B dropbear -[flag arguments] [\-b +[\fIflag arguments\fR] [\-b .I banner\fR] [\-r -.I hostkeyfile\fR] [\-p -.IR [address:]port ] +.I hostkeyfile\fR] [\-p [\fIaddress\fR:]\fIport\fR] .SH DESCRIPTION .B dropbear is a small SSH server @@ -54,7 +53,7 @@ Disable local port forwarding. .B \-k Disable remote port forwarding. .TP -.B \-p \fI[address:]port +.B \-p\fR [\fIaddress\fR:]\fIport Listen on specified .I address and TCP @@ -128,7 +127,7 @@ Disable PTY allocation. Note that a user can still obtain most of the same functionality with other means even if no-pty is set. .TP -.B command="\fIforced_command\fR" +.B command=\fR"\fIforced_command\fR" Disregard the command provided by the user and always run \fIforced_command\fR. The authorized_keys file and its containing ~/.ssh directory must only be diff --git a/dropbearconvert.1 b/dropbearconvert.1 index b2f34ef..dd97ad4 100644 --- a/dropbearconvert.1 +++ b/dropbearconvert.1 @@ -21,24 +21,24 @@ from a private key by using .P Encrypted private keys are not supported, use ssh-keygen(1) to decrypt them first. -.SH OPTIONS +.SH ARGUMENTS .TP -.B input type +.I input_type Either .I dropbear or .I openssh .TP -.B output type +.I output_type Either .I dropbear or .I openssh .TP -.B input file +.I input_file An existing Dropbear or OpenSSH private key file .TP -.B output file +.I output_file The path to write the converted private key file. For client authentication ~/.ssh/id_dropbear is loaded by default .SH EXAMPLE # dropbearconvert openssh dropbear ~/.ssh/id_rsa ~/.ssh/id_dropbear diff --git a/dropbearkey.1 b/dropbearkey.1 index b4d202e..65dc9d2 100644 --- a/dropbearkey.1 +++ b/dropbearkey.1 @@ -12,10 +12,7 @@ dropbearkey \- create private keys for the use with dropbear(8) or dbclient(1) .SH DESCRIPTION .B dropbearkey generates a -.I RSA -.I DSS, -or -.I ECDSA +\fIRSA\fR, \fIDSS\fR, or \fIECDSA\fR format SSH private key, and saves it to a file for the use with the Dropbear client or server. Note that @@ -33,7 +30,7 @@ or .TP .B \-f \fIfile Write the secret key to the file -.IR file . For client authentication ~/.ssh/id_dropbear is loaded by default +\fIfile\fR. For client authentication ~/.ssh/id_dropbear is loaded by default .TP .B \-s \fIbits Set the key size to -- 2.6.2 From guilhem at fripost.org Sun Oct 25 02:39:50 2015 From: guilhem at fripost.org (Guilhem Moulin) Date: Sat, 24 Oct 2015 20:39:50 +0200 Subject: [PATCH] Fix minor manpage formatting issues. In-Reply-To: <1445711529-9216-1-git-send-email-guilhem@fripost.org> References: <1445711529-9216-1-git-send-email-guilhem@fripost.org> Message-ID: <20151024183950.GA9902@localhost.localdomain> Hi, We've also got the two attached patches in the Debian package. Please consider applying them upstream. (Actually both dropbear(8) and dropbearconvert(1) mention the ?-y? flag for dropbearkey, but it's currently undocumented in the upstream manpage.) Cheers, -- Guilhem. -------------- next part -------------- A non-text attachment was scrubbed... Name: 0001-dbclient.1-dbclient-uses-compression-if-compiled-with.diff Type: text/x-diff Size: 1516 bytes Desc: not available Url : http://lists.ucc.gu.uwa.edu.au/pipermail/dropbear/attachments/20151024/5c43d77d/attachment.diff -------------- next part -------------- A non-text attachment was scrubbed... Name: 0002-dropbearkey.8-mention-y-option-add-example.diff Type: text/x-diff Size: 1302 bytes Desc: not available Url : http://lists.ucc.gu.uwa.edu.au/pipermail/dropbear/attachments/20151024/5c43d77d/attachment-0001.diff -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available Url : http://lists.ucc.gu.uwa.edu.au/pipermail/dropbear/attachments/20151024/5c43d77d/attachment.sig From matt at ucc.asn.au Wed Oct 28 21:44:34 2015 From: matt at ucc.asn.au (Matt Johnston) Date: Wed, 28 Oct 2015 21:44:34 +0800 Subject: [PATCH] Fix minor manpage formatting issues. In-Reply-To: <20151024183950.GA9902@localhost.localdomain> References: <1445711529-9216-1-git-send-email-guilhem@fripost.org> <20151024183950.GA9902@localhost.localdomain> Message-ID: Thanks, I've applied these. Matt > On Sun 25/10/2015, at 2:39 am, Guilhem Moulin wrote: > > Hi, > > We've also got the two attached patches in the Debian package. Please > consider applying them upstream. (Actually both dropbear(8) and > dropbearconvert(1) mention the ?-y? flag for dropbearkey, but it's > currently undocumented in the upstream manpage.) > > Cheers, > -- > Guilhem. > <0001-dbclient.1-dbclient-uses-compression-if-compiled-with.diff><0002-dropbearkey.8-mention-y-option-add-example.diff> From matt at ucc.asn.au Wed Oct 28 21:47:24 2015 From: matt at ucc.asn.au (Matt Johnston) Date: Wed, 28 Oct 2015 21:47:24 +0800 Subject: svr_getopts should either support bundling or fail if bundling is used In-Reply-To: <20151022002412.GA14007@localhost.localdomain> References: <20151013191331.GA10317@localhost.localdomain> <068C216B-5328-4AB6-86B5-DB4FC2EF5472@ucc.asn.au> <20151021172155.GA5874@localhost.localdomain> <463831C1-9E9E-48F8-BACA-15DC734AA300@ucc.asn.au> <20151022002412.GA14007@localhost.localdomain> Message-ID: > On Thu 22/10/2015, at 8:24 am, Guilhem Moulin wrote: > >>> By the way, out of curiosity, is there a reason why you're not using >>> getopt()? It's POSIX after all, and you're already using it for scp. >> >> I think I looked into it a long time ago and it resulted in a larger >> static binary size. It might be worth revisiting though. Backward >> compatibility would still be an issue. > > Alright, I'll give it a try when time allows. I don't understand your > last sentence though: backward compatibility for what? I've changed the code to just print a warning for the time being. I'm intending for the next release to be soon with small bugfixes. Using getopt would probably be good though would require checking availability for the platforms where Dropbear is used. By backwards compatibility I just meant the issue where the behaviour would change slightly. Cheers, Matt From guilhem at fripost.org Fri Oct 30 04:41:31 2015 From: guilhem at fripost.org (Guilhem Moulin) Date: Thu, 29 Oct 2015 21:41:31 +0100 Subject: [PATCH] Enable bundling in svr-runopts's svr_getopts. In-Reply-To: References: Message-ID: <1446151291-3869-1-git-send-email-guilhem@fripost.org> On Wed, 28 Oct 2015 at 21:47:24 +0800, Matt Johnston wrote: > I've changed the code to just print a warning for the time being. I'm > intending for the next release to be soon with small bugfixes. Using getopt > would probably be good though would require checking availability for the > platforms where Dropbear is used. In fact the current code can easily be tweaked to enable bundling. (I've only touched svr-runopts for now; will proceed with cli-runopts if you're fine with that patch.) Refactoring the code to use getopt is actually cumpersome due to the #ifdef changing the option string. > By backwards compatibility I just meant the issue where the behaviour would > change slightly. --- svr-runopts.c | 59 +++++++++++++++++++++++++++++++---------------------------- 1 file changed, 31 insertions(+), 28 deletions(-) diff --git a/svr-runopts.c b/svr-runopts.c index 26c199b..ec29883 100644 --- a/svr-runopts.c +++ b/svr-runopts.c @@ -112,13 +112,14 @@ static void printhelp(const char * progname) { void svr_getopts(int argc, char ** argv) { - unsigned int i; + unsigned int i, j; char ** next = 0; int nextisport = 0; char* recv_window_arg = NULL; char* keepalive_arg = NULL; char* idle_timeout_arg = NULL; char* keyfile = NULL; + char c; /* see printhelp() for options */ @@ -168,33 +169,10 @@ void svr_getopts(int argc, char ** argv) { #endif for (i = 1; i < (unsigned int)argc; i++) { - if (nextisport) { - addportandaddress(argv[i]); - nextisport = 0; - continue; - } - - if (next) { - *next = argv[i]; - if (*next == NULL) { - dropbear_exit("Invalid null argument"); - } - next = 0x00; + if (argv[i][0] != '-' || argv[i][1] == '\0') + dropbear_exit("Invalid argument: %s", argv[i]); - if (keyfile) { - addhostkey(keyfile); - keyfile = NULL; - } - continue; - } - - if (argv[i][0] == '-') { - char c = argv[i][1]; - if (strlen(argv[i]) != 2) { - /* We only handle one flag per hyphen */ - fprintf(stderr, "Warning, trailing '%s' of '%s' is ignored.\n", - &argv[i][2], argv[i]); - } + for (j = 1; (c = argv[i][j]) != '\0' && !next && !nextisport; j++) { switch (c) { case 'b': next = &svr_opts.bannerfile; @@ -284,12 +262,37 @@ void svr_getopts(int argc, char ** argv) { exit(EXIT_SUCCESS); break; default: - fprintf(stderr, "Unknown argument %s\n", argv[i]); + fprintf(stderr, "Invalid option -%c\n", c); printhelp(argv[0]); exit(EXIT_FAILURE); break; } } + + if (!next && !nextisport) + continue; + + if (c == '\0') { + i++; + j = 0; + } + + if (nextisport) { + addportandaddress(&argv[i][j]); + nextisport = 0; + } + else if (next) { + *next = &argv[i][j]; + if (*next == NULL) { + dropbear_exit("Invalid null argument"); + } + next = 0x00; + + if (keyfile) { + addhostkey(keyfile); + keyfile = NULL; + } + } } /* Set up listening ports */ -- 2.6.2 From bacon at atrodo.org Wed Nov 4 12:25:44 2015 From: bacon at atrodo.org (Jon Gentle) Date: Tue, 3 Nov 2015 23:25:44 -0500 Subject: Module for perl that uses Dropbear Message-ID: Hello. I've been working on a perl module that would allow me to hook into and interact with Dropbear, Net::Dropbear. I just finished publishing it and wanted to let you know that it exists. I wrote it partially so that I could control a SSH server session from start to finish, including acceptable usernames and public keys. The other part that I wrote it was to understand the SSH protocol a little bit better. The Net::Dropbear package includes an entire copy of 2015.67 and when the perl package is configured, it patches Dropbear to add the hooks into Dropbear that I added for it's functionality. The reason it's 2015.67 and not 2015.68 is because I started a few months ago and haven't had a chance to go over how well it applies to 2015.68. You can find a copy of patch that gets applied at https://github.com/atrodo/Net-Dropbear/blob/master/dropbear.patch and the code in general is at https://github.com/atrodo/Net-Dropbear . Thank you for making Dropbear, it's been fun to work with. -Jon Gentle -------------- next part -------------- An HTML attachment was scrubbed... URL: http://lists.ucc.gu.uwa.edu.au/pipermail/dropbear/attachments/20151103/51f2fcfa/attachment-0001.htm From matt at ucc.asn.au Sat Nov 7 00:03:14 2015 From: matt at ucc.asn.au (Matt Johnston) Date: Sat, 7 Nov 2015 00:03:14 +0800 Subject: [PATCH] Enable bundling in svr-runopts's svr_getopts. In-Reply-To: <1446151291-3869-1-git-send-email-guilhem@fripost.org> References: <1446151291-3869-1-git-send-email-guilhem@fripost.org> Message-ID: Hi Guilhem, Thanks for the patch, it looks good. I've committed it with one small change (otherwise "dropbear -p" would segfault). I'll do the same for client unless you want to. Cheers, Matt > On Fri 30/10/2015, at 4:41 am, Guilhem Moulin wrote: > > On Wed, 28 Oct 2015 at 21:47:24 +0800, Matt Johnston wrote: >> I've changed the code to just print a warning for the time being. I'm >> intending for the next release to be soon with small bugfixes. Using getopt >> would probably be good though would require checking availability for the >> platforms where Dropbear is used. > > In fact the current code can easily be tweaked to enable bundling. > (I've only touched svr-runopts for now; will proceed with cli-runopts if > you're fine with that patch.) Refactoring the code to use getopt is > actually cumpersome due to the #ifdef changing the option string. > >> By backwards compatibility I just meant the issue where the behaviour would >> change slightly. > > --- > svr-runopts.c | 59 +++++++++++++++++++++++++++++++---------------------------- > 1 file changed, 31 insertions(+), 28 deletions(-) > > diff --git a/svr-runopts.c b/svr-runopts.c > index 26c199b..ec29883 100644 > --- a/svr-runopts.c > +++ b/svr-runopts.c > @@ -112,13 +112,14 @@ static void printhelp(const char * progname) { > > void svr_getopts(int argc, char ** argv) { > > - unsigned int i; > + unsigned int i, j; > char ** next = 0; > int nextisport = 0; > char* recv_window_arg = NULL; > char* keepalive_arg = NULL; > char* idle_timeout_arg = NULL; > char* keyfile = NULL; > + char c; > > > /* see printhelp() for options */ > @@ -168,33 +169,10 @@ void svr_getopts(int argc, char ** argv) { > #endif > > for (i = 1; i < (unsigned int)argc; i++) { > - if (nextisport) { > - addportandaddress(argv[i]); > - nextisport = 0; > - continue; > - } > - > - if (next) { > - *next = argv[i]; > - if (*next == NULL) { > - dropbear_exit("Invalid null argument"); > - } > - next = 0x00; > + if (argv[i][0] != '-' || argv[i][1] == '\0') > + dropbear_exit("Invalid argument: %s", argv[i]); > > - if (keyfile) { > - addhostkey(keyfile); > - keyfile = NULL; > - } > - continue; > - } > - > - if (argv[i][0] == '-') { > - char c = argv[i][1]; > - if (strlen(argv[i]) != 2) { > - /* We only handle one flag per hyphen */ > - fprintf(stderr, "Warning, trailing '%s' of '%s' is ignored.\n", > - &argv[i][2], argv[i]); > - } > + for (j = 1; (c = argv[i][j]) != '\0' && !next && !nextisport; j++) { > switch (c) { > case 'b': > next = &svr_opts.bannerfile; > @@ -284,12 +262,37 @@ void svr_getopts(int argc, char ** argv) { > exit(EXIT_SUCCESS); > break; > default: > - fprintf(stderr, "Unknown argument %s\n", argv[i]); > + fprintf(stderr, "Invalid option -%c\n", c); > printhelp(argv[0]); > exit(EXIT_FAILURE); > break; > } > } > + > + if (!next && !nextisport) > + continue; > + > + if (c == '\0') { > + i++; > + j = 0; > + } > + > + if (nextisport) { > + addportandaddress(&argv[i][j]); > + nextisport = 0; > + } > + else if (next) { > + *next = &argv[i][j]; > + if (*next == NULL) { > + dropbear_exit("Invalid null argument"); > + } > + next = 0x00; > + > + if (keyfile) { > + addhostkey(keyfile); > + keyfile = NULL; > + } > + } > } > > /* Set up listening ports */ > -- > 2.6.2 > From guilhem at fripost.org Sat Nov 7 00:26:16 2015 From: guilhem at fripost.org (Guilhem Moulin) Date: Fri, 6 Nov 2015 17:26:16 +0100 Subject: [PATCH] Enable bundling in svr-runopts's svr_getopts. In-Reply-To: References: <1446151291-3869-1-git-send-email-guilhem@fripost.org> Message-ID: <20151106162616.GA23477@localhost.localdomain> On Sat, 07 Nov 2015 at 00:03:14 +0800, Matt Johnston wrote: > Thanks for the patch, it looks good. I've committed it with one small > change (otherwise "dropbear -p" would segfault). I'll do the same for > client unless you want to. Awesome! I don't have time this week-end, but I can have a look during the next one. Unless you've already done it, that is ;-) -- Guilhem. -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available Url : http://lists.ucc.gu.uwa.edu.au/pipermail/dropbear/attachments/20151106/a9ea6c8d/attachment.sig From guilhem at fripost.org Thu Nov 12 02:02:36 2015 From: guilhem at fripost.org (Guilhem Moulin) Date: Wed, 11 Nov 2015 19:02:36 +0100 Subject: [PATCH] Enable bundling for dbclient. In-Reply-To: References: Message-ID: <1447264956-524-1-git-send-email-guilhem@fripost.org> --- cli-runopts.c | 202 ++++++++++++++++++++++++++-------------------------------- 1 file changed, 92 insertions(+), 110 deletions(-) diff --git a/cli-runopts.c b/cli-runopts.c index 2b0cb7d..59ebb5a 100644 --- a/cli-runopts.c +++ b/cli-runopts.c @@ -105,25 +105,30 @@ static void printhelp() { void cli_getopts(int argc, char ** argv) { unsigned int i, j; char ** next = 0; - unsigned int cmdlen; + enum { #ifdef ENABLE_CLI_PUBKEY_AUTH - int nextiskey = 0; /* A flag if the next argument is a keyfile */ + OPT_AUTHKEY, #endif #ifdef ENABLE_CLI_LOCALTCPFWD - int nextislocal = 0; + OPT_LOCALTCPFWD, #endif #ifdef ENABLE_CLI_REMOTETCPFWD - int nextisremote = 0; + OPT_REMOTETCPFWD, #endif #ifdef ENABLE_CLI_NETCAT - int nextisnetcat = 0; + OPT_NETCAT, #endif + /* a flag (no arg) if 'next' is NULL, a string-valued option otherwise */ + OPT_OTHER + } opt; + unsigned int cmdlen; char* dummy = NULL; /* Not used for anything real */ char* recv_window_arg = NULL; char* keepalive_arg = NULL; char* idle_timeout_arg = NULL; char *host_arg = NULL; + char c; /* see printhelp() for options */ cli_opts.progname = argv[0]; @@ -172,58 +177,8 @@ void cli_getopts(int argc, char ** argv) { fill_own_user(); - /* Iterate all the arguments */ - for (i = 1; i < (unsigned int)argc; i++) { -#ifdef ENABLE_CLI_PUBKEY_AUTH - if (nextiskey) { - /* Load a hostkey since the previous argument was "-i" */ - loadidentityfile(argv[i], 1); - nextiskey = 0; - continue; - } -#endif -#ifdef ENABLE_CLI_REMOTETCPFWD - if (nextisremote) { - TRACE(("nextisremote true")) - addforward(argv[i], cli_opts.remotefwds); - nextisremote = 0; - continue; - } -#endif -#ifdef ENABLE_CLI_LOCALTCPFWD - if (nextislocal) { - TRACE(("nextislocal true")) - addforward(argv[i], cli_opts.localfwds); - nextislocal = 0; - continue; - } -#endif -#ifdef ENABLE_CLI_NETCAT - if (nextisnetcat) { - TRACE(("nextisnetcat true")) - add_netcat(argv[i]); - nextisnetcat = 0; - continue; - } -#endif - if (next) { - /* The previous flag set a value to assign */ - *next = argv[i]; - if (*next == NULL) { - dropbear_exit("Invalid null argument"); - } - next = NULL; - continue; - } - - if (argv[i][0] == '-') { - /* A flag *waves* */ - char c = argv[i][1]; - if (strlen(argv[i]) != 2) { - /* We only handle one flag per hyphen */ - fprintf(stderr, "Warning, trailing '%s' of '%s' is ignored.\n", - &argv[i][2], argv[i]); - } + for (i = 1; i < (unsigned int)argc && argv[i][0] == '-'; i++) { + for (j = 1; (c = argv[i][j]) != '\0' && !next && opt == OPT_OTHER; j++) { switch (c) { case 'y': /* always accept the remote hostkey */ if (cli_opts.always_accept_key) { @@ -237,12 +192,7 @@ void cli_getopts(int argc, char ** argv) { break; #ifdef ENABLE_CLI_PUBKEY_AUTH case 'i': /* an identityfile */ - /* Keep scp happy when it changes "-i file" to "-ifile" */ - if (strlen(argv[i]) > 2) { - loadidentityfile(&argv[i][2], 1); - } else { - nextiskey = 1; - } + opt = OPT_AUTHKEY; break; #endif case 't': /* we want a pty */ @@ -262,7 +212,7 @@ void cli_getopts(int argc, char ** argv) { break; #ifdef ENABLE_CLI_LOCALTCPFWD case 'L': - nextislocal = 1; + opt = OPT_LOCALTCPFWD; break; case 'g': opts.listen_fwd_all = 1; @@ -270,12 +220,12 @@ void cli_getopts(int argc, char ** argv) { #endif #ifdef ENABLE_CLI_REMOTETCPFWD case 'R': - nextisremote = 1; + opt = OPT_REMOTETCPFWD; break; #endif #ifdef ENABLE_CLI_NETCAT case 'B': - nextisnetcat = 1; + opt = OPT_NETCAT; break; #endif #ifdef ENABLE_CLI_PROXYCMD @@ -341,50 +291,87 @@ void cli_getopts(int argc, char ** argv) { case 'b': next = &dummy; default: - fprintf(stderr, - "WARNING: Ignoring unknown argument '%s'\n", argv[i]); + fprintf(stderr, + "WARNING: Ignoring unknown option -%c\n", c); break; } /* Switch */ - - /* Now we handle args where they might be "-luser" (no spaces)*/ - if (next && strlen(argv[i]) > 2) { - *next = &argv[i][2]; - next = NULL; - } + } - continue; /* next argument */ + if (!next && opt == OPT_OTHER) /* got a flag */ + continue; - } else { - TRACE(("non-flag arg: '%s'", argv[i])) - - /* Either the hostname or commands */ - - if (host_arg == NULL) { - host_arg = argv[i]; - } else { - - /* this is part of the commands to send - after this we - * don't parse any more options, and flags are sent as the - * command */ - cmdlen = 0; - for (j = i; j < (unsigned int)argc; j++) { - cmdlen += strlen(argv[j]) + 1; /* +1 for spaces */ - } - /* Allocate the space */ - cli_opts.cmd = (char*)m_malloc(cmdlen); - cli_opts.cmd[0] = '\0'; - - /* Append all the bits */ - for (j = i; j < (unsigned int)argc; j++) { - strlcat(cli_opts.cmd, argv[j], cmdlen); - strlcat(cli_opts.cmd, " ", cmdlen); - } - /* It'll be null-terminated here */ - - /* We've eaten all the options and flags */ - break; - } + if (c == '\0') { + i++; + j = 0; + if (!argv[i]) + dropbear_exit("Missing argument"); } + +#ifdef ENABLE_CLI_PUBKEY_AUTH + if (opt == OPT_AUTHKEY) { + TRACE(("opt authkey")) + loadidentityfile(&argv[i][j], 1); + } + else +#endif +#ifdef ENABLE_CLI_REMOTETCPFWD + if (opt == OPT_REMOTETCPFWD) { + TRACE(("opt remotetcpfwd")) + addforward(&argv[i][j], cli_opts.remotefwds); + } + else +#endif +#ifdef ENABLE_CLI_LOCALTCPFWD + if (opt == OPT_LOCALTCPFWD) { + TRACE(("opt localtcpfwd")) + addforward(&argv[i][j], cli_opts.localfwds); + } + else +#endif +#ifdef ENABLE_CLI_NETCAT + if (opt == OPT_NETCAT) { + TRACE(("opt netcat")) + add_netcat(&argv[i][j]); + } + else +#endif + if (next) { + /* The previous flag set a value to assign */ + *next = &argv[i][j]; + if (*next == NULL) + dropbear_exit("Invalid null argument"); + next = NULL; + } + opt = OPT_OTHER; + } + + /* Done with options/flags; now handle the hostname (which may not + * start with a hyphen) and optional command */ + + if (i >= (unsigned int)argc) { /* missing hostname */ + printhelp(); + exit(EXIT_FAILURE); + } + host_arg = argv[i++]; + TRACE(("host is: %s", host_arg)) + + if (i < (unsigned int)argc) { + /* Build the command to send */ + cmdlen = 0; + for (j = i; j < (unsigned int)argc; j++) + cmdlen += strlen(argv[j]) + 1; /* +1 for spaces */ + + /* Allocate the space */ + cli_opts.cmd = (char*)m_malloc(cmdlen); + cli_opts.cmd[0] = '\0'; + + /* Append all the bits */ + for (j = i; j < (unsigned int)argc; j++) { + strlcat(cli_opts.cmd, argv[j], cmdlen); + strlcat(cli_opts.cmd, " ", cmdlen); + } + /* It'll be null-terminated here */ + TRACE(("cmd is: %s", cli_opts.cmd)) } /* And now a few sanity checks and setup */ @@ -393,11 +380,6 @@ void cli_getopts(int argc, char ** argv) { parse_ciphers_macs(); #endif - if (host_arg == NULL) { - printhelp(); - exit(EXIT_FAILURE); - } - #ifdef ENABLE_CLI_PROXYCMD if (cli_opts.proxycmd) { /* To match the common path of m_freeing it */ -- 2.6.2 From zbynek.michl at gmail.com Thu Nov 12 17:28:27 2015 From: zbynek.michl at gmail.com (Zbynek Michl) Date: Thu, 12 Nov 2015 10:28:27 +0100 Subject: Dropbear SSH integrity error Message-ID: Hello, I have a problem with SSH connection from my Ubiquiti AirOS device to a Mikrotik router. I use AirOS v5.5.11 (Dropbear SSH client v2013.58) and Mikrotik's RouterOS version 6.33 (also older versions have the same problem). # ssh demo at demo.mt.lv ssh: Connection to demo at demo.mt.lv:22 exited: Integrity error (bad packet size -22146333) Note: demo at demo.mt.lv is publicly accessible demo box usually with the latest version of RouterOS installed. It seems that this is a general bug of Dropbear SSH client (I have tried a different versions and all of them are problematic). I can access Mikrotik from my computer with no problem (OpenSSH_6.0p1). Any idea how to fix it? Thanks, Zbynek From ska-dietlibc at skarnet.org Thu Nov 12 18:17:06 2015 From: ska-dietlibc at skarnet.org (Laurent Bercot) Date: Thu, 12 Nov 2015 11:17:06 +0100 Subject: Dropbear SSH integrity error In-Reply-To: References: Message-ID: <56446722.2090703@skarnet.org> On 12/11/2015 10:28, Zbynek Michl wrote: > # ssh demo at demo.mt.lv > ssh: Connection to demo at demo.mt.lv:22 exited: Integrity error (bad > packet size -22146333) Hi Zbynek, I had the same problem earlier this year with GitHub: git over ssh worked with an OpenSSH client, but not with a dropbear client. I reported it to GitHub; I didn't get any answer, but two weeks or so later, they fixed the problem. I suspect this has to do with some sshd server that is not compatible with a dropbear client - I'm inclined to think that the server makes assumptions about unspecified protocol details and those assumptions are false with dbclient. Or the other way around. Do you know what sshd server RouterOS uses? I can't tell from demo at demo.mt.lv because they don't provide a shell :) -- Laurent From zbynek.michl at gmail.com Thu Nov 12 18:33:31 2015 From: zbynek.michl at gmail.com (Zbynek Michl) Date: Thu, 12 Nov 2015 11:33:31 +0100 Subject: Dropbear SSH integrity error In-Reply-To: <56446722.2090703@skarnet.org> References: <56446722.2090703@skarnet.org> Message-ID: Hi Laurent, RouterOS SSH server identifies as follows: $ telnet demo at demo.mt.lv 22 SSH-2.0-ROSSSH A connection in the opposite direction (from RouterOS to AirOS) is OK. Regards, Zbynek On Thu, Nov 12, 2015 at 11:17 AM, Laurent Bercot wrote: > On 12/11/2015 10:28, Zbynek Michl wrote: >> >> # ssh demo at demo.mt.lv >> ssh: Connection to demo at demo.mt.lv:22 exited: Integrity error (bad >> packet size -22146333) > > > Hi Zbynek, > > I had the same problem earlier this year with GitHub: git over ssh > worked with an OpenSSH client, but not with a dropbear client. > I reported it to GitHub; I didn't get any answer, but two weeks or > so later, they fixed the problem. > > I suspect this has to do with some sshd server that is not compatible > with a dropbear client - I'm inclined to think that the server makes > assumptions about unspecified protocol details and those assumptions > are false with dbclient. Or the other way around. > > Do you know what sshd server RouterOS uses? I can't tell from > demo at demo.mt.lv because they don't provide a shell :) > > -- > Laurent > From matt at ucc.asn.au Thu Nov 12 22:01:46 2015 From: matt at ucc.asn.au (Matt Johnston) Date: Thu, 12 Nov 2015 22:01:46 +0800 Subject: Dropbear SSH integrity error In-Reply-To: References: Message-ID: <20151112140146.GF7184@ucc.gu.uwa.edu.au> On Thu, Nov 12, 2015 at 10:28:27AM +0100, Zbynek Michl wrote: > Hello, > > I have a problem with SSH connection from my Ubiquiti AirOS device to > a Mikrotik router. > I use AirOS v5.5.11 (Dropbear SSH client v2013.58) and Mikrotik's > RouterOS version 6.33 (also older versions have the same problem). > > # ssh demo at demo.mt.lv > ssh: Connection to demo at demo.mt.lv:22 exited: Integrity error (bad > packet size -22146333) It looks like ROSSH doesn't support "kex first follows". Commenting out USE_KEX_FIRST_FOLLOWS in sysoptions.h will fix that. Could you report it to Mikrotik? Github used libssh which had the same issue, fixed in libssh 0.7 Cheers, Matt From zbynek.michl at gmail.com Fri Nov 13 19:32:36 2015 From: zbynek.michl at gmail.com (Zbynek Michl) Date: Fri, 13 Nov 2015 12:32:36 +0100 Subject: Dropbear SSH integrity error In-Reply-To: <20151112140146.GF7184@ucc.gu.uwa.edu.au> References: <20151112140146.GF7184@ucc.gu.uwa.edu.au> Message-ID: Hi Matt, Ok, I reported it to Mikrotik, thanks for your help! :) Regards, Zbynek On Thu, Nov 12, 2015 at 3:01 PM, Matt Johnston wrote: > On Thu, Nov 12, 2015 at 10:28:27AM +0100, Zbynek Michl wrote: >> Hello, >> >> I have a problem with SSH connection from my Ubiquiti AirOS device to >> a Mikrotik router. >> I use AirOS v5.5.11 (Dropbear SSH client v2013.58) and Mikrotik's >> RouterOS version 6.33 (also older versions have the same problem). >> >> # ssh demo at demo.mt.lv >> ssh: Connection to demo at demo.mt.lv:22 exited: Integrity error (bad >> packet size -22146333) > > It looks like ROSSH doesn't support "kex first follows". > Commenting out USE_KEX_FIRST_FOLLOWS in sysoptions.h will > fix that. Could you report it to Mikrotik? Github used > libssh which had the same issue, fixed in libssh 0.7 > > Cheers, > Matt From matt at ucc.asn.au Mon Nov 23 23:04:19 2015 From: matt at ucc.asn.au (Matt Johnston) Date: Mon, 23 Nov 2015 23:04:19 +0800 Subject: [PATCH] Enable bundling for dbclient. In-Reply-To: <1447264956-524-1-git-send-email-guilhem@fripost.org> References: <1447264956-524-1-git-send-email-guilhem@fripost.org> Message-ID: <20151123150419.GC13889@ucc.gu.uwa.edu.au> Hi, Thanks for the patch, I've applied it slight changes. I had to re-add the undocumented support for extra arguments after the hostname - that's in the manpage now. Cheers, Matt On Wed, Nov 11, 2015 at 07:02:36PM +0100, Guilhem Moulin wrote: > --- > cli-runopts.c | 202 ++++++++++++++++++++++++++-------------------------------- > 1 file changed, 92 insertions(+), 110 deletions(-) > > diff --git a/cli-runopts.c b/cli-runopts.c > index 2b0cb7d..59ebb5a 100644 > --- a/cli-runopts.c > +++ b/cli-runopts.c > @@ -105,25 +105,30 @@ static void printhelp() { > void cli_getopts(int argc, char ** argv) { > unsigned int i, j; > char ** next = 0; > - unsigned int cmdlen; > + enum { > #ifdef ENABLE_CLI_PUBKEY_AUTH > - int nextiskey = 0; /* A flag if the next argument is a keyfile */ > + OPT_AUTHKEY, > #endif > #ifdef ENABLE_CLI_LOCALTCPFWD > - int nextislocal = 0; > + OPT_LOCALTCPFWD, > #endif > #ifdef ENABLE_CLI_REMOTETCPFWD > - int nextisremote = 0; > + OPT_REMOTETCPFWD, > #endif > #ifdef ENABLE_CLI_NETCAT > - int nextisnetcat = 0; > + OPT_NETCAT, > #endif > + /* a flag (no arg) if 'next' is NULL, a string-valued option otherwise */ > + OPT_OTHER > + } opt; > + unsigned int cmdlen; > char* dummy = NULL; /* Not used for anything real */ > > char* recv_window_arg = NULL; > char* keepalive_arg = NULL; > char* idle_timeout_arg = NULL; > char *host_arg = NULL; > + char c; > > /* see printhelp() for options */ > cli_opts.progname = argv[0]; > @@ -172,58 +177,8 @@ void cli_getopts(int argc, char ** argv) { > > fill_own_user(); > > - /* Iterate all the arguments */ > - for (i = 1; i < (unsigned int)argc; i++) { > -#ifdef ENABLE_CLI_PUBKEY_AUTH > - if (nextiskey) { > - /* Load a hostkey since the previous argument was "-i" */ > - loadidentityfile(argv[i], 1); > - nextiskey = 0; > - continue; > - } > -#endif > -#ifdef ENABLE_CLI_REMOTETCPFWD > - if (nextisremote) { > - TRACE(("nextisremote true")) > - addforward(argv[i], cli_opts.remotefwds); > - nextisremote = 0; > - continue; > - } > -#endif > -#ifdef ENABLE_CLI_LOCALTCPFWD > - if (nextislocal) { > - TRACE(("nextislocal true")) > - addforward(argv[i], cli_opts.localfwds); > - nextislocal = 0; > - continue; > - } > -#endif > -#ifdef ENABLE_CLI_NETCAT > - if (nextisnetcat) { > - TRACE(("nextisnetcat true")) > - add_netcat(argv[i]); > - nextisnetcat = 0; > - continue; > - } > -#endif > - if (next) { > - /* The previous flag set a value to assign */ > - *next = argv[i]; > - if (*next == NULL) { > - dropbear_exit("Invalid null argument"); > - } > - next = NULL; > - continue; > - } > - > - if (argv[i][0] == '-') { > - /* A flag *waves* */ > - char c = argv[i][1]; > - if (strlen(argv[i]) != 2) { > - /* We only handle one flag per hyphen */ > - fprintf(stderr, "Warning, trailing '%s' of '%s' is ignored.\n", > - &argv[i][2], argv[i]); > - } > + for (i = 1; i < (unsigned int)argc && argv[i][0] == '-'; i++) { > + for (j = 1; (c = argv[i][j]) != '\0' && !next && opt == OPT_OTHER; j++) { > switch (c) { > case 'y': /* always accept the remote hostkey */ > if (cli_opts.always_accept_key) { > @@ -237,12 +192,7 @@ void cli_getopts(int argc, char ** argv) { > break; > #ifdef ENABLE_CLI_PUBKEY_AUTH > case 'i': /* an identityfile */ > - /* Keep scp happy when it changes "-i file" to "-ifile" */ > - if (strlen(argv[i]) > 2) { > - loadidentityfile(&argv[i][2], 1); > - } else { > - nextiskey = 1; > - } > + opt = OPT_AUTHKEY; > break; > #endif > case 't': /* we want a pty */ > @@ -262,7 +212,7 @@ void cli_getopts(int argc, char ** argv) { > break; > #ifdef ENABLE_CLI_LOCALTCPFWD > case 'L': > - nextislocal = 1; > + opt = OPT_LOCALTCPFWD; > break; > case 'g': > opts.listen_fwd_all = 1; > @@ -270,12 +220,12 @@ void cli_getopts(int argc, char ** argv) { > #endif > #ifdef ENABLE_CLI_REMOTETCPFWD > case 'R': > - nextisremote = 1; > + opt = OPT_REMOTETCPFWD; > break; > #endif > #ifdef ENABLE_CLI_NETCAT > case 'B': > - nextisnetcat = 1; > + opt = OPT_NETCAT; > break; > #endif > #ifdef ENABLE_CLI_PROXYCMD > @@ -341,50 +291,87 @@ void cli_getopts(int argc, char ** argv) { > case 'b': > next = &dummy; > default: > - fprintf(stderr, > - "WARNING: Ignoring unknown argument '%s'\n", argv[i]); > + fprintf(stderr, > + "WARNING: Ignoring unknown option -%c\n", c); > break; > } /* Switch */ > - > - /* Now we handle args where they might be "-luser" (no spaces)*/ > - if (next && strlen(argv[i]) > 2) { > - *next = &argv[i][2]; > - next = NULL; > - } > + } > > - continue; /* next argument */ > + if (!next && opt == OPT_OTHER) /* got a flag */ > + continue; > > - } else { > - TRACE(("non-flag arg: '%s'", argv[i])) > - > - /* Either the hostname or commands */ > - > - if (host_arg == NULL) { > - host_arg = argv[i]; > - } else { > - > - /* this is part of the commands to send - after this we > - * don't parse any more options, and flags are sent as the > - * command */ > - cmdlen = 0; > - for (j = i; j < (unsigned int)argc; j++) { > - cmdlen += strlen(argv[j]) + 1; /* +1 for spaces */ > - } > - /* Allocate the space */ > - cli_opts.cmd = (char*)m_malloc(cmdlen); > - cli_opts.cmd[0] = '\0'; > - > - /* Append all the bits */ > - for (j = i; j < (unsigned int)argc; j++) { > - strlcat(cli_opts.cmd, argv[j], cmdlen); > - strlcat(cli_opts.cmd, " ", cmdlen); > - } > - /* It'll be null-terminated here */ > - > - /* We've eaten all the options and flags */ > - break; > - } > + if (c == '\0') { > + i++; > + j = 0; > + if (!argv[i]) > + dropbear_exit("Missing argument"); > } > + > +#ifdef ENABLE_CLI_PUBKEY_AUTH > + if (opt == OPT_AUTHKEY) { > + TRACE(("opt authkey")) > + loadidentityfile(&argv[i][j], 1); > + } > + else > +#endif > +#ifdef ENABLE_CLI_REMOTETCPFWD > + if (opt == OPT_REMOTETCPFWD) { > + TRACE(("opt remotetcpfwd")) > + addforward(&argv[i][j], cli_opts.remotefwds); > + } > + else > +#endif > +#ifdef ENABLE_CLI_LOCALTCPFWD > + if (opt == OPT_LOCALTCPFWD) { > + TRACE(("opt localtcpfwd")) > + addforward(&argv[i][j], cli_opts.localfwds); > + } > + else > +#endif > +#ifdef ENABLE_CLI_NETCAT > + if (opt == OPT_NETCAT) { > + TRACE(("opt netcat")) > + add_netcat(&argv[i][j]); > + } > + else > +#endif > + if (next) { > + /* The previous flag set a value to assign */ > + *next = &argv[i][j]; > + if (*next == NULL) > + dropbear_exit("Invalid null argument"); > + next = NULL; > + } > + opt = OPT_OTHER; > + } > + > + /* Done with options/flags; now handle the hostname (which may not > + * start with a hyphen) and optional command */ > + > + if (i >= (unsigned int)argc) { /* missing hostname */ > + printhelp(); > + exit(EXIT_FAILURE); > + } > + host_arg = argv[i++]; > + TRACE(("host is: %s", host_arg)) > + > + if (i < (unsigned int)argc) { > + /* Build the command to send */ > + cmdlen = 0; > + for (j = i; j < (unsigned int)argc; j++) > + cmdlen += strlen(argv[j]) + 1; /* +1 for spaces */ > + > + /* Allocate the space */ > + cli_opts.cmd = (char*)m_malloc(cmdlen); > + cli_opts.cmd[0] = '\0'; > + > + /* Append all the bits */ > + for (j = i; j < (unsigned int)argc; j++) { > + strlcat(cli_opts.cmd, argv[j], cmdlen); > + strlcat(cli_opts.cmd, " ", cmdlen); > + } > + /* It'll be null-terminated here */ > + TRACE(("cmd is: %s", cli_opts.cmd)) > } > > /* And now a few sanity checks and setup */ > @@ -393,11 +380,6 @@ void cli_getopts(int argc, char ** argv) { > parse_ciphers_macs(); > #endif > > - if (host_arg == NULL) { > - printhelp(); > - exit(EXIT_FAILURE); > - } > - > #ifdef ENABLE_CLI_PROXYCMD > if (cli_opts.proxycmd) { > /* To match the common path of m_freeing it */ > -- > 2.6.2 > From guilhem at fripost.org Tue Nov 24 19:11:36 2015 From: guilhem at fripost.org (Guilhem Moulin) Date: Tue, 24 Nov 2015 12:11:36 +0100 Subject: [PATCH] Fix minor manpage formatting issues. In-Reply-To: References: <1445711529-9216-1-git-send-email-guilhem@fripost.org> <20151024183950.GA9902@localhost.localdomain> Message-ID: <20151124111136.GA9443@localhost.localdomain> On Wed, 28 Oct 2015 at 21:44:34 +0800, Matt Johnston wrote: > Thanks, I've applied these. In case you just missed it: FYI the patch at the root of this thread has not been applied. -- Guilhem. -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available Url : http://lists.ucc.gu.uwa.edu.au/pipermail/dropbear/attachments/20151124/6f69fc9d/attachment.sig From naveen.mamindlapalli at gmail.com Wed Nov 25 14:55:34 2015 From: naveen.mamindlapalli at gmail.com (Naveen Mamindlapalli) Date: Wed, 25 Nov 2015 12:25:34 +0530 Subject: dropbear uclinux multiple client sessions Message-ID: Hi All, I am using dropbear sshd v0.52 (non-inetd mode) ported onto uClinux os running on ARM cortex M3 processor. I found that dropbear server is not accepting multiple ssh clients at a time. I need to exit the first client & then second client is connected. Until then the second client is suspended. I have debugged the code and found that vfork() in main_noinetd() is not returned for parent process & after reading man pages, I found that vfork() suspends the parent process until child exits or calls execve(). Is there any fix for this issue? Thanks and Regards, Naveen From matt at ucc.asn.au Wed Nov 25 20:55:08 2015 From: matt at ucc.asn.au (Matt Johnston) Date: Wed, 25 Nov 2015 20:55:08 +0800 Subject: dropbear uclinux multiple client sessions In-Reply-To: References: Message-ID: <20151125125508.GD13889@ucc.gu.uwa.edu.au> Hi Naveen, This is a limitation of uClinux. Running from inetd is the recommended solution. Cheers, Matt On Wed, Nov 25, 2015 at 12:25:34PM +0530, Naveen Mamindlapalli wrote: > Hi All, > > I am using dropbear sshd v0.52 (non-inetd mode) ported onto uClinux os > running on ARM cortex M3 processor. > > I found that dropbear server is not accepting multiple ssh clients at > a time. I need to exit the first client & then second client is > connected. Until then the second client is suspended. > > I have debugged the code and found that vfork() in main_noinetd() is > not returned for parent process & after reading man pages, I found > that vfork() suspends the parent process until child exits or calls > execve(). > > Is there any fix for this issue? > > Thanks and Regards, > Naveen From zbynek.michl at gmail.com Wed Nov 25 21:15:08 2015 From: zbynek.michl at gmail.com (Zbynek Michl) Date: Wed, 25 Nov 2015 14:15:08 +0100 Subject: Dropbear SSH integrity error Message-ID: Hi, So it seems to be fixed in RouterOS 6.34rc8. Regards, Zbynek On Fri, Nov 13, 2015 at 12:32 PM, Zbynek Michl wrote: > Hi Matt, > > Ok, I reported it to Mikrotik, thanks for your help! :) > > Regards, > Zbynek > > On Thu, Nov 12, 2015 at 3:01 PM, Matt Johnston wrote: >> On Thu, Nov 12, 2015 at 10:28:27AM +0100, Zbynek Michl wrote: >>> Hello, >>> >>> I have a problem with SSH connection from my Ubiquiti AirOS device to >>> a Mikrotik router. >>> I use AirOS v5.5.11 (Dropbear SSH client v2013.58) and Mikrotik's >>> RouterOS version 6.33 (also older versions have the same problem). >>> >>> # ssh demo at demo.mt.lv >>> ssh: Connection to demo at demo.mt.lv:22 exited: Integrity error (bad >>> packet size -22146333) >> >> It looks like ROSSH doesn't support "kex first follows". >> Commenting out USE_KEX_FIRST_FOLLOWS in sysoptions.h will >> fix that. Could you report it to Mikrotik? Github used >> libssh which had the same issue, fixed in libssh 0.7 >> >> Cheers, >> Matt From matt at ucc.asn.au Wed Nov 25 23:30:21 2015 From: matt at ucc.asn.au (Matt Johnston) Date: Wed, 25 Nov 2015 23:30:21 +0800 Subject: Dropbear 2015.69 Message-ID: <20151125153021.GG13889@ucc.gu.uwa.edu.au> Hi all, It's the third Dropbear release this year, 2015.69. The most important change is a fix for TCP forwarding that broke in 2015.68 and affected a few people. As usual downloads at https://matt.ucc.asn.au/dropbear/dropbear.html and https://dropbear.nl/mirror/ Cheers, Matt 2015.69 - 25 November 2015 - Fix crash when forwarded TCP connections fail to connect (bug introduced in 2015.68) - Avoid hang on session close when multiple sessions are started, affects Qt Creator Patch from Andrzej Szombierski - Reduce per-channel memory consumption in common case, increase default channel limit from 100 to 1000 which should improve SOCKS forwarding for modern webpages - Handle multiple command line arguments in a single flag, thanks to Guilhem Moulin - Manpage improvements from Guilhem Moulin - Build fixes for Android from Mike Frysinger - Don't display the MOTD when an explicit command is run from Guilhem Moulin - Check curve25519 shared secret isn't zero From vapier at gentoo.org Thu Nov 26 15:33:58 2015 From: vapier at gentoo.org (Mike Frysinger) Date: Thu, 26 Nov 2015 02:33:58 -0500 Subject: dropbear uclinux multiple client sessions In-Reply-To: <20151125125508.GD13889@ucc.gu.uwa.edu.au> References: <20151125125508.GD13889@ucc.gu.uwa.edu.au> Message-ID: <20151126073358.GQ23754@vapier.lan> On 25 Nov 2015 20:55, Matt Johnston wrote: > This is a limitation of uClinux. Running from inetd is the > recommended solution. sure, but it's also a limitation in the current dropbear implementation. if the child execed something, the parent would be free to handle more requests. dropbear would need a way to exec itself and pass the info along to the child. considering it already works via inetd, it should be feasible to do it in dropbear itself too. -mike -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: Digital signature Url : http://lists.ucc.gu.uwa.edu.au/pipermail/dropbear/attachments/20151126/c5d78f26/attachment.sig From peter at softwolves.pp.se Thu Nov 26 16:40:26 2015 From: peter at softwolves.pp.se (Peter Krefting) Date: Thu, 26 Nov 2015 09:40:26 +0100 (CET) Subject: 2015.69 and password authentication Message-ID: Hi! Upgrading from 2015.68 to 2015.69 on the embedded platform at $DAYJOB, password authentication does no longer work. This seems to be related to a newly added test for crypt() in the configure script, which fails to find it and thus disables password authentication. Has anyone else been bit by this, and is there a better workaround than to edit options.h to unconditionally enable ENABLE_SVR_PASSWORD_AUTH (removing the #ifdef HAVE_CRYPT test, which works fine). -- \\// Peter - http://www.softwolves.pp.se/ From pmeerw at pmeerw.net Thu Nov 26 16:57:16 2015 From: pmeerw at pmeerw.net (Peter Meerwald-Stadler) Date: Thu, 26 Nov 2015 09:57:16 +0100 (CET) Subject: 2015.69 and password authentication In-Reply-To: References: Message-ID: > Upgrading from 2015.68 to 2015.69 on the embedded platform at $DAYJOB, > password authentication does no longer work. This seems to be related to a > newly added test for crypt() in the configure script, which fails to find it > and thus disables password authentication. same here > Has anyone else been bit by this, and is there a better workaround than to > edit options.h to unconditionally enable ENABLE_SVR_PASSWORD_AUTH (removing > the #ifdef HAVE_CRYPT test, which works fine). export ac_cv_func_crypt=yes before calling configure -- just a nasty workaround, haven't looked into the problem yet p. -- Peter Meerwald-Stadler +43-664-2444418 (mobile) From matt at ucc.asn.au Thu Nov 26 17:37:25 2015 From: matt at ucc.asn.au (Matt Johnston) Date: Thu, 26 Nov 2015 17:37:25 +0800 Subject: 2015.69 and password authentication In-Reply-To: References: Message-ID: <112A4DC5-13E7-413F-AB01-D5AC08D8C2BC@ucc.asn.au> I'll sort out a new release to fix this later today. Cheers, Matt On 26 November 2015 4:57:16 pm AWST, Peter Meerwald-Stadler wrote: > >> Upgrading from 2015.68 to 2015.69 on the embedded platform at >$DAYJOB, >> password authentication does no longer work. This seems to be related >to a >> newly added test for crypt() in the configure script, which fails to >find it >> and thus disables password authentication. > >same here > >> Has anyone else been bit by this, and is there a better workaround >than to >> edit options.h to unconditionally enable ENABLE_SVR_PASSWORD_AUTH >(removing >> the #ifdef HAVE_CRYPT test, which works fine). > >export ac_cv_func_crypt=yes >before calling configure -- just a nasty workaround, haven't looked >into >the problem yet > >p. From naveen.mamindlapalli at gmail.com Thu Nov 26 19:32:07 2015 From: naveen.mamindlapalli at gmail.com (Naveen Mamindlapalli) Date: Thu, 26 Nov 2015 17:02:07 +0530 Subject: dropbear uclinux multiple client sessions In-Reply-To: References: Message-ID: Hi All, Running from inetd solved the issue of multiple client connections at a time. There is a limitation with non-inetd mode. Am I correct? Thanks and Regards, Naveen On Wed, Nov 25, 2015 at 12:25 PM, Naveen Mamindlapalli wrote: > Hi All, > > I am using dropbear sshd v0.52 (non-inetd mode) ported onto uClinux os > running on ARM cortex M3 processor. > > I found that dropbear server is not accepting multiple ssh clients at > a time. I need to exit the first client & then second client is > connected. Until then the second client is suspended. > > I have debugged the code and found that vfork() in main_noinetd() is > not returned for parent process & after reading man pages, I found > that vfork() suspends the parent process until child exits or calls > execve(). > > Is there any fix for this issue? > > Thanks and Regards, > Naveen From naveen.mamindlapalli at gmail.com Thu Nov 26 20:22:23 2015 From: naveen.mamindlapalli at gmail.com (Naveen Mamindlapalli) Date: Thu, 26 Nov 2015 17:52:23 +0530 Subject: dropbear uclinux multiple client sessions In-Reply-To: References: Message-ID: Hi Matt, I didn't see your reply since it went into trash folder.Thanks for the reply. I tried running from inetd & it worked fine. Regards, Naveen On Thu, Nov 26, 2015 at 5:02 PM, Naveen Mamindlapalli wrote: > Hi All, > > Running from inetd solved the issue of multiple client connections at a time. > > There is a limitation with non-inetd mode. Am I correct? > > Thanks and Regards, > Naveen > > On Wed, Nov 25, 2015 at 12:25 PM, Naveen Mamindlapalli > wrote: >> Hi All, >> >> I am using dropbear sshd v0.52 (non-inetd mode) ported onto uClinux os >> running on ARM cortex M3 processor. >> >> I found that dropbear server is not accepting multiple ssh clients at >> a time. I need to exit the first client & then second client is >> connected. Until then the second client is suspended. >> >> I have debugged the code and found that vfork() in main_noinetd() is >> not returned for parent process & after reading man pages, I found >> that vfork() suspends the parent process until child exits or calls >> execve(). >> >> Is there any fix for this issue? >> >> Thanks and Regards, >> Naveen From matt at ucc.asn.au Thu Nov 26 23:21:20 2015 From: matt at ucc.asn.au (Matt Johnston) Date: Thu, 26 Nov 2015 23:21:20 +0800 Subject: Dropbear 2015.70 fixes password authentication In-Reply-To: <112A4DC5-13E7-413F-AB01-D5AC08D8C2BC@ucc.asn.au> References: <112A4DC5-13E7-413F-AB01-D5AC08D8C2BC@ucc.asn.au> Message-ID: <20151126152120.GH13889@ucc.gu.uwa.edu.au> Hi, Dropbear 2015.70 released now fixes password server authentication on Linux. It's a bit of an embarassing mistake, apologies for that. Commenting out the test in options.h is a fine workaround. Cheers, Matt On Thu, Nov 26, 2015 at 05:37:25PM +0800, Matt Johnston wrote: > I'll sort out a new release to fix this later today. > > Cheers, > Matt > > On 26 November 2015 4:57:16 pm AWST, Peter Meerwald-Stadler wrote: > > > >> Upgrading from 2015.68 to 2015.69 on the embedded platform at > >$DAYJOB, > >> password authentication does no longer work. This seems to be related > >to a > >> newly added test for crypt() in the configure script, which fails to > >find it > >> and thus disables password authentication. > > > >same here > > > >> Has anyone else been bit by this, and is there a better workaround > >than to > >> edit options.h to unconditionally enable ENABLE_SVR_PASSWORD_AUTH > >(removing > >> the #ifdef HAVE_CRYPT test, which works fine). > > > >export ac_cv_func_crypt=yes > >before calling configure -- just a nasty workaround, haven't looked > >into > >the problem yet > > > >p. > From jue at jue.li Fri Nov 27 16:44:52 2015 From: jue at jue.li (Juergen Daubert) Date: Fri, 27 Nov 2015 09:44:52 +0100 Subject: [PATCH] the '==' comparision operator is not defined by POSIX, use '=' instead in configure.ac Message-ID: <20151127084452.GA17880@jue.netz> Hello, the '==' operator is not defined by POSIX and it's use breaks building of dropbear with dash and possibly other POSIX shells. Greetings Juergen --- configure.ac.orig 2015-11-27 09:37:27.852925351 +0100 +++ configure.ac 2015-11-27 09:38:16.271165234 +0100 @@ -92,7 +92,7 @@ found_crypt_func=here ]) AC_SUBST(CRYPTLIB) -if test "t$found_crypt_func" == there; then +if test "t$found_crypt_func" = there; then AC_DEFINE(HAVE_CRYPT, 1, [crypt() function]) fi From peter at softwolves.pp.se Sun Nov 29 06:14:41 2015 From: peter at softwolves.pp.se (Peter Krefting) Date: Sat, 28 Nov 2015 23:14:41 +0100 (CET) Subject: Dropbear 2015.70 fixes password authentication In-Reply-To: <20151126152120.GH13889@ucc.gu.uwa.edu.au> References: <112A4DC5-13E7-413F-AB01-D5AC08D8C2BC@ucc.asn.au> <20151126152120.GH13889@ucc.gu.uwa.edu.au> Message-ID: Matt Johnston: > Dropbear 2015.70 released now fixes password server authentication > on Linux. This version does indeed fix the issue I was seeing. Thank you for the swift bug fix! -- \\// Peter - http://www.softwolves.pp.se/ From anthony.sherwin at dynamicratings.com Mon Nov 30 06:29:23 2015 From: anthony.sherwin at dynamicratings.com (Anthony Sherwin) Date: Sun, 29 Nov 2015 22:29:23 +0000 Subject: Dropbear 2015.70 scp upload errors Message-ID: Hi, I have cross compiled dropbear to my embedded product from 2015.68 to 2015.70 and am getting the following error: dropbear[2528]: Exit (user): Bad buf_incrpos This only happens when I am uploading a file through sftp. The client side is also hanging even though it says 100% complete. I tried looking into the code but I do not know where the error might be happening so any help would be great. Might also be useful to know that I have compiled libpam into dropbear and am using libpam for authentication. Thanks, Anthony -------------- next part -------------- An HTML attachment was scrubbed... URL: http://lists.ucc.gu.uwa.edu.au/pipermail/dropbear/attachments/20151129/b0dd90bb/attachment.htm From annulen at yandex.ru Mon Nov 30 19:24:23 2015 From: annulen at yandex.ru (Konstantin Tokarev) Date: Mon, 30 Nov 2015 14:24:23 +0300 Subject: Force dbclient to exit if remote forwarding request failed Message-ID: <362051448882663@web29g.yandex.ru> Hi all, I'm using dbclient to create persistent reverse tunnel, allowing SSH connection to device from outside. I use sh script like this: while true; do dbclient -y -NT -R 3320:localhost:22 user at host echo "dbclient exited with code $?. Respawning..." sleep 60 done However, sometimes I get message dbclient: Remote TCP forward request failed (port 3320 -> localhost:22) however dbclient keeps connection after this failure and does not exit. AFAIU, in this case SSH connection is absolutely useless, and dbclient could safely exit to allow my script to retry forward request. * -N => no command is executed * -T => not interactive session * Remote TCP forward request failed => no data will ever come from server 1. Is there any way to force dbclient to exit in this case? 2. If not, would it be reasonable feature to have in dbclient? -- Regards, Konstantin From matt at ucc.asn.au Mon Nov 30 20:34:33 2015 From: matt at ucc.asn.au (Matt Johnston) Date: Mon, 30 Nov 2015 20:34:33 +0800 Subject: Dropbear 2015.70 scp upload errors In-Reply-To: References: Message-ID: <319F15D5-45BF-42EC-B20D-3C2D5EED7891@ucc.asn.au> > On Mon 30/11/2015, at 6:29 am, Anthony Sherwin wrote: > > I have cross compiled dropbear to my embedded product from 2015.68 to 2015.70 and am getting the following error: > dropbear[2528]: Exit (user): Bad buf_incrpos Hi Anthony, This should be fixed by the latest change https://secure.ucc.asn.au/hg/dropbear/rev/af940cefdba1 Looks like I'll have to sort out another release shortly. Cheers, Matt -------------- next part -------------- An HTML attachment was scrubbed... URL: http://lists.ucc.gu.uwa.edu.au/pipermail/dropbear/attachments/20151130/f2ade526/attachment.htm From annulen at yandex.ru Mon Nov 30 20:38:34 2015 From: annulen at yandex.ru (Konstantin Tokarev) Date: Mon, 30 Nov 2015 15:38:34 +0300 Subject: Syslog logging in client Message-ID: <1049571448887114@web8j.yandex.ru> Hi all, I think it would be useful to have syslog loggin in client when it is running in background (e.g., when -f option is used, or, like in my case, it is started from init system to create tunnel). Would such contribution be welcome? -- Regards, Konstantin From matt at ucc.asn.au Mon Nov 30 20:39:15 2015 From: matt at ucc.asn.au (Matt Johnston) Date: Mon, 30 Nov 2015 20:39:15 +0800 Subject: Force dbclient to exit if remote forwarding request failed In-Reply-To: <362051448882663@web29g.yandex.ru> References: <362051448882663@web29g.yandex.ru> Message-ID: > On Mon 30/11/2015, at 7:24 pm, Konstantin Tokarev wrote: > dbclient: Remote TCP forward request failed (port 3320 -> localhost:22) > > ... > 1. Is there any way to force dbclient to exit in this case? > 2. If not, would it be reasonable feature to have in dbclient? Hi Konstantin, I can't think of a good way to force exit apart from grepping the command output for the error message. I'd like to see that as a feature - I was thinking about it a few days ago. I'd keep it under a command line flag for backwards compatibility. Failing to bind "-L" forward ports would also be handled similarly. I'll put it on the todo list, or patches accepted. Cheers, Matt From annulen at yandex.ru Mon Nov 30 20:40:12 2015 From: annulen at yandex.ru (Konstantin Tokarev) Date: Mon, 30 Nov 2015 15:40:12 +0300 Subject: Dropbear 2015.70 scp upload errors In-Reply-To: <319F15D5-45BF-42EC-B20D-3C2D5EED7891@ucc.asn.au> References: <319F15D5-45BF-42EC-B20D-3C2D5EED7891@ucc.asn.au> Message-ID: <1062501448887212@web8j.yandex.ru> 30.11.2015, 15:36, "Matt Johnston" : >> On Mon 30/11/2015, at 6:29 am, Anthony Sherwin wrote: >> >> I have cross compiled dropbear to my embedded product from 2015.68 to 2015.70 and am getting the following error: >> >> dropbear[2528]: Exit (user): Bad buf_incrpos > Hi Anthony, > > This should be fixed by the latest change?https://secure.ucc.asn.au/hg/dropbear/rev/af940cefdba1 > Looks like I'll have to sort out another release shortly. BTW, could you look into https://github.com/mkj/dropbear/pull/15? It's trivial and it would be nice to see it in release. -- Regards, Konstantin From annulen at yandex.ru Mon Nov 30 20:41:51 2015 From: annulen at yandex.ru (Konstantin Tokarev) Date: Mon, 30 Nov 2015 15:41:51 +0300 Subject: Force dbclient to exit if remote forwarding request failed In-Reply-To: References: <362051448882663@web29g.yandex.ru> Message-ID: <1074051448887311@web8j.yandex.ru> 30.11.2015, 15:39, "Matt Johnston" : >> ?On Mon 30/11/2015, at 7:24 pm, Konstantin Tokarev wrote: >> ?dbclient: Remote TCP forward request failed (port 3320 -> localhost:22) >> >> ?... >> ?1. Is there any way to force dbclient to exit in this case? >> ?2. If not, would it be reasonable feature to have in dbclient? > > Hi Konstantin, > > I can't think of a good way to force exit apart from grepping the command output for the error message. > > I'd like to see that as a feature - I was thinking about it a few days ago. I'd keep it under a command line flag for backwards compatibility. Failing to bind "-L" forward ports would also be handled similarly. > > I'll put it on the todo list, or patches accepted. Could you propose command line option name? I guess the main problem is to avoid collision with OpenSSH client options. -- Regards, Konstantin From matt at ucc.asn.au Mon Nov 30 21:00:29 2015 From: matt at ucc.asn.au (Matt Johnston) Date: Mon, 30 Nov 2015 21:00:29 +0800 Subject: [PATCH] the '==' comparision operator is not defined by POSIX, use '=' instead in configure.ac In-Reply-To: <20151127084452.GA17880@jue.netz> References: <20151127084452.GA17880@jue.netz> Message-ID: <187FC9CB-072F-4EEF-82C6-402610758B6C@ucc.asn.au> Thanks, I've applied that. Cheers, Matt > On Fri 27/11/2015, at 4:44 pm, Juergen Daubert wrote: > > Hello, > > the '==' operator is not defined by POSIX and it's use breaks building > of dropbear with dash and possibly other POSIX shells. > > Greetings > Juergen > > > --- configure.ac.orig 2015-11-27 09:37:27.852925351 +0100 > +++ configure.ac 2015-11-27 09:38:16.271165234 +0100 > @@ -92,7 +92,7 @@ > found_crypt_func=here > ]) > AC_SUBST(CRYPTLIB) > -if test "t$found_crypt_func" == there; then > +if test "t$found_crypt_func" = there; then > AC_DEFINE(HAVE_CRYPT, 1, [crypt() function]) > fi > From matt at ucc.asn.au Mon Nov 30 21:18:58 2015 From: matt at ucc.asn.au (Matt Johnston) Date: Mon, 30 Nov 2015 21:18:58 +0800 Subject: Syslog logging in client In-Reply-To: <1049571448887114@web8j.yandex.ru> References: <1049571448887114@web8j.yandex.ru> Message-ID: <140BD2BF-7C78-4480-9D63-BBC2A65D6AFA@ucc.asn.au> > On Mon 30/11/2015, at 8:38 pm, Konstantin Tokarev wrote: > > Hi all, > > I think it would be useful to have syslog loggin in client when it is running in background (e.g., when -f option is used, or, like in my case, it is started from init system to create tunnel). > > Would such contribution be welcome? That sounds useful. Another related thing I've been meaning to add is "-P pidfile" for dbclient -f, so the client can be killed. (I'm not suggesting you work on that, just as a thought). Cheers, Matt From annulen at yandex.ru Mon Nov 30 21:21:29 2015 From: annulen at yandex.ru (Konstantin Tokarev) Date: Mon, 30 Nov 2015 16:21:29 +0300 Subject: Force dbclient to exit if remote forwarding request failed In-Reply-To: <1074051448887311@web8j.yandex.ru> References: <362051448882663@web29g.yandex.ru> <1074051448887311@web8j.yandex.ru> Message-ID: <1598101448889689@web2h.yandex.ru> 30.11.2015, 15:47, "Konstantin Tokarev" : > 30.11.2015, 15:39, "Matt Johnston" : >>> ??On Mon 30/11/2015, at 7:24 pm, Konstantin Tokarev wrote: >>> ??dbclient: Remote TCP forward request failed (port 3320 -> localhost:22) >>> >>> ??... >>> ??1. Is there any way to force dbclient to exit in this case? >>> ??2. If not, would it be reasonable feature to have in dbclient? >> >> ?Hi Konstantin, >> >> ?I can't think of a good way to force exit apart from grepping the command output for the error message. >> >> ?I'd like to see that as a feature - I was thinking about it a few days ago. I'd keep it under a command line flag for backwards compatibility. Failing to bind "-L" forward ports would also be handled similarly. >> >> ?I'll put it on the todo list, or patches accepted. > > Could you propose command line option name? I guess the main problem is to avoid collision with OpenSSH client options. Maybe its time to add -o and use OpenSSH-compatible -o ExitOnForwardFailure=yes? -- Regards, Konstantin From annulen at yandex.ru Mon Nov 30 21:24:52 2015 From: annulen at yandex.ru (Konstantin Tokarev) Date: Mon, 30 Nov 2015 16:24:52 +0300 Subject: Syslog logging in client In-Reply-To: <140BD2BF-7C78-4480-9D63-BBC2A65D6AFA@ucc.asn.au> References: <1049571448887114@web8j.yandex.ru> <140BD2BF-7C78-4480-9D63-BBC2A65D6AFA@ucc.asn.au> Message-ID: <1623011448889892@web2h.yandex.ru> 30.11.2015, 16:19, "Matt Johnston" : >> ?On Mon 30/11/2015, at 8:38 pm, Konstantin Tokarev wrote: >> >> ?Hi all, >> >> ?I think it would be useful to have syslog loggin in client when it is running in background (e.g., when -f option is used, or, like in my case, it is started from init system to create tunnel). >> >> ?Would such contribution be welcome? > > That sounds useful. Another related thing I've been meaning to add is "-P pidfile" for dbclient -f, so the client can be killed. (I'm not suggesting you work on that, just as a thought). I was missing this feature once, but now I also need to restart dbclient after exit, so it won't help anyway. -- Regards, Konstantin From matt at ucc.asn.au Mon Nov 30 21:58:39 2015 From: matt at ucc.asn.au (Matt Johnston) Date: Mon, 30 Nov 2015 21:58:39 +0800 Subject: Force dbclient to exit if remote forwarding request failed In-Reply-To: <1598101448889689@web2h.yandex.ru> References: <362051448882663@web29g.yandex.ru> <1074051448887311@web8j.yandex.ru> <1598101448889689@web2h.yandex.ru> Message-ID: <20151130135839.GA11465@ucc.gu.uwa.edu.au> On Mon, Nov 30, 2015 at 04:21:29PM +0300, Konstantin Tokarev wrote: > >> ?I'd like to see that as a feature - I was thinking about it a few days ago. I'd keep it under a command line flag for backwards compatibility. Failing to bind "-L" forward ports would also be handled similarly. > >> > > Could you propose command line option name? I guess the main problem is to avoid collision with OpenSSH client options. > Maybe its time to add -o and use OpenSSH-compatible -o ExitOnForwardFailure=yes? That does seem like it might the best option. As long as people don't expect all the OpenSSH -o names to start getting implemented! :) Cheers, Matt From annulen at yandex.ru Tue Dec 1 02:08:20 2015 From: annulen at yandex.ru (Konstantin Tokarev) Date: Mon, 30 Nov 2015 21:08:20 +0300 Subject: Force dbclient to exit if remote forwarding request failed In-Reply-To: <20151130135839.GA11465@ucc.gu.uwa.edu.au> References: <362051448882663@web29g.yandex.ru> <1074051448887311@web8j.yandex.ru> <1598101448889689@web2h.yandex.ru> <20151130135839.GA11465@ucc.gu.uwa.edu.au> Message-ID: <2738771448906900@web13h.yandex.ru> 30.11.2015, 16:58, "Matt Johnston" : > On Mon, Nov 30, 2015 at 04:21:29PM +0300, Konstantin Tokarev wrote: >> ?>> ?I'd like to see that as a feature - I was thinking about it a few days ago. I'd keep it under a command line flag for backwards compatibility. Failing to bind "-L" forward ports would also be handled similarly. >> ?>> >> ?> Could you propose command line option name? I guess the main problem is to avoid collision with OpenSSH client options. >> ?Maybe its time to add -o and use OpenSSH-compatible -o ExitOnForwardFailure=yes? > > That does seem like it might the best option. As long as > people don't expect all the OpenSSH -o names to start > getting implemented! :) Implemented in https://github.com/mkj/dropbear/pull/16 1. I'm not sure if "extended options" is the right term, as well as if help texts are written properly. 2. Should -o handling itself be opt-in feature? 3. Should netcat-like forwarding be affected by ExitOnForwardFailure? If so, where its failure should be handled? -- Regards, Konstantin From annulen at yandex.ru Wed Dec 2 03:10:44 2015 From: annulen at yandex.ru (Konstantin Tokarev) Date: Tue, 01 Dec 2015 22:10:44 +0300 Subject: Syslog logging in client In-Reply-To: <140BD2BF-7C78-4480-9D63-BBC2A65D6AFA@ucc.asn.au> References: <1049571448887114@web8j.yandex.ru> <140BD2BF-7C78-4480-9D63-BBC2A65D6AFA@ucc.asn.au> Message-ID: <2422871448997044@web25j.yandex.ru> 30.11.2015, 16:19, "Matt Johnston" : >> ?On Mon 30/11/2015, at 8:38 pm, Konstantin Tokarev wrote: >> >> ?Hi all, >> >> ?I think it would be useful to have syslog loggin in client when it is running in background (e.g., when -f option is used, or, like in my case, it is started from init system to create tunnel). >> >> ?Would such contribution be welcome? > > That sounds useful. Another related thing I've been meaning to add is "-P pidfile" for dbclient -f, so the client can be killed. (I'm not suggesting you work on that, just as a thought). Implemented here: https://github.com/mkj/dropbear/pull/18 However, I didn't add command line option. I thought it may be better to use -o, but it has 2 issues currently: * -o patch was not approved yet * There's no corresponding OpenSSH option, so it may require "vendor-prefixing", like DropbearUseSyslog or DbclientUseSyslog -- Regards, Konstantin From matt at ucc.asn.au Wed Dec 2 22:59:57 2015 From: matt at ucc.asn.au (Matt Johnston) Date: Wed, 2 Dec 2015 22:59:57 +0800 Subject: Syslog logging in client In-Reply-To: <2422871448997044@web25j.yandex.ru> References: <1049571448887114@web8j.yandex.ru> <140BD2BF-7C78-4480-9D63-BBC2A65D6AFA@ucc.asn.au> <2422871448997044@web25j.yandex.ru> Message-ID: <6A80C03D-408E-4FC8-8D4A-C87DE123F84F@ucc.asn.au> > On Wed 2/12/2015, at 3:10 am, Konstantin Tokarev wrote: > Implemented here: > https://github.com/mkj/dropbear/pull/18 > > However, I didn't add command line option. I thought it may be better to use -o, but it has 2 issues currently: > > * -o patch was not approved yet > * There's no corresponding OpenSSH option, so it may require "vendor-prefixing", like DropbearUseSyslog or DbclientUseSyslog Thanks, from a first look over that patch looks good. I think "-o usesyslog" is safe enough. I won't merge these new features until after 2015.71 in a day or so - bugfixes only for now. Cheers, Matt From matt at ucc.asn.au Thu Dec 3 21:37:26 2015 From: matt at ucc.asn.au (Matt Johnston) Date: Thu, 3 Dec 2015 21:37:26 +0800 Subject: Dropbear 2015.71 Message-ID: <20151203133726.GB11465@ucc.gu.uwa.edu.au> Hi all, Dropbear 2015.71 is available with fixes for a few recent problems. - Fix "bad buf_incrpos" when data is transferred, broke in 2015.69 - Fix crash on exit when -p address:port is used, broke in 2015.68 - Fix building with only ENABLE_CLI_REMOTETCPFWD given, patch from Konstantin Tokarev - Fix bad configure script test which didn't work with dash shell, patch from Juergen Daubert, broke in 2015.70 - Fix server race condition that could cause sessions to hang on exit, https://github.com/robotframework/SSHLibrary/issues/128 Downloads at https://matt.ucc.asn.au/dropbear/dropbear.html and https://dropbear.nl/mirror/ Cheers, Matt From annulen at yandex.ru Fri Dec 4 18:45:35 2015 From: annulen at yandex.ru (Konstantin Tokarev) Date: Fri, 04 Dec 2015 13:45:35 +0300 Subject: Feature request: disable X11 forwarding and SFTP via configure options Message-ID: <3470341449225935@web30h.yandex.ru> I believe that most of dropbear users run it on embedded systems, where these features are rarely needed, so it would be convenient not have to patch this things out of options.h -- Regards, Konstantin From mfwitten at gmail.com Tue Dec 8 06:42:59 2015 From: mfwitten at gmail.com (Michael Witten) Date: Mon, 07 Dec 2015 22:42:59 -0000 Subject: [PATCH 00/16] Improvements, mainly to user name handling and scp. Message-ID: User names have hitherto been handled neither consistently nor well; this series alleviates at least some of the issues. Fear not the long patch series! Most commits involve a fairly small number of changes; while I could have consolidated these changes into fewer commits, I think the series as a whole provides a better narration for what's going. Besides a few small improvements along the way, the main thrust is: * Removing user-name handling from `scp' (in favor of using the handling that is already present in `dropbear'/`dbclient'). * Lazily looking up the current user's name. * Removing unused code. Overall, 7 files were changed, with 37 insertions(+) and 158 deletions(-): cli-main.c | 2 +- cli-runopts.c | 32 ++++++-------- common-session.c | 1 + runopts.h | 2 +- scp.c | 125 ++++++++++--------------------------------------------- scpmisc.c | 31 +------------- scpmisc.h | 2 - This is the series: [01] scp: Insert comma into stderr message [02] scp: Have `fatal()' append a newline to the message [03] scp: only pass `-v' when DEBUG_TRACE is set [04] scp: `-l%s' -> `-l %s' [05] scp: const/static correctness improvements [06] scp: Introduce `get_user_name()' [07] scp: Use "unknown" when the user name cannot be retrieved [08] scp: style: simplify code by using a tertiary operator [09] scp: Use `get_user_name()' more often [10] scp: Simplify code now that the user name is never `NULL' [11] scp: Remove parsing of `[user@]host' [12] scp: Remove unused functions [13] scp: Remove `replacearg()' [14] runopts: Mark `*cli_runopts.own_user' as `const' [15] runopts: There's no reason to make a duplicate of "unknown" [16] runopts: Re-introduce the `get_user_name()' function from `scp' development Lastly, for convenience in reviewing the changes, here is the overall patch: --- a/cli-main.c +++ b/cli-main.c @@ -135,7 +135,7 @@ static void cli_proxy_cmd(int *sock_in, int *sock_out) { int ret; - fill_passwd(cli_opts.own_user); + fill_passwd(get_user_name()); ret = spawn_command(exec_proxy_cmd, cli_opts.proxycmd, sock_out, sock_in, NULL, NULL); --- a/cli-runopts.c +++ b/cli-runopts.c @@ -36,7 +36,6 @@ static void printhelp(); static void parse_hostname(const char* orighostarg); static void parse_multihop_hostname(const char* orighostarg, const char* argv0); -static void fill_own_user(); #ifdef ENABLE_CLI_PUBKEY_AUTH static void loadidentityfile(const char* filename, int warnfail); #endif @@ -102,6 +101,17 @@ } +const char *get_user_name() { + static const char *user_name = NULL; + + if (user_name == NULL) { + struct passwd *pwd = getpwuid(getuid()); + user_name = pwd ? m_strdup(pwd->pw_name) : "unknown"; + } + + return user_name; +} + void cli_getopts(int argc, char ** argv) { unsigned int i, j; char ** next = 0; @@ -175,8 +185,6 @@ opts.keepalive_secs = DEFAULT_KEEPALIVE; opts.idle_timeout_secs = DEFAULT_IDLE_TIMEOUT; - fill_own_user(); - for (i = 1; i < (unsigned int)argc; i++) { /* Handle non-flag arguments such as hostname or commands for the remote host */ if (argv[i][0] != '-') @@ -640,7 +648,7 @@ } if (cli_opts.username == NULL) { - cli_opts.username = m_strdup(cli_opts.own_user); + cli_opts.username = m_strdup(get_user_name()); } port = strchr(cli_opts.remotehost, '^'); @@ -695,22 +703,6 @@ } #endif -static void fill_own_user() { - uid_t uid; - struct passwd *pw = NULL; - - uid = getuid(); - - pw = getpwuid(uid); - if (pw && pw->pw_name != NULL) { - cli_opts.own_user = m_strdup(pw->pw_name); - } else { - dropbear_log(LOG_INFO, "Warning: failed to identify current user. Trying anyway."); - cli_opts.own_user = m_strdup("unknown"); - } - -} - #ifdef ENABLE_CLI_ANYTCPFWD /* Turn a "[listenaddr:]listenport:remoteaddr:remoteport" string into into a forwarding * set, and add it to the forwarding list */ --- a/common-session.c +++ b/common-session.c @@ -581,6 +581,7 @@ return ses.authstate.pw_shell; } } + void fill_passwd(const char* username) { struct passwd *pw = NULL; if (ses.authstate.pw_name) --- a/runopts.h +++ b/runopts.h @@ -127,7 +127,6 @@ char *remotehost; char *remoteport; - char *own_user; char *username; char *cmd; @@ -165,6 +164,7 @@ extern cli_runopts cli_opts; void cli_getopts(int argc, char ** argv); +const char *get_user_name(); #ifdef ENABLE_USER_ALGO_LIST void parse_ciphers_macs(); --- a/scp.c +++ b/scp.c @@ -172,25 +172,21 @@ */ static void -arg_setup(char *host, char *remuser, char *cmd) +arg_setup(const char *user_at_host, const char *cmd) { - replacearg(&args, 0, "%s", ssh_program); - if (remuser != NULL) - addargs(&args, "-l%s", remuser); - addargs(&args, "%s", host); + addargs(&args, "%s", user_at_host); addargs(&args, "%s", cmd); } int -do_cmd(char *host, char *remuser, char *cmd, int *fdin, int *fdout, int argc) +do_cmd(const char *user_at_host, const char *cmd, int *fdin, int *fdout, int argc) { int pin[2], pout[2], reserved[2]; if (verbose_mode) fprintf(stderr, - "Executing: program %s host %s, user %s, command %s\n", - ssh_program, host, - remuser ? remuser : "(unspecified)", cmd); + "Executing: program %s, login %s, command %s\n", + ssh_program, user_at_host, cmd); /* * Reserve two descriptors so that the real pipes won't get @@ -211,7 +207,7 @@ /* uClinux needs to build the args here before vforking, otherwise we do it later on. */ #ifdef USE_VFORK - arg_setup(host, remuser, cmd); + arg_setup(user_at_host, cmd); #endif /* Fork a child to execute the command on the remote host using ssh. */ @@ -231,7 +227,7 @@ close(pout[1]); #ifndef USE_VFORK - arg_setup(host, remuser, cmd); + arg_setup(user_at_host, cmd); #endif execvp(ssh_program, args.list); @@ -251,16 +247,10 @@ xfree(args.list[args.num-1]); args.list[args.num-1]=NULL; args.num--; - /* pop host */ + /* pop user_at_host */ xfree(args.list[args.num-1]); args.list[args.num-1]=NULL; args.num--; - /* pop user */ - if (remuser != NULL) { - xfree(args.list[args.num-1]); - args.list[args.num-1]=NULL; - args.num--; - } #endif /* Parent. Close the other side, and return the local side. */ @@ -282,12 +272,9 @@ BUF *allocbuf(BUF *, int, int); void lostconn(int); void nospace(void); -int okname(char *); void run_err(const char *,...); void verifydir(char *); -struct passwd *pwd; -uid_t userid; int errs, remin, remout; int pflag, iamremote, iamrecursive, targetshouldbedirectory; @@ -362,7 +349,9 @@ ssh_program = xstrdup(optarg); break; case 'v': +#ifdef DEBUG_TRACE addargs(&args, "-v"); +#endif verbose_mode = 1; break; case 'q': @@ -393,9 +382,6 @@ argc -= optind; argv += optind; - if ((pwd = getpwuid(userid = getuid())) == NULL) - fatal("unknown user %u", (u_int) userid); - if (!isatty(STDERR_FILENO)) showprogress = 0; @@ -459,7 +445,8 @@ toremote(char *targ, int argc, char **argv) { int i, len; - char *bp, *host, *src, *suser, *thost, *tuser, *arg; + const char *targ_user_at_host; + char *bp, *src; arglist alist; memset(&alist, '\0', sizeof(alist)); @@ -469,30 +456,17 @@ if (*targ == 0) targ = "."; - arg = xstrdup(argv[argc - 1]); - if ((thost = strrchr(arg, '@'))) { - /* user at host */ - *thost++ = 0; - tuser = arg; - if (*tuser == '\0') - tuser = NULL; - } else { - thost = arg; - tuser = NULL; - } - - if (tuser != NULL && !okname(tuser)) { - xfree(arg); - return; - } + targ_user_at_host = argv[argc - 1]; for (i = 0; i < argc - 1; i++) { src = colon(argv[i]); if (src) { /* remote to remote */ freeargs(&alist); addargs(&alist, "%s", ssh_program); +#ifdef DEBUG_TRACE if (verbose_mode) addargs(&alist, "-v"); +#endif #if 0 /* Disabled since dbclient won't understand them and scp works fine without them. */ @@ -504,27 +478,11 @@ *src++ = 0; if (*src == 0) src = "."; - host = strrchr(argv[i], '@'); - - if (host) { - *host++ = 0; - host = cleanhostname(host); - suser = argv[i]; - if (*suser == '\0') - suser = pwd->pw_name; - else if (!okname(suser)) - continue; - addargs(&alist, "-l"); - addargs(&alist, "%s", suser); - } else { - host = cleanhostname(argv[i]); - } - addargs(&alist, "%s", host); + + addargs(&alist, "%s", argv[i]); /* [user@]host */ addargs(&alist, "%s", cmd); addargs(&alist, "%s", src); - addargs(&alist, "%s%s%s:%s", - tuser ? tuser : "", tuser ? "@" : "", - thost, targ); + addargs(&alist, "%s:%s", targ_user_at_host, targ); if (do_local_cmd(&alist) != 0) errs = 1; } else { /* local to remote */ @@ -532,8 +490,7 @@ len = strlen(targ) + CMDNEEDS + 20; bp = xmalloc(len); (void) snprintf(bp, len, "%s -t %s", cmd, targ); - host = cleanhostname(thost); - if (do_cmd(host, tuser, bp, &remin, + if (do_cmd(targ_user_at_host, bp, &remin, &remout, argc) < 0) exit(1); if (response() < 0) @@ -549,7 +506,7 @@ tolocal(int argc, char **argv) { int i, len; - char *bp, *host, *src, *suser; + char *bp, *src; arglist alist; memset(&alist, '\0', sizeof(alist)); @@ -572,20 +529,10 @@ *src++ = 0; if (*src == 0) src = "."; - if ((host = strrchr(argv[i], '@')) == NULL) { - host = argv[i]; - suser = NULL; - } else { - *host++ = 0; - suser = argv[i]; - if (*suser == '\0') - suser = pwd->pw_name; - } - host = cleanhostname(host); len = strlen(src) + CMDNEEDS + 20; bp = xmalloc(len); (void) snprintf(bp, len, "%s -f %s", cmd, src); - if (do_cmd(host, suser, bp, &remin, &remout, argc) < 0) { + if (do_cmd(argv[i], bp, &remin, &remout, argc) < 0) { (void) xfree(bp); ++errs; continue; @@ -1190,36 +1137,6 @@ killchild(0); } -int -okname(char *cp0) -{ - int c; - char *cp; - - cp = cp0; - do { - c = (int)*cp; - if (c & 0200) - goto bad; - if (!isalpha(c) && !isdigit(c)) { - switch (c) { - case '\'': - case '"': - case '`': - case ' ': - case '#': - goto bad; - default: - break; - } - } - } while (*++cp); - return (1); - -bad: fprintf(stderr, "%s: invalid user name\n", cp0); - return (0); -} - BUF * allocbuf(BUF *bp, int fd, int blksize) { --- a/scpmisc.c +++ b/scpmisc.c @@ -104,16 +104,6 @@ } char * -cleanhostname(char *host) -{ - if (*host == '[' && host[strlen(host) - 1] == ']') { - host[strlen(host) - 1] = '\0'; - return (host + 1); - } else - return host; -} - -char * colon(char *cp) { int flag = 0; @@ -165,26 +155,6 @@ } void -replacearg(arglist *args, u_int which, char *fmt, ...) -{ - va_list ap; - char *cp; - int r; - - va_start(ap, fmt); - r = vasprintf(&cp, fmt, ap); - va_end(ap); - if (r == -1) - fatal("replacearg: argument too long"); - - if (which >= args->num) - fatal("replacearg: tried to replace invalid arg %d >= %d", - which, args->num); - xfree(args->list[which]); - args->list[which] = cp; -} - -void freeargs(arglist *args) { u_int i; @@ -223,6 +193,7 @@ va_start(args, fmt); vfprintf(stderr, fmt, args); va_end(args); + fputc('\n', stderr); exit(255); } --- a/scpmisc.h +++ b/scpmisc.h @@ -21,7 +21,6 @@ void unset_nonblock(int); void set_nodelay(int); int a2port(const char *); -char *cleanhostname(char *); char *colon(char *); long convtime(const char *); @@ -34,7 +33,6 @@ int nalloc; }; void addargs(arglist *, char *, ...); -void replacearg(arglist *, u_int, char *, ...); void freeargs(arglist *); /* from xmalloc.h */ From mfwitten at gmail.com Tue Dec 8 06:49:35 2015 From: mfwitten at gmail.com (Michael Witten) Date: Mon, 07 Dec 2015 22:49:35 -0000 Subject: [PATCH 01/16] scp: Insert comma into stderr message In-Reply-To: References: Message-ID: Date: Wed, 4 Nov 2015 05:56:09 -0000 --- scp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scp.c b/scp.c index 70f45e3..b9aad3f 100644 --- a/scp.c +++ b/scp.c @@ -188,7 +188,7 @@ do_cmd(char *host, char *remuser, char *cmd, int *fdin, int *fdout, int argc) if (verbose_mode) fprintf(stderr, - "Executing: program %s host %s, user %s, command %s\n", + "Executing: program %s, host %s, user %s, command %s\n", ssh_program, host, remuser ? remuser : "(unspecified)", cmd); -- 2.4.3 From mfwitten at gmail.com Tue Dec 8 06:51:18 2015 From: mfwitten at gmail.com (Michael Witten) Date: Mon, 07 Dec 2015 22:51:18 -0000 Subject: [PATCH 02/16] scp: Have `fatal()' append a newline to the message In-Reply-To: References: Message-ID: Date: Wed, 4 Nov 2015 20:33:19 -0000 It would seem that it's standard practice not to include a newline in the message text, but that results in poor formatting, as a shell's command line then begins on the line of the error message itself. This commit simply instructs `fatal()' to append a newline after the message, which should be suitable behavior for all of the invocations I've come across. --- scpmisc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/scpmisc.c b/scpmisc.c index ec4df35..ea5b735 100644 --- a/scpmisc.c +++ b/scpmisc.c @@ -223,6 +223,7 @@ void fatal(char* fmt,...) va_start(args, fmt); vfprintf(stderr, fmt, args); va_end(args); + fputc('\n', stderr); exit(255); } -- 2.4.3 From mfwitten at gmail.com Tue Dec 8 06:52:29 2015 From: mfwitten at gmail.com (Michael Witten) Date: Mon, 07 Dec 2015 22:52:29 -0000 Subject: [PATCH 03/16] scp: pass `-v' only when DEBUG_TRACE is set In-Reply-To: References: Message-ID: <39a5ba9369eeeefe4b5a068816b8d0fdd6b6bd09.1449517677.git.mfwitten@gmail.com> Date: Wed, 4 Nov 2015 06:30:28 -0000 The program `dbclient' understands the option `-v' only when it is built with the macro DEBUG_TRACE defined; ergo, the program `scp' should pass `-v' to `dbclient' only when the macro DEBUG_TRACE is defined. --- scp.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/scp.c b/scp.c index b9aad3f..53789bf 100644 --- a/scp.c +++ b/scp.c @@ -362,7 +362,9 @@ main(int argc, char **argv) ssh_program = xstrdup(optarg); break; case 'v': +#ifdef DEBUG_TRACE addargs(&args, "-v"); +#endif verbose_mode = 1; break; case 'q': @@ -491,8 +493,10 @@ toremote(char *targ, int argc, char **argv) if (src) { /* remote to remote */ freeargs(&alist); addargs(&alist, "%s", ssh_program); +#ifdef DEBUG_TRACE if (verbose_mode) addargs(&alist, "-v"); +#endif #if 0 /* Disabled since dbclient won't understand them and scp works fine without them. */ -- 2.4.3 From mfwitten at gmail.com Tue Dec 8 06:53:50 2015 From: mfwitten at gmail.com (Michael Witten) Date: Mon, 07 Dec 2015 22:53:50 -0000 Subject: [PATCH 04/16] scp: `-l%s' -> `-l %s' In-Reply-To: References: Message-ID: <85b159ca64e1736587460eea339535decb8b0192.1449517677.git.mfwitten@gmail.com> Date: Wed, 4 Nov 2015 04:15:11 -0000 Options parsing apparently requires an option to be separate from its argument; in particular, the logic that disallows the combination of multiple options into one option also disallows the combination of an option and its argument into one option. See `cli-runopts.c:cli_getopts()' for details on option parsing. --- scp.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/scp.c b/scp.c index 53789bf..bb19c8b 100644 --- a/scp.c +++ b/scp.c @@ -175,8 +175,10 @@ static void arg_setup(char *host, char *remuser, char *cmd) { replacearg(&args, 0, "%s", ssh_program); - if (remuser != NULL) - addargs(&args, "-l%s", remuser); + if (remuser != NULL) { + addargs(&args, "-l"); + addargs(&args, "%s", remuser); + } addargs(&args, "%s", host); addargs(&args, "%s", cmd); } -- 2.4.3 From mfwitten at gmail.com Tue Dec 8 06:55:09 2015 From: mfwitten at gmail.com (Michael Witten) Date: Mon, 07 Dec 2015 22:55:09 -0000 Subject: [PATCH 05/16] scp: const/static correctness improvements In-Reply-To: References: Message-ID: Date: Wed, 4 Nov 2015 05:37:08 -0000 The variables `*suser' and `*tuser' should be of type `const char' rather than just `char'. Furthermore, the function `okname' should be declared as being `static'. While these changes could be applied to other identifiers, they are valuable in this commit as preparatory changes for a future commit. --- scp.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/scp.c b/scp.c index bb19c8b..9df8fa5 100644 --- a/scp.c +++ b/scp.c @@ -172,7 +172,7 @@ do_local_cmd(arglist *a) */ static void -arg_setup(char *host, char *remuser, char *cmd) +arg_setup(char *host, const char *remuser, char *cmd) { replacearg(&args, 0, "%s", ssh_program); if (remuser != NULL) { @@ -184,7 +184,7 @@ arg_setup(char *host, char *remuser, char *cmd) } int -do_cmd(char *host, char *remuser, char *cmd, int *fdin, int *fdout, int argc) +do_cmd(char *host, const char *remuser, char *cmd, int *fdin, int *fdout, int argc) { int pin[2], pout[2], reserved[2]; @@ -284,7 +284,7 @@ typedef struct { BUF *allocbuf(BUF *, int, int); void lostconn(int); void nospace(void); -int okname(char *); +static int okname(const char *); void run_err(const char *,...); void verifydir(char *); @@ -463,7 +463,8 @@ void toremote(char *targ, int argc, char **argv) { int i, len; - char *bp, *host, *src, *suser, *thost, *tuser, *arg; + const char *suser, *tuser; + char *bp, *host, *src, *thost, *arg; arglist alist; memset(&alist, '\0', sizeof(alist)); @@ -555,7 +556,8 @@ void tolocal(int argc, char **argv) { int i, len; - char *bp, *host, *src, *suser; + const char *suser; + char *bp, *host, *src; arglist alist; memset(&alist, '\0', sizeof(alist)); @@ -1196,11 +1198,11 @@ verifydir(char *cp) killchild(0); } -int -okname(char *cp0) +static int +okname(const char *cp0) { int c; - char *cp; + const char *cp; cp = cp0; do { -- 2.4.3 From mfwitten at gmail.com Tue Dec 8 06:56:06 2015 From: mfwitten at gmail.com (Michael Witten) Date: Mon, 07 Dec 2015 22:56:06 -0000 Subject: [PATCH 06/16] scp: Introduce `get_user_name()' In-Reply-To: References: Message-ID: <3cc06842ab1c2fbeb23c2f6f114f3f6985b24f3f.1449517677.git.mfwitten@gmail.com> Date: Wed, 4 Nov 2015 05:41:53 -0000 There's no reason to assume that there is a user name; userspace might be so primitive that there is no such thing, and `scp' should continue to run as long as the user explicitly specifies a user name on the command line. So, this commit introduces a new function for retrieving the user name. This function is called only when a user name is not specified by the user, in which case there MUST be a user name that can be retrieved from the current context of the program; if there is no such user name, then the user is notified of the error, and informed that a user name must be specified. --- scp.c | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/scp.c b/scp.c index 9df8fa5..dd49b68 100644 --- a/scp.c +++ b/scp.c @@ -288,8 +288,6 @@ static int okname(const char *); void run_err(const char *,...); void verifydir(char *); -struct passwd *pwd; -uid_t userid; int errs, remin, remout; int pflag, iamremote, iamrecursive, targetshouldbedirectory; @@ -303,6 +301,7 @@ void source(int, char *[]); void tolocal(int, char *[]); void toremote(char *, int, char *[]); void usage(void); +static const char *get_user_name(); #if defined(DBMULTI_scp) || !defined(DROPBEAR_MULTI) #if defined(DBMULTI_scp) && defined(DROPBEAR_MULTI) @@ -397,9 +396,6 @@ main(int argc, char **argv) argc -= optind; argv += optind; - if ((pwd = getpwuid(userid = getuid())) == NULL) - fatal("unknown user %u", (u_int) userid); - if (!isatty(STDERR_FILENO)) showprogress = 0; @@ -518,7 +514,7 @@ toremote(char *targ, int argc, char **argv) host = cleanhostname(host); suser = argv[i]; if (*suser == '\0') - suser = pwd->pw_name; + suser = get_user_name(); else if (!okname(suser)) continue; addargs(&alist, "-l"); @@ -587,7 +583,7 @@ tolocal(int argc, char **argv) *host++ = 0; suser = argv[i]; if (*suser == '\0') - suser = pwd->pw_name; + suser = get_user_name(); } host = cleanhostname(host); len = strlen(src) + CMDNEEDS + 20; @@ -1159,6 +1155,23 @@ usage(void) exit(1); } +static const char* +get_user_name() +{ + static const char *user_name = NULL; + + if (user_name == NULL) { + uid_t userid; + struct passwd *pwd; + if (pwd = getpwuid(userid = getuid())) + user_name = xstrdup(pwd->pw_name); + else + fatal("unknown user ID '%u'; specify a user name", (u_int) userid); + } + + return user_name; +} + void run_err(const char *fmt,...) { -- 2.4.3 From mfwitten at gmail.com Tue Dec 8 06:57:55 2015 From: mfwitten at gmail.com (Michael Witten) Date: Mon, 07 Dec 2015 22:57:55 -0000 Subject: [PATCH 08/16] scp: style: simplify code by using a tertiary operator In-Reply-To: References: Message-ID: <4e248609d2e35d6fd317729593aa9283090fecfb.1449517677.git.mfwitten@gmail.com> Date: Fri, 6 Nov 2015 20:05:54 -0000 --- scp.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/scp.c b/scp.c index dd92451..9a24490 100644 --- a/scp.c +++ b/scp.c @@ -1161,11 +1161,8 @@ get_user_name() static const char *user_name = NULL; if (user_name == NULL) { - struct passwd *pwd; - if (pwd = getpwuid(getuid())) - user_name = xstrdup(pwd->pw_name); - else - user_name = "unknown"; + struct passwd *pwd = getpwuid(getuid()); + user_name = pwd ? xstrdup(pwd->pw_name) : "unknown"; } return user_name; -- 2.4.3 From mfwitten at gmail.com Tue Dec 8 06:57:03 2015 From: mfwitten at gmail.com (Michael Witten) Date: Mon, 07 Dec 2015 22:57:03 -0000 Subject: [PATCH 07/16] scp: Use "unknown" when the user name cannot be retrieved In-Reply-To: References: Message-ID: <9028ef6e010d37de56edf7624f8af9ae77f57aed.1449517677.git.mfwitten@gmail.com> Date: Fri, 6 Nov 2015 19:12:49 -0000 It seems unnecessarily draconian to treat an unspecified user name as a fatal error; indeed, `dbclient' attempts to use "unknown" in this case, which is better than nothing, and similarly warns the user about the problem via the usual machinery. Furthermore, throwing out the error message saves bytes, which could be useful to embedded systems, where smaller is better. --- scp.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/scp.c b/scp.c index dd49b68..dd92451 100644 --- a/scp.c +++ b/scp.c @@ -1161,12 +1161,11 @@ get_user_name() static const char *user_name = NULL; if (user_name == NULL) { - uid_t userid; struct passwd *pwd; - if (pwd = getpwuid(userid = getuid())) + if (pwd = getpwuid(getuid())) user_name = xstrdup(pwd->pw_name); else - fatal("unknown user ID '%u'; specify a user name", (u_int) userid); + user_name = "unknown"; } return user_name; -- 2.4.3 From mfwitten at gmail.com Tue Dec 8 06:58:25 2015 From: mfwitten at gmail.com (Michael Witten) Date: Mon, 07 Dec 2015 22:58:25 -0000 Subject: [PATCH 09/16] scp: Use `get_user_name()' more often In-Reply-To: References: Message-ID: Date: Thu, 5 Nov 2015 07:13:01 -0000 Whenever the user name is not specified, try to use the current user's name. --- scp.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/scp.c b/scp.c index 9a24490..a19d28f 100644 --- a/scp.c +++ b/scp.c @@ -482,7 +482,9 @@ toremote(char *targ, int argc, char **argv) tuser = NULL; } - if (tuser != NULL && !okname(tuser)) { + if (tuser == NULL) + tuser = get_user_name(); + else if (!okname(tuser)) { xfree(arg); return; } @@ -514,14 +516,17 @@ toremote(char *targ, int argc, char **argv) host = cleanhostname(host); suser = argv[i]; if (*suser == '\0') - suser = get_user_name(); + suser = NULL; - else if (!okname(suser)) - continue; - addargs(&alist, "-l"); - addargs(&alist, "%s", suser); } else { host = cleanhostname(argv[i]); + suser = NULL; } + if (suser == NULL) + suser = get_user_name(); + else if (!okname(suser)) + continue; + addargs(&alist, "-l"); + addargs(&alist, "%s", suser); addargs(&alist, "%s", host); addargs(&alist, "%s", cmd); addargs(&alist, "%s", src); @@ -583,8 +588,12 @@ tolocal(int argc, char **argv) *host++ = 0; suser = argv[i]; if (*suser == '\0') - suser = get_user_name(); + suser = NULL; } + if (suser == NULL) + suser = get_user_name(); + else if (!okname(suser)) + continue; host = cleanhostname(host); len = strlen(src) + CMDNEEDS + 20; bp = xmalloc(len); -- 2.4.3 From mfwitten at gmail.com Tue Dec 8 06:59:12 2015 From: mfwitten at gmail.com (Michael Witten) Date: Mon, 07 Dec 2015 22:59:12 -0000 Subject: [PATCH 10/16] scp: Simplify code now that the user name is never `NULL' In-Reply-To: References: Message-ID: <586396c38032360fd30ec41286dbd38f66e3eb7f.1449517677.git.mfwitten@gmail.com> Date: Fri, 6 Nov 2015 23:13:25 -0000 The variables with the name `remuser' cannot hold the value `NULL'; this commit removes the code that handles that case. --- scp.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/scp.c b/scp.c index a19d28f..0adb590 100644 --- a/scp.c +++ b/scp.c @@ -175,10 +175,8 @@ static void arg_setup(char *host, const char *remuser, char *cmd) { replacearg(&args, 0, "%s", ssh_program); - if (remuser != NULL) { - addargs(&args, "-l"); - addargs(&args, "%s", remuser); - } + addargs(&args, "-l"); + addargs(&args, "%s", remuser); addargs(&args, "%s", host); addargs(&args, "%s", cmd); } @@ -192,7 +190,7 @@ do_cmd(char *host, const char *remuser, char *cmd, int *fdin, int *fdout, int ar fprintf(stderr, "Executing: program %s, host %s, user %s, command %s\n", ssh_program, host, - remuser ? remuser : "(unspecified)", cmd); + remuser, cmd); /* * Reserve two descriptors so that the real pipes won't get @@ -258,11 +256,9 @@ do_cmd(char *host, const char *remuser, char *cmd, int *fdin, int *fdout, int ar args.list[args.num-1]=NULL; args.num--; /* pop user */ - if (remuser != NULL) { - xfree(args.list[args.num-1]); - args.list[args.num-1]=NULL; - args.num--; - } + xfree(args.list[args.num-1]); + args.list[args.num-1]=NULL; + args.num--; #endif /* Parent. Close the other side, and return the local side. */ -- 2.4.3 From mfwitten at gmail.com Tue Dec 8 06:59:50 2015 From: mfwitten at gmail.com (Michael Witten) Date: Mon, 07 Dec 2015 22:59:50 -0000 Subject: [PATCH 11/16] scp: Remove parsing of `[user@]host' In-Reply-To: References: Message-ID: <21bab7461182b59428f69c6e5242a64451d844ef.1449517677.git.mfwitten@gmail.com> Date: Sat, 7 Nov 2015 03:27:36 -0000 The program `dbclient' can already handle parsing such arguments, including determining the current user's name. It should be noted that relying on this centralized machinery changes the behavior: Whereas before, `@host' (i.e., the empty string as the user name) prompted `scp' to substitute the current user's name, now the empty string is regarded as an acceptable name and passed through; this is the behavior that `dbclient' implements, and it might be useful to someone, anyway. --- scp.c | 93 +++++++++++++------------------------------------------------------ 1 file changed, 17 insertions(+), 76 deletions(-) diff --git a/scp.c b/scp.c index 0adb590..fc992b6 100644 --- a/scp.c +++ b/scp.c @@ -172,25 +172,22 @@ do_local_cmd(arglist *a) */ static void -arg_setup(char *host, const char *remuser, char *cmd) +arg_setup(const char *user_at_host, const char *cmd) { replacearg(&args, 0, "%s", ssh_program); - addargs(&args, "-l"); - addargs(&args, "%s", remuser); - addargs(&args, "%s", host); + addargs(&args, "%s", user_at_host); addargs(&args, "%s", cmd); } int -do_cmd(char *host, const char *remuser, char *cmd, int *fdin, int *fdout, int argc) +do_cmd(const char *user_at_host, const char *cmd, int *fdin, int *fdout, int argc) { int pin[2], pout[2], reserved[2]; if (verbose_mode) fprintf(stderr, - "Executing: program %s, host %s, user %s, command %s\n", - ssh_program, host, - remuser, cmd); + "Executing: program %s, login %s, command %s\n", + ssh_program, user_at_host, cmd); /* * Reserve two descriptors so that the real pipes won't get @@ -211,7 +208,7 @@ do_cmd(char *host, const char *remuser, char *cmd, int *fdin, int *fdout, int ar /* uClinux needs to build the args here before vforking, otherwise we do it later on. */ #ifdef USE_VFORK - arg_setup(host, remuser, cmd); + arg_setup(user_at_host, cmd); #endif /* Fork a child to execute the command on the remote host using ssh. */ @@ -231,7 +228,7 @@ do_cmd(char *host, const char *remuser, char *cmd, int *fdin, int *fdout, int ar close(pout[1]); #ifndef USE_VFORK - arg_setup(host, remuser, cmd); + arg_setup(user_at_host, cmd); #endif execvp(ssh_program, args.list); @@ -251,11 +248,7 @@ do_cmd(char *host, const char *remuser, char *cmd, int *fdin, int *fdout, int ar xfree(args.list[args.num-1]); args.list[args.num-1]=NULL; args.num--; - /* pop host */ - xfree(args.list[args.num-1]); - args.list[args.num-1]=NULL; - args.num--; - /* pop user */ + /* pop user_at_host */ xfree(args.list[args.num-1]); args.list[args.num-1]=NULL; args.num--; @@ -455,8 +448,8 @@ void toremote(char *targ, int argc, char **argv) { int i, len; - const char *suser, *tuser; + const char *targ_user_at_host; - char *bp, *host, *src, *thost, *arg; + char *bp, *src; arglist alist; memset(&alist, '\0', sizeof(alist)); @@ -466,24 +459,7 @@ toremote(char *targ, int argc, char **argv) if (*targ == 0) targ = "."; - arg = xstrdup(argv[argc - 1]); - if ((thost = strrchr(arg, '@'))) { - /* user at host */ - *thost++ = 0; - tuser = arg; - if (*tuser == '\0') - tuser = NULL; - } else { - thost = arg; - tuser = NULL; - } - - if (tuser == NULL) - tuser = get_user_name(); - else if (!okname(tuser)) { - xfree(arg); - return; - } + targ_user_at_host = argv[argc - 1]; for (i = 0; i < argc - 1; i++) { src = colon(argv[i]); @@ -505,30 +481,11 @@ toremote(char *targ, int argc, char **argv) *src++ = 0; if (*src == 0) src = "."; - host = strrchr(argv[i], '@'); - - if (host) { - *host++ = 0; - host = cleanhostname(host); - suser = argv[i]; - if (*suser == '\0') - suser = NULL; - } else { - host = cleanhostname(argv[i]); - suser = NULL; - } - if (suser == NULL) - suser = get_user_name(); - else if (!okname(suser)) - continue; - addargs(&alist, "-l"); - addargs(&alist, "%s", suser); - addargs(&alist, "%s", host); + + addargs(&alist, "%s", argv[i]); /* [user@]host */ addargs(&alist, "%s", cmd); addargs(&alist, "%s", src); - addargs(&alist, "%s%s%s:%s", - tuser ? tuser : "", tuser ? "@" : "", - thost, targ); + addargs(&alist, "%s:%s", targ_user_at_host, targ); if (do_local_cmd(&alist) != 0) errs = 1; } else { /* local to remote */ @@ -536,8 +493,7 @@ toremote(char *targ, int argc, char **argv) len = strlen(targ) + CMDNEEDS + 20; bp = xmalloc(len); (void) snprintf(bp, len, "%s -t %s", cmd, targ); - host = cleanhostname(thost); - if (do_cmd(host, tuser, bp, &remin, + if (do_cmd(targ_user_at_host, bp, &remin, &remout, argc) < 0) exit(1); if (response() < 0) @@ -553,8 +509,7 @@ void tolocal(int argc, char **argv) { int i, len; - const char *suser; - char *bp, *host, *src; + char *bp, *src; arglist alist; memset(&alist, '\0', sizeof(alist)); @@ -577,24 +532,10 @@ tolocal(int argc, char **argv) *src++ = 0; if (*src == 0) src = "."; - if ((host = strrchr(argv[i], '@')) == NULL) { - host = argv[i]; - suser = NULL; - } else { - *host++ = 0; - suser = argv[i]; - if (*suser == '\0') - suser = NULL; - } - if (suser == NULL) - suser = get_user_name(); - else if (!okname(suser)) - continue; - host = cleanhostname(host); len = strlen(src) + CMDNEEDS + 20; bp = xmalloc(len); (void) snprintf(bp, len, "%s -f %s", cmd, src); - if (do_cmd(host, suser, bp, &remin, &remout, argc) < 0) { + if (do_cmd(argv[i], bp, &remin, &remout, argc) < 0) { (void) xfree(bp); ++errs; continue; -- 2.4.3 From mfwitten at gmail.com Tue Dec 8 07:00:26 2015 From: mfwitten at gmail.com (Michael Witten) Date: Mon, 07 Dec 2015 23:00:26 -0000 Subject: [PATCH 12/16] scp: Remove unused functions In-Reply-To: References: Message-ID: <167158fdf3ca3a0c4564af79e984cae03c12ea7d.1449517677.git.mfwitten@gmail.com> Date: Sat, 7 Nov 2015 03:30:02 -0000 These functions were part of the machinery for parsing `[user@]host' arguments, which `scp' no longer handles. --- scp.c | 45 --------------------------------------------- scpmisc.c | 10 ---------- scpmisc.h | 1 - 3 files changed, 56 deletions(-) diff --git a/scp.c b/scp.c index fc992b6..9bf22e1 100644 --- a/scp.c +++ b/scp.c @@ -273,7 +273,6 @@ typedef struct { BUF *allocbuf(BUF *, int, int); void lostconn(int); void nospace(void); -static int okname(const char *); void run_err(const char *,...); void verifydir(char *); @@ -290,7 +289,6 @@ void source(int, char *[]); void tolocal(int, char *[]); void toremote(char *, int, char *[]); void usage(void); -static const char *get_user_name(); #if defined(DBMULTI_scp) || !defined(DROPBEAR_MULTI) #if defined(DBMULTI_scp) && defined(DROPBEAR_MULTI) @@ -1101,19 +1099,6 @@ usage(void) exit(1); } -static const char* -get_user_name() -{ - static const char *user_name = NULL; - - if (user_name == NULL) { - struct passwd *pwd = getpwuid(getuid()); - user_name = pwd ? xstrdup(pwd->pw_name) : "unknown"; - } - - return user_name; -} - void run_err(const char *fmt,...) { @@ -1153,36 +1138,6 @@ verifydir(char *cp) killchild(0); } -static int -okname(const char *cp0) -{ - int c; - const char *cp; - - cp = cp0; - do { - c = (int)*cp; - if (c & 0200) - goto bad; - if (!isalpha(c) && !isdigit(c)) { - switch (c) { - case '\'': - case '"': - case '`': - case ' ': - case '#': - goto bad; - default: - break; - } - } - } while (*++cp); - return (1); - -bad: fprintf(stderr, "%s: invalid user name\n", cp0); - return (0); -} - BUF * allocbuf(BUF *bp, int fd, int blksize) { diff --git a/scpmisc.c b/scpmisc.c index ea5b735..88a00ca 100644 --- a/scpmisc.c +++ b/scpmisc.c @@ -104,16 +104,6 @@ xstrdup(const char *str) } -char * -cleanhostname(char *host) -{ - if (*host == '[' && host[strlen(host) - 1] == ']') { - host[strlen(host) - 1] = '\0'; - return (host + 1); - } else - return host; -} - char * colon(char *cp) { int flag = 0; diff --git a/scpmisc.h b/scpmisc.h index 7d0b326..6978c66 100644 --- a/scpmisc.h +++ b/scpmisc.h @@ -21,7 +21,6 @@ void set_nonblock(int); void unset_nonblock(int); void set_nodelay(int); int a2port(const char *); -char *cleanhostname(char *); char *colon(char *); long convtime(const char *); -- 2.4.3 From mfwitten at gmail.com Tue Dec 8 07:00:56 2015 From: mfwitten at gmail.com (Michael Witten) Date: Mon, 07 Dec 2015 23:00:56 -0000 Subject: [PATCH 13/16] scp: Remove `replacearg()' In-Reply-To: References: Message-ID: Date: Sat, 7 Nov 2015 03:48:23 -0000 It seems that it has no purpose. --- scp.c | 1 - scpmisc.c | 20 -------------------- scpmisc.h | 1 - 3 files changed, 22 deletions(-) diff --git a/scp.c b/scp.c index 9bf22e1..599042c 100644 --- a/scp.c +++ b/scp.c @@ -174,7 +174,6 @@ do_local_cmd(arglist *a) static void arg_setup(const char *user_at_host, const char *cmd) { - replacearg(&args, 0, "%s", ssh_program); addargs(&args, "%s", user_at_host); addargs(&args, "%s", cmd); } diff --git a/scpmisc.c b/scpmisc.c index 88a00ca..ce27628 100644 --- a/scpmisc.c +++ b/scpmisc.c @@ -155,26 +155,6 @@ addargs(arglist *args, char *fmt, ...) } -void -replacearg(arglist *args, u_int which, char *fmt, ...) -{ - va_list ap; - char *cp; - int r; - - va_start(ap, fmt); - r = vasprintf(&cp, fmt, ap); - va_end(ap); - if (r == -1) - fatal("replacearg: argument too long"); - - if (which >= args->num) - fatal("replacearg: tried to replace invalid arg %d >= %d", - which, args->num); - xfree(args->list[which]); - args->list[which] = cp; -} - void freeargs(arglist *args) { u_int i; diff --git a/scpmisc.h b/scpmisc.h index 6978c66..2a18173 100644 --- a/scpmisc.h +++ b/scpmisc.h @@ -33,7 +33,6 @@ struct arglist { int nalloc; }; void addargs(arglist *, char *, ...); -void replacearg(arglist *, u_int, char *, ...); void freeargs(arglist *); /* from xmalloc.h */ -- 2.4.3 From mfwitten at gmail.com Tue Dec 8 07:01:30 2015 From: mfwitten at gmail.com (Michael Witten) Date: Mon, 07 Dec 2015 23:01:30 -0000 Subject: [PATCH 14/16] runopts: Mark `*cli_runopts.own_user' as `const' In-Reply-To: References: Message-ID: <130f83300b155c713cb9b72da2d831ffa4e5ece6.1449517677.git.mfwitten@gmail.com> Date: Sat, 7 Nov 2015 16:55:53 -0000 This is in preparation for treating `own_user' with less explicit safety. --- runopts.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runopts.h b/runopts.h index 7d6ae06..0a97937 100644 --- a/runopts.h +++ b/runopts.h @@ -127,7 +127,7 @@ typedef struct cli_runopts { char *remotehost; char *remoteport; - char *own_user; + const char *own_user; char *username; char *cmd; -- 2.4.3 From mfwitten at gmail.com Tue Dec 8 07:02:11 2015 From: mfwitten at gmail.com (Michael Witten) Date: Mon, 07 Dec 2015 23:02:11 -0000 Subject: [PATCH 15/16] runopts: There's no reason to make a duplicate of "unknown" In-Reply-To: References: Message-ID: <39e54385cecc9b288f7a07e25ec5c0def0a2374e.1449517677.git.mfwitten@gmail.com> Date: Sat, 7 Nov 2015 17:13:01 -0000 The variable `*cli_opts.own_user is of type `const char', and it doesn't seem to be freed anywhere. --- cli-runopts.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli-runopts.c b/cli-runopts.c index e8cb313..c4cd12c 100644 --- a/cli-runopts.c +++ b/cli-runopts.c @@ -706,7 +706,7 @@ static void fill_own_user() { cli_opts.own_user = m_strdup(pw->pw_name); } else { dropbear_log(LOG_INFO, "Warning: failed to identify current user. Trying anyway."); - cli_opts.own_user = m_strdup("unknown"); + cli_opts.own_user = "unknown"; } } -- 2.4.3 From mfwitten at gmail.com Tue Dec 8 07:03:01 2015 From: mfwitten at gmail.com (Michael Witten) Date: Mon, 07 Dec 2015 23:03:01 -0000 Subject: [PATCH 16/16] runopts: Re-introduce the `get_user_name()' function from `scp' development In-Reply-To: References: Message-ID: <14736da88b7d3161729b255ceea200bf6d7730b3.1449517677.git.mfwitten@gmail.com> Date: Sun, 8 Nov 2015 04:15:38 -0000 The command `scp' originally handled user name parsing on its own, including the retrieval of the current user's name when no user name is specified. The `get_user_name()' function was developed to retrieve the current user's name lazily (on demand), so as not to waste cycles when a user name is *explicitly* specified. Furthermore, there was a warning message (actually, a fatal error message) produced when it was impossible to retrieve the current user's name; this message was eventually stripped out because it was relatively useless and takes up space. Ultimately, though, all of the `[user@]host' parsing machinery was ripped out of `scp', because it is already handled by `dropbear/dbclient'. However, as it turns out, this centralized machinery could also benefit from the same changes that were made for `scp'; hence, this commit re-introduces the `get_user_name()' function (albeit for use by `dropbear/dbclient' instead), including the removal of the relatively useless warning message. This function has been added to `cli-runopts.c', because the current user's name represents part of the invocation context (i.e., part of the runtime options); by including the declaration in `runopts.h', it automatically becomes visible where it is needed. --- cli-main.c | 2 +- cli-runopts.c | 32 ++++++++++++-------------------- common-session.c | 1 + runopts.h | 2 +- 4 files changed, 15 insertions(+), 22 deletions(-) diff --git a/cli-main.c b/cli-main.c index c7c9035..a811312 100644 --- a/cli-main.c +++ b/cli-main.c @@ -135,7 +135,7 @@ static void exec_proxy_cmd(void *user_data_cmd) { static void cli_proxy_cmd(int *sock_in, int *sock_out) { int ret; - fill_passwd(cli_opts.own_user); + fill_passwd(get_user_name()); ret = spawn_command(exec_proxy_cmd, cli_opts.proxycmd, sock_out, sock_in, NULL, NULL); diff --git a/cli-runopts.c b/cli-runopts.c index c4cd12c..34cad00 100644 --- a/cli-runopts.c +++ b/cli-runopts.c @@ -36,7 +36,6 @@ cli_runopts cli_opts; /* GLOBAL */ static void printhelp(); static void parse_hostname(const char* orighostarg); static void parse_multihop_hostname(const char* orighostarg, const char* argv0); -static void fill_own_user(); #ifdef ENABLE_CLI_PUBKEY_AUTH static void loadidentityfile(const char* filename, int warnfail); #endif @@ -102,6 +101,17 @@ static void printhelp() { } +const char *get_user_name() { + static const char *user_name = NULL; + + if (user_name == NULL) { + struct passwd *pwd = getpwuid(getuid()); + user_name = pwd ? m_strdup(pwd->pw_name) : "unknown"; + } + + return user_name; +} + void cli_getopts(int argc, char ** argv) { unsigned int i, j; char ** next = 0; @@ -175,8 +185,6 @@ void cli_getopts(int argc, char ** argv) { opts.keepalive_secs = DEFAULT_KEEPALIVE; opts.idle_timeout_secs = DEFAULT_IDLE_TIMEOUT; - fill_own_user(); - for (i = 1; i < (unsigned int)argc; i++) { /* Handle non-flag arguments such as hostname or commands for the remote host */ if (argv[i][0] != '-') @@ -640,7 +648,7 @@ static void parse_hostname(const char* orighostarg) { } if (cli_opts.username == NULL) { - cli_opts.username = m_strdup(cli_opts.own_user); + cli_opts.username = m_strdup(get_user_name()); } port = strchr(cli_opts.remotehost, '^'); @@ -695,22 +703,6 @@ fail: } #endif -static void fill_own_user() { - uid_t uid; - struct passwd *pw = NULL; - - uid = getuid(); - - pw = getpwuid(uid); - if (pw && pw->pw_name != NULL) { - cli_opts.own_user = m_strdup(pw->pw_name); - } else { - dropbear_log(LOG_INFO, "Warning: failed to identify current user. Trying anyway."); - cli_opts.own_user = "unknown"; - } - -} - #ifdef ENABLE_CLI_ANYTCPFWD /* Turn a "[listenaddr:]listenport:remoteaddr:remoteport" string into into a forwarding * set, and add it to the forwarding list */ diff --git a/common-session.c b/common-session.c index 874d539..dd818b2 100644 --- a/common-session.c +++ b/common-session.c @@ -581,6 +581,7 @@ const char* get_user_shell() { return ses.authstate.pw_shell; } } + void fill_passwd(const char* username) { struct passwd *pw = NULL; if (ses.authstate.pw_name) diff --git a/runopts.h b/runopts.h index 0a97937..654d6d4 100644 --- a/runopts.h +++ b/runopts.h @@ -127,7 +127,6 @@ typedef struct cli_runopts { char *remotehost; char *remoteport; - const char *own_user; char *username; char *cmd; @@ -165,6 +164,7 @@ typedef struct cli_runopts { extern cli_runopts cli_opts; void cli_getopts(int argc, char ** argv); +const char *get_user_name(); #ifdef ENABLE_USER_ALGO_LIST void parse_ciphers_macs(); From annulen at yandex.ru Wed Dec 9 01:58:12 2015 From: annulen at yandex.ru (Konstantin Tokarev) Date: Tue, 08 Dec 2015 20:58:12 +0300 Subject: Prepending 'exec' to proxy command automatically Message-ID: <732151449597492@web3h.yandex.ru> If dbclient is given proxy command which does not start with exec, shell process is lingering alongside proxy command process. However, I've found that OpenSSH client (ProxyCommand option) prepends exec automatically. I think it would be convenient if dbclient added exec by default, but this will break if someone is already using explicit exec in -J argument. Note that OpenSSH client also breaks with exec. So options are: 1. Just prepend exec to all proxy commands and make too clever people fix their scripts. 2. Check if proxy command starts with exec, if not, prepend it. 3. Add separate -o ProxyCommand option prepending exec (it also should substitute %h, %p, and %r to be compatible with OpenSSH). Leave -J as is for compatibility but mark it as deprecated. What is the best way? -- Regards, Konstantin From annulen at yandex.ru Wed Dec 16 21:32:03 2015 From: annulen at yandex.ru (Konstantin Tokarev) Date: Wed, 16 Dec 2015 16:32:03 +0300 Subject: Prepending 'exec' to proxy command automatically In-Reply-To: <732151449597492@web3h.yandex.ru> References: <732151449597492@web3h.yandex.ru> Message-ID: <1951450272723@web21j.yandex.ru> 08.12.2015, 21:00, "Konstantin Tokarev" : > If dbclient is given proxy command which does not start with exec, shell process is lingering alongside proxy command process. However, I've found that OpenSSH client (ProxyCommand option) prepends exec automatically. > > I think it would be convenient if dbclient added exec by default, but this will break if someone is already using explicit exec in -J argument. Note that OpenSSH client also breaks with exec. > > So options are: > > 1. Just prepend exec to all proxy commands and make too clever people fix their scripts. > 2. Check if proxy command starts with exec, if not, prepend it. > 3. Add separate -o ProxyCommand option prepending exec (it also should substitute %h, %p, and %r to be compatible with OpenSSH). Leave -J as is for compatibility but mark it as deprecated. > > What is the best way? Matt, what do you think? -- Regards, Konstantin From matt at ucc.asn.au Fri Dec 18 21:24:11 2015 From: matt at ucc.asn.au (Matt Johnston) Date: Fri, 18 Dec 2015 21:24:11 +0800 Subject: Prepending 'exec' to proxy command automatically In-Reply-To: <1951450272723@web21j.yandex.ru> References: <732151449597492@web3h.yandex.ru> <1951450272723@web21j.yandex.ru> Message-ID: <20151218132410.GA27198@ucc.gu.uwa.edu.au> On Wed, Dec 16, 2015 at 04:32:03PM +0300, Konstantin Tokarev wrote: > 08.12.2015, 21:00, "Konstantin Tokarev" : > > If dbclient is given proxy command which does not start with exec, shell process is lingering alongside proxy command process. However, I've found that OpenSSH client (ProxyCommand option) prepends exec automatically. > > > > I think it would be convenient if dbclient added exec by default, but this will break if someone is already using explicit exec in -J argument. Note that OpenSSH client also breaks with exec. > > > > So options are: > > > > 1. Just prepend exec to all proxy commands and make too clever people fix their scripts. > > 2. Check if proxy command starts with exec, if not, prepend it. > > 3. Add separate -o ProxyCommand option prepending exec (it also should substitute %h, %p, and %r to be compatible with OpenSSH). Leave -J as is for compatibility but mark it as deprecated. > > > > What is the best way? Sorry, missed this mail originally. I think 1 is best, I've pushed that as https://secure.ucc.asn.au/hg/dropbear/rev/f7d565054e5f Interestingly bash and zsh seem to avoid the lingering shell themselves without "exec". dash works for testing. Cheers, Matt From annulen at yandex.ru Fri Dec 18 22:04:17 2015 From: annulen at yandex.ru (Konstantin Tokarev) Date: Fri, 18 Dec 2015 17:04:17 +0300 Subject: Prepending 'exec' to proxy command automatically In-Reply-To: <20151218132410.GA27198@ucc.gu.uwa.edu.au> References: <732151449597492@web3h.yandex.ru> <1951450272723@web21j.yandex.ru> <20151218132410.GA27198@ucc.gu.uwa.edu.au> Message-ID: <1320071450447457@web18j.yandex.ru> 18.12.2015, 16:24, "Matt Johnston" : > On Wed, Dec 16, 2015 at 04:32:03PM +0300, Konstantin Tokarev wrote: >> ?08.12.2015, 21:00, "Konstantin Tokarev" : >> ?> If dbclient is given proxy command which does not start with exec, shell process is lingering alongside proxy command process. However, I've found that OpenSSH client (ProxyCommand option) prepends exec automatically. >> ?> >> ?> I think it would be convenient if dbclient added exec by default, but this will break if someone is already using explicit exec in -J argument. Note that OpenSSH client also breaks with exec. >> ?> >> ?> So options are: >> ?> >> ?> 1. Just prepend exec to all proxy commands and make too clever people fix their scripts. >> ?> 2. Check if proxy command starts with exec, if not, prepend it. >> ?> 3. Add separate -o ProxyCommand option prepending exec (it also should substitute %h, %p, and %r to be compatible with OpenSSH). Leave -J as is for compatibility but mark it as deprecated. >> ?> >> ?> What is the best way? > > Sorry, missed this mail originally. I think 1 is best, I've > pushed that as > https://secure.ucc.asn.au/hg/dropbear/rev/f7d565054e5f Why not use asprintf? I see scpmisc.c already uses vasprintf. > Interestingly bash and zsh seem to avoid the lingering shell > themselves without "exec". dash works for testing. I'm using busybox shell (and I guess it's the most popular shell choice among dropbear users ;) -- Regards, Konstantin From matt at ucc.asn.au Fri Dec 18 22:59:37 2015 From: matt at ucc.asn.au (Matt Johnston) Date: Fri, 18 Dec 2015 22:59:37 +0800 Subject: Prepending 'exec' to proxy command automatically In-Reply-To: <1320071450447457@web18j.yandex.ru> References: <732151449597492@web3h.yandex.ru> <1951450272723@web21j.yandex.ru> <20151218132410.GA27198@ucc.gu.uwa.edu.au> <1320071450447457@web18j.yandex.ru> Message-ID: <6BCBB9E0-DF35-4F8F-9FED-B4B592239A55@ucc.asn.au> On Fri 18/12/2015, at 10:04 pm, Konstantin Tokarev wrote: >> https://secure.ucc.asn.au/hg/dropbear/rev/f7d565054e5f > > Why not use asprintf? I see scpmisc.c already uses vasprintf. I suspect it wouldn't work on some of the more uncommon platforms where people run Dropbear (I guess they don't run scp). malloc() is cheap enough given it's forking anyway. > I'm using busybox shell (and I guess it's the most popular shell choice among dropbear users ;) Yep. I was just a bit puzzled when I first tried in my default shell. I wonder how much memory is wasted on embedded systems because they don't have that auto-exec trick... Matt From annulen at yandex.ru Wed Dec 23 18:48:39 2015 From: annulen at yandex.ru (Konstantin Tokarev) Date: Wed, 23 Dec 2015 13:48:39 +0300 Subject: [PATCH 00/16] Improvements, mainly to user name handling and scp. In-Reply-To: References: Message-ID: <3582821450867719@web5o.yandex.ru> 08.12.2015, 01:48, "Michael Witten" : > User names have hitherto been handled neither consistently nor well; this > series alleviates at least some of the issues. > > Fear not the long patch series! > > Most commits involve a fairly small number of changes; while I could have > consolidated these changes into fewer commits, I think the series as a whole > provides a better narration for what's going. > > Besides a few small improvements along the way, the main thrust is: > > ??* Removing user-name handling from `scp' (in favor of using the > ????handling that is already present in `dropbear'/`dbclient'). > > ??* Lazily looking up the current user's name. > > ??* Removing unused code. > > Overall, 7 files were changed, with 37 insertions(+) and 158 deletions(-): > > ??cli-main.c | 2 +- > ??cli-runopts.c | 32 ++++++-------- > ??common-session.c | 1 + > ??runopts.h | 2 +- > ??scp.c | 125 ++++++++++--------------------------------------------- > ??scpmisc.c | 31 +------------- > ??scpmisc.h | 2 - As can be seen from copyright header and git log, scp.c is a copy of corresponding file from OpenSSH (currently 4.3p2) with a few local changes. It might be a better idea to synchronize with upstream than to diverge it more. Just my 2 cents. > > This is the series: > > ??[01] scp: Insert comma into stderr message > ??[02] scp: Have `fatal()' append a newline to the message > ??[03] scp: only pass `-v' when DEBUG_TRACE is set > ??[04] scp: `-l%s' -> `-l %s' > ??[05] scp: const/static correctness improvements > ??[06] scp: Introduce `get_user_name()' > ??[07] scp: Use "unknown" when the user name cannot be retrieved > ??[08] scp: style: simplify code by using a tertiary operator > ??[09] scp: Use `get_user_name()' more often > ??[10] scp: Simplify code now that the user name is never `NULL' > ??[11] scp: Remove parsing of `[user@]host' > ??[12] scp: Remove unused functions > ??[13] scp: Remove `replacearg()' > ??[14] runopts: Mark `*cli_runopts.own_user' as `const' > ??[15] runopts: There's no reason to make a duplicate of "unknown" > ??[16] runopts: Re-introduce the `get_user_name()' function from `scp' development > > Lastly, for convenience in reviewing the changes, here is the overall patch: > > --- a/cli-main.c > +++ b/cli-main.c > @@ -135,7 +135,7 @@ > ?static void cli_proxy_cmd(int *sock_in, int *sock_out) { > ?????????int ret; > > - fill_passwd(cli_opts.own_user); > + fill_passwd(get_user_name()); > > ?????????ret = spawn_command(exec_proxy_cmd, cli_opts.proxycmd, > ?????????????????????????sock_out, sock_in, NULL, NULL); > --- a/cli-runopts.c > +++ b/cli-runopts.c > @@ -36,7 +36,6 @@ > ?static void printhelp(); > ?static void parse_hostname(const char* orighostarg); > ?static void parse_multihop_hostname(const char* orighostarg, const char* argv0); > -static void fill_own_user(); > ?#ifdef ENABLE_CLI_PUBKEY_AUTH > ?static void loadidentityfile(const char* filename, int warnfail); > ?#endif > @@ -102,6 +101,17 @@ > > ?} > > +const char *get_user_name() { > + static const char *user_name = NULL; > + > + if (user_name == NULL) { > + struct passwd *pwd = getpwuid(getuid()); > + user_name = pwd ? m_strdup(pwd->pw_name) : "unknown"; > + } > + > + return user_name; > +} > + > ?void cli_getopts(int argc, char ** argv) { > ?????????unsigned int i, j; > ?????????char ** next = 0; > @@ -175,8 +185,6 @@ > ?????????opts.keepalive_secs = DEFAULT_KEEPALIVE; > ?????????opts.idle_timeout_secs = DEFAULT_IDLE_TIMEOUT; > > - fill_own_user(); > - > ?????????for (i = 1; i < (unsigned int)argc; i++) { > ?????????????????/* Handle non-flag arguments such as hostname or commands for the remote host */ > ?????????????????if (argv[i][0] != '-') > @@ -640,7 +648,7 @@ > ?????????} > > ?????????if (cli_opts.username == NULL) { > - cli_opts.username = m_strdup(cli_opts.own_user); > + cli_opts.username = m_strdup(get_user_name()); > ?????????} > > ?????????port = strchr(cli_opts.remotehost, '^'); > @@ -695,22 +703,6 @@ > ?} > ?#endif > > -static void fill_own_user() { > - uid_t uid; > - struct passwd *pw = NULL; > - > - uid = getuid(); > - > - pw = getpwuid(uid); > - if (pw && pw->pw_name != NULL) { > - cli_opts.own_user = m_strdup(pw->pw_name); > - } else { > - dropbear_log(LOG_INFO, "Warning: failed to identify current user. Trying anyway."); > - cli_opts.own_user = m_strdup("unknown"); > - } > - > -} > - > ?#ifdef ENABLE_CLI_ANYTCPFWD > ?/* Turn a "[listenaddr:]listenport:remoteaddr:remoteport" string into into a forwarding > ??* set, and add it to the forwarding list */ > --- a/common-session.c > +++ b/common-session.c > @@ -581,6 +581,7 @@ > ?????????????????return ses.authstate.pw_shell; > ?????????} > ?} > + > ?void fill_passwd(const char* username) { > ?????????struct passwd *pw = NULL; > ?????????if (ses.authstate.pw_name) > --- a/runopts.h > +++ b/runopts.h > @@ -127,7 +127,6 @@ > ?????????char *remotehost; > ?????????char *remoteport; > > - char *own_user; > ?????????char *username; > > ?????????char *cmd; > @@ -165,6 +164,7 @@ > > ?extern cli_runopts cli_opts; > ?void cli_getopts(int argc, char ** argv); > +const char *get_user_name(); > > ?#ifdef ENABLE_USER_ALGO_LIST > ?void parse_ciphers_macs(); > --- a/scp.c > +++ b/scp.c > @@ -172,25 +172,21 @@ > ??*/ > > ?static void > -arg_setup(char *host, char *remuser, char *cmd) > +arg_setup(const char *user_at_host, const char *cmd) > ?{ > - replacearg(&args, 0, "%s", ssh_program); > - if (remuser != NULL) > - addargs(&args, "-l%s", remuser); > - addargs(&args, "%s", host); > + addargs(&args, "%s", user_at_host); > ?????????addargs(&args, "%s", cmd); > ?} > > ?int > -do_cmd(char *host, char *remuser, char *cmd, int *fdin, int *fdout, int argc) > +do_cmd(const char *user_at_host, const char *cmd, int *fdin, int *fdout, int argc) > ?{ > ?????????int pin[2], pout[2], reserved[2]; > > ?????????if (verbose_mode) > ?????????????????fprintf(stderr, > - "Executing: program %s host %s, user %s, command %s\n", > - ssh_program, host, > - remuser ? remuser : "(unspecified)", cmd); > + "Executing: program %s, login %s, command %s\n", > + ssh_program, user_at_host, cmd); > > ?????????/* > ??????????* Reserve two descriptors so that the real pipes won't get > @@ -211,7 +207,7 @@ > ?????????/* uClinux needs to build the args here before vforking, > ????????????otherwise we do it later on. */ > ?#ifdef USE_VFORK > - arg_setup(host, remuser, cmd); > + arg_setup(user_at_host, cmd); > ?#endif > > ?????????/* Fork a child to execute the command on the remote host using ssh. */ > @@ -231,7 +227,7 @@ > ?????????????????close(pout[1]); > > ?#ifndef USE_VFORK > - arg_setup(host, remuser, cmd); > + arg_setup(user_at_host, cmd); > ?#endif > > ?????????????????execvp(ssh_program, args.list); > @@ -251,16 +247,10 @@ > ?????????xfree(args.list[args.num-1]); > ?????????args.list[args.num-1]=NULL; > ?????????args.num--; > - /* pop host */ > + /* pop user_at_host */ > ?????????xfree(args.list[args.num-1]); > ?????????args.list[args.num-1]=NULL; > ?????????args.num--; > - /* pop user */ > - if (remuser != NULL) { > - xfree(args.list[args.num-1]); > - args.list[args.num-1]=NULL; > - args.num--; > - } > ?#endif > > ?????????/* Parent. Close the other side, and return the local side. */ > @@ -282,12 +272,9 @@ > ?BUF *allocbuf(BUF *, int, int); > ?void lostconn(int); > ?void nospace(void); > -int okname(char *); > ?void run_err(const char *,...); > ?void verifydir(char *); > > -struct passwd *pwd; > -uid_t userid; > ?int errs, remin, remout; > ?int pflag, iamremote, iamrecursive, targetshouldbedirectory; > > @@ -362,7 +349,9 @@ > ?????????????????????????ssh_program = xstrdup(optarg); > ?????????????????????????break; > ?????????????????case 'v': > +#ifdef DEBUG_TRACE > ?????????????????????????addargs(&args, "-v"); > +#endif > ?????????????????????????verbose_mode = 1; > ?????????????????????????break; > ?????????????????case 'q': > @@ -393,9 +382,6 @@ > ?????????argc -= optind; > ?????????argv += optind; > > - if ((pwd = getpwuid(userid = getuid())) == NULL) > - fatal("unknown user %u", (u_int) userid); > - > ?????????if (!isatty(STDERR_FILENO)) > ?????????????????showprogress = 0; > > @@ -459,7 +445,8 @@ > ?toremote(char *targ, int argc, char **argv) > ?{ > ?????????int i, len; > - char *bp, *host, *src, *suser, *thost, *tuser, *arg; > + const char *targ_user_at_host; > + char *bp, *src; > ?????????arglist alist; > > ?????????memset(&alist, '\0', sizeof(alist)); > @@ -469,30 +456,17 @@ > ?????????if (*targ == 0) > ?????????????????targ = "."; > > - arg = xstrdup(argv[argc - 1]); > - if ((thost = strrchr(arg, '@'))) { > - /* user at host */ > - *thost++ = 0; > - tuser = arg; > - if (*tuser == '\0') > - tuser = NULL; > - } else { > - thost = arg; > - tuser = NULL; > - } > - > - if (tuser != NULL && !okname(tuser)) { > - xfree(arg); > - return; > - } > + targ_user_at_host = argv[argc - 1]; > > ?????????for (i = 0; i < argc - 1; i++) { > ?????????????????src = colon(argv[i]); > ?????????????????if (src) { /* remote to remote */ > ?????????????????????????freeargs(&alist); > ?????????????????????????addargs(&alist, "%s", ssh_program); > +#ifdef DEBUG_TRACE > ?????????????????????????if (verbose_mode) > ?????????????????????????????????addargs(&alist, "-v"); > +#endif > ?#if 0 > ?????????????????????????/* Disabled since dbclient won't understand them > ????????????????????????????and scp works fine without them. */ > @@ -504,27 +478,11 @@ > ?????????????????????????*src++ = 0; > ?????????????????????????if (*src == 0) > ?????????????????????????????????src = "."; > - host = strrchr(argv[i], '@'); > - > - if (host) { > - *host++ = 0; > - host = cleanhostname(host); > - suser = argv[i]; > - if (*suser == '\0') > - suser = pwd->pw_name; > - else if (!okname(suser)) > - continue; > - addargs(&alist, "-l"); > - addargs(&alist, "%s", suser); > - } else { > - host = cleanhostname(argv[i]); > - } > - addargs(&alist, "%s", host); > + > + addargs(&alist, "%s", argv[i]); /* [user@]host */ > ?????????????????????????addargs(&alist, "%s", cmd); > ?????????????????????????addargs(&alist, "%s", src); > - addargs(&alist, "%s%s%s:%s", > - tuser ? tuser : "", tuser ? "@" : "", > - thost, targ); > + addargs(&alist, "%s:%s", targ_user_at_host, targ); > ?????????????????????????if (do_local_cmd(&alist) != 0) > ?????????????????????????????????errs = 1; > ?????????????????} else { /* local to remote */ > @@ -532,8 +490,7 @@ > ?????????????????????????????????len = strlen(targ) + CMDNEEDS + 20; > ?????????????????????????????????bp = xmalloc(len); > ?????????????????????????????????(void) snprintf(bp, len, "%s -t %s", cmd, targ); > - host = cleanhostname(thost); > - if (do_cmd(host, tuser, bp, &remin, > + if (do_cmd(targ_user_at_host, bp, &remin, > ?????????????????????????????????????&remout, argc) < 0) > ?????????????????????????????????????????exit(1); > ?????????????????????????????????if (response() < 0) > @@ -549,7 +506,7 @@ > ?tolocal(int argc, char **argv) > ?{ > ?????????int i, len; > - char *bp, *host, *src, *suser; > + char *bp, *src; > ?????????arglist alist; > > ?????????memset(&alist, '\0', sizeof(alist)); > @@ -572,20 +529,10 @@ > ?????????????????*src++ = 0; > ?????????????????if (*src == 0) > ?????????????????????????src = "."; > - if ((host = strrchr(argv[i], '@')) == NULL) { > - host = argv[i]; > - suser = NULL; > - } else { > - *host++ = 0; > - suser = argv[i]; > - if (*suser == '\0') > - suser = pwd->pw_name; > - } > - host = cleanhostname(host); > ?????????????????len = strlen(src) + CMDNEEDS + 20; > ?????????????????bp = xmalloc(len); > ?????????????????(void) snprintf(bp, len, "%s -f %s", cmd, src); > - if (do_cmd(host, suser, bp, &remin, &remout, argc) < 0) { > + if (do_cmd(argv[i], bp, &remin, &remout, argc) < 0) { > ?????????????????????????(void) xfree(bp); > ?????????????????????????++errs; > ?????????????????????????continue; > @@ -1190,36 +1137,6 @@ > ?????????killchild(0); > ?} > > -int > -okname(char *cp0) > -{ > - int c; > - char *cp; > - > - cp = cp0; > - do { > - c = (int)*cp; > - if (c & 0200) > - goto bad; > - if (!isalpha(c) && !isdigit(c)) { > - switch (c) { > - case '\'': > - case '"': > - case '`': > - case ' ': > - case '#': > - goto bad; > - default: > - break; > - } > - } > - } while (*++cp); > - return (1); > - > -bad: fprintf(stderr, "%s: invalid user name\n", cp0); > - return (0); > -} > - > ?BUF * > ?allocbuf(BUF *bp, int fd, int blksize) > ?{ > --- a/scpmisc.c > +++ b/scpmisc.c > @@ -104,16 +104,6 @@ > ?} > > ?char * > -cleanhostname(char *host) > -{ > - if (*host == '[' && host[strlen(host) - 1] == ']') { > - host[strlen(host) - 1] = '\0'; > - return (host + 1); > - } else > - return host; > -} > - > -char * > ?colon(char *cp) > ?{ > ?????????int flag = 0; > @@ -165,26 +155,6 @@ > ?} > > ?void > -replacearg(arglist *args, u_int which, char *fmt, ...) > -{ > - va_list ap; > - char *cp; > - int r; > - > - va_start(ap, fmt); > - r = vasprintf(&cp, fmt, ap); > - va_end(ap); > - if (r == -1) > - fatal("replacearg: argument too long"); > - > - if (which >= args->num) > - fatal("replacearg: tried to replace invalid arg %d >= %d", > - which, args->num); > - xfree(args->list[which]); > - args->list[which] = cp; > -} > - > -void > ?freeargs(arglist *args) > ?{ > ?????????u_int i; > @@ -223,6 +193,7 @@ > ?????????va_start(args, fmt); > ?????????vfprintf(stderr, fmt, args); > ?????????va_end(args); > + fputc('\n', stderr); > ?????????exit(255); > ?} > > --- a/scpmisc.h > +++ b/scpmisc.h > @@ -21,7 +21,6 @@ > ?void unset_nonblock(int); > ?void set_nodelay(int); > ?int a2port(const char *); > -char *cleanhostname(char *); > ?char *colon(char *); > ?long convtime(const char *); > > @@ -34,7 +33,6 @@ > ?????????int nalloc; > ?}; > ?void addargs(arglist *, char *, ...); > -void replacearg(arglist *, u_int, char *, ...); > ?void freeargs(arglist *); > > ?/* from xmalloc.h */ -- Regards, Konstantin From sebastian.huber at embedded-brains.de Wed Dec 23 21:31:11 2015 From: sebastian.huber at embedded-brains.de (Sebastian Huber) Date: Wed, 23 Dec 2015 14:31:11 +0100 Subject: Single-address space, no processes? Message-ID: <567AA21F.7090703@embedded-brains.de> Hello, I would like to use SSH to do some remote administration for an application running with the RTEMS real-time operating system. In this environment we have no virtual memory and user and kernel space separation. So this is like an application running in kernel mode only. Is it in possible to use the dropbear SSH server in such an environment? A quick grep showed that fork() is used for example to spawn new processes. Would it be possible to spawn a new thread instead (e.g. via pthread_create())? -- Sebastian Huber, embedded brains GmbH Address : Dornierstr. 4, D-82178 Puchheim, Germany Phone : +49 89 189 47 41-16 Fax : +49 89 189 47 41-09 E-Mail : sebastian.huber at embedded-brains.de PGP : Public key available on request. Diese Nachricht ist keine gesch?ftliche Mitteilung im Sinne des EHUG. From annulen at yandex.ru Thu Dec 24 18:25:58 2015 From: annulen at yandex.ru (Konstantin Tokarev) Date: Thu, 24 Dec 2015 13:25:58 +0300 Subject: Single-address space, no processes? In-Reply-To: <567AA21F.7090703@embedded-brains.de> References: <567AA21F.7090703@embedded-brains.de> Message-ID: <1673491450952758@web4o.yandex.ru> 23.12.2015, 16:33, "Sebastian Huber" : > Hello, > > I would like to use SSH to do some remote administration for an > application running with the RTEMS real-time operating system. In this > environment we have no virtual memory and user and kernel space > separation. So this is like an application running in kernel mode only. > Is it in possible to use the dropbear SSH server in such an environment? > A quick grep showed that fork() is used for example to spawn new > processes. Would it be possible to spawn a new thread instead (e.g. via > pthread_create())? fork() is not a mandatory part of SSH server workflow, e.g. look at inetd mode. -- Regards, Konstantin From matt at ucc.asn.au Tue Dec 29 23:03:34 2015 From: matt at ucc.asn.au (Matt Johnston) Date: Tue, 29 Dec 2015 23:03:34 +0800 Subject: [PATCH 00/16] Improvements, mainly to user name handling and scp. In-Reply-To: References: Message-ID: <20151229150334.GB27198@ucc.gu.uwa.edu.au> Hi Michael, I think the general change of these patches makes sense (avoiding failure when a local user doesn't exist) but it needs to be more minimal. scp comes straight from OpenSSH with some small changes for uClinux etc. I've tried to avoid additional changes since it really needs updating to a more recent OpenSSH - the more changes, the larger that work is. Upstreaming it might be an option. Cheers, Matt On Mon, Dec 07, 2015 at 10:42:59PM -0000, Michael Witten wrote: > User names have hitherto been handled neither consistently nor well; this > series alleviates at least some of the issues. > > Fear not the long patch series! > > Most commits involve a fairly small number of changes; while I could have > consolidated these changes into fewer commits, I think the series as a whole > provides a better narration for what's going. > > Besides a few small improvements along the way, the main thrust is: > > * Removing user-name handling from `scp' (in favor of using the > handling that is already present in `dropbear'/`dbclient'). > > * Lazily looking up the current user's name. > > * Removing unused code. > > Overall, 7 files were changed, with 37 insertions(+) and 158 deletions(-): > > cli-main.c | 2 +- > cli-runopts.c | 32 ++++++-------- > common-session.c | 1 + > runopts.h | 2 +- > scp.c | 125 ++++++++++--------------------------------------------- > scpmisc.c | 31 +------------- > scpmisc.h | 2 - > > This is the series: > > [01] scp: Insert comma into stderr message > [02] scp: Have `fatal()' append a newline to the message > [03] scp: only pass `-v' when DEBUG_TRACE is set > [04] scp: `-l%s' -> `-l %s' > [05] scp: const/static correctness improvements > [06] scp: Introduce `get_user_name()' > [07] scp: Use "unknown" when the user name cannot be retrieved > [08] scp: style: simplify code by using a tertiary operator > [09] scp: Use `get_user_name()' more often > [10] scp: Simplify code now that the user name is never `NULL' > [11] scp: Remove parsing of `[user@]host' > [12] scp: Remove unused functions > [13] scp: Remove `replacearg()' > [14] runopts: Mark `*cli_runopts.own_user' as `const' > [15] runopts: There's no reason to make a duplicate of "unknown" > [16] runopts: Re-introduce the `get_user_name()' function from `scp' development > > Lastly, for convenience in reviewing the changes, here is the overall patch: > > --- a/cli-main.c > +++ b/cli-main.c > @@ -135,7 +135,7 @@ > static void cli_proxy_cmd(int *sock_in, int *sock_out) { > int ret; > > - fill_passwd(cli_opts.own_user); > + fill_passwd(get_user_name()); > > ret = spawn_command(exec_proxy_cmd, cli_opts.proxycmd, > sock_out, sock_in, NULL, NULL); > --- a/cli-runopts.c > +++ b/cli-runopts.c > @@ -36,7 +36,6 @@ > static void printhelp(); > static void parse_hostname(const char* orighostarg); > static void parse_multihop_hostname(const char* orighostarg, const char* argv0); > -static void fill_own_user(); > #ifdef ENABLE_CLI_PUBKEY_AUTH > static void loadidentityfile(const char* filename, int warnfail); > #endif > @@ -102,6 +101,17 @@ > > } > > +const char *get_user_name() { > + static const char *user_name = NULL; > + > + if (user_name == NULL) { > + struct passwd *pwd = getpwuid(getuid()); > + user_name = pwd ? m_strdup(pwd->pw_name) : "unknown"; > + } > + > + return user_name; > +} > + > void cli_getopts(int argc, char ** argv) { > unsigned int i, j; > char ** next = 0; > @@ -175,8 +185,6 @@ > opts.keepalive_secs = DEFAULT_KEEPALIVE; > opts.idle_timeout_secs = DEFAULT_IDLE_TIMEOUT; > > - fill_own_user(); > - > for (i = 1; i < (unsigned int)argc; i++) { > /* Handle non-flag arguments such as hostname or commands for the remote host */ > if (argv[i][0] != '-') > @@ -640,7 +648,7 @@ > } > > if (cli_opts.username == NULL) { > - cli_opts.username = m_strdup(cli_opts.own_user); > + cli_opts.username = m_strdup(get_user_name()); > } > > port = strchr(cli_opts.remotehost, '^'); > @@ -695,22 +703,6 @@ > } > #endif > > -static void fill_own_user() { > - uid_t uid; > - struct passwd *pw = NULL; > - > - uid = getuid(); > - > - pw = getpwuid(uid); > - if (pw && pw->pw_name != NULL) { > - cli_opts.own_user = m_strdup(pw->pw_name); > - } else { > - dropbear_log(LOG_INFO, "Warning: failed to identify current user. Trying anyway."); > - cli_opts.own_user = m_strdup("unknown"); > - } > - > -} > - > #ifdef ENABLE_CLI_ANYTCPFWD > /* Turn a "[listenaddr:]listenport:remoteaddr:remoteport" string into into a forwarding > * set, and add it to the forwarding list */ > --- a/common-session.c > +++ b/common-session.c > @@ -581,6 +581,7 @@ > return ses.authstate.pw_shell; > } > } > + > void fill_passwd(const char* username) { > struct passwd *pw = NULL; > if (ses.authstate.pw_name) > --- a/runopts.h > +++ b/runopts.h > @@ -127,7 +127,6 @@ > char *remotehost; > char *remoteport; > > - char *own_user; > char *username; > > char *cmd; > @@ -165,6 +164,7 @@ > > extern cli_runopts cli_opts; > void cli_getopts(int argc, char ** argv); > +const char *get_user_name(); > > #ifdef ENABLE_USER_ALGO_LIST > void parse_ciphers_macs(); > --- a/scp.c > +++ b/scp.c > @@ -172,25 +172,21 @@ > */ > > static void > -arg_setup(char *host, char *remuser, char *cmd) > +arg_setup(const char *user_at_host, const char *cmd) > { > - replacearg(&args, 0, "%s", ssh_program); > - if (remuser != NULL) > - addargs(&args, "-l%s", remuser); > - addargs(&args, "%s", host); > + addargs(&args, "%s", user_at_host); > addargs(&args, "%s", cmd); > } > > int > -do_cmd(char *host, char *remuser, char *cmd, int *fdin, int *fdout, int argc) > +do_cmd(const char *user_at_host, const char *cmd, int *fdin, int *fdout, int argc) > { > int pin[2], pout[2], reserved[2]; > > if (verbose_mode) > fprintf(stderr, > - "Executing: program %s host %s, user %s, command %s\n", > - ssh_program, host, > - remuser ? remuser : "(unspecified)", cmd); > + "Executing: program %s, login %s, command %s\n", > + ssh_program, user_at_host, cmd); > > /* > * Reserve two descriptors so that the real pipes won't get > @@ -211,7 +207,7 @@ > /* uClinux needs to build the args here before vforking, > otherwise we do it later on. */ > #ifdef USE_VFORK > - arg_setup(host, remuser, cmd); > + arg_setup(user_at_host, cmd); > #endif > > /* Fork a child to execute the command on the remote host using ssh. */ > @@ -231,7 +227,7 @@ > close(pout[1]); > > #ifndef USE_VFORK > - arg_setup(host, remuser, cmd); > + arg_setup(user_at_host, cmd); > #endif > > execvp(ssh_program, args.list); > @@ -251,16 +247,10 @@ > xfree(args.list[args.num-1]); > args.list[args.num-1]=NULL; > args.num--; > - /* pop host */ > + /* pop user_at_host */ > xfree(args.list[args.num-1]); > args.list[args.num-1]=NULL; > args.num--; > - /* pop user */ > - if (remuser != NULL) { > - xfree(args.list[args.num-1]); > - args.list[args.num-1]=NULL; > - args.num--; > - } > #endif > > /* Parent. Close the other side, and return the local side. */ > @@ -282,12 +272,9 @@ > BUF *allocbuf(BUF *, int, int); > void lostconn(int); > void nospace(void); > -int okname(char *); > void run_err(const char *,...); > void verifydir(char *); > > -struct passwd *pwd; > -uid_t userid; > int errs, remin, remout; > int pflag, iamremote, iamrecursive, targetshouldbedirectory; > > @@ -362,7 +349,9 @@ > ssh_program = xstrdup(optarg); > break; > case 'v': > +#ifdef DEBUG_TRACE > addargs(&args, "-v"); > +#endif > verbose_mode = 1; > break; > case 'q': > @@ -393,9 +382,6 @@ > argc -= optind; > argv += optind; > > - if ((pwd = getpwuid(userid = getuid())) == NULL) > - fatal("unknown user %u", (u_int) userid); > - > if (!isatty(STDERR_FILENO)) > showprogress = 0; > > @@ -459,7 +445,8 @@ > toremote(char *targ, int argc, char **argv) > { > int i, len; > - char *bp, *host, *src, *suser, *thost, *tuser, *arg; > + const char *targ_user_at_host; > + char *bp, *src; > arglist alist; > > memset(&alist, '\0', sizeof(alist)); > @@ -469,30 +456,17 @@ > if (*targ == 0) > targ = "."; > > - arg = xstrdup(argv[argc - 1]); > - if ((thost = strrchr(arg, '@'))) { > - /* user at host */ > - *thost++ = 0; > - tuser = arg; > - if (*tuser == '\0') > - tuser = NULL; > - } else { > - thost = arg; > - tuser = NULL; > - } > - > - if (tuser != NULL && !okname(tuser)) { > - xfree(arg); > - return; > - } > + targ_user_at_host = argv[argc - 1]; > > for (i = 0; i < argc - 1; i++) { > src = colon(argv[i]); > if (src) { /* remote to remote */ > freeargs(&alist); > addargs(&alist, "%s", ssh_program); > +#ifdef DEBUG_TRACE > if (verbose_mode) > addargs(&alist, "-v"); > +#endif > #if 0 > /* Disabled since dbclient won't understand them > and scp works fine without them. */ > @@ -504,27 +478,11 @@ > *src++ = 0; > if (*src == 0) > src = "."; > - host = strrchr(argv[i], '@'); > - > - if (host) { > - *host++ = 0; > - host = cleanhostname(host); > - suser = argv[i]; > - if (*suser == '\0') > - suser = pwd->pw_name; > - else if (!okname(suser)) > - continue; > - addargs(&alist, "-l"); > - addargs(&alist, "%s", suser); > - } else { > - host = cleanhostname(argv[i]); > - } > - addargs(&alist, "%s", host); > + > + addargs(&alist, "%s", argv[i]); /* [user@]host */ > addargs(&alist, "%s", cmd); > addargs(&alist, "%s", src); > - addargs(&alist, "%s%s%s:%s", > - tuser ? tuser : "", tuser ? "@" : "", > - thost, targ); > + addargs(&alist, "%s:%s", targ_user_at_host, targ); > if (do_local_cmd(&alist) != 0) > errs = 1; > } else { /* local to remote */ > @@ -532,8 +490,7 @@ > len = strlen(targ) + CMDNEEDS + 20; > bp = xmalloc(len); > (void) snprintf(bp, len, "%s -t %s", cmd, targ); > - host = cleanhostname(thost); > - if (do_cmd(host, tuser, bp, &remin, > + if (do_cmd(targ_user_at_host, bp, &remin, > &remout, argc) < 0) > exit(1); > if (response() < 0) > @@ -549,7 +506,7 @@ > tolocal(int argc, char **argv) > { > int i, len; > - char *bp, *host, *src, *suser; > + char *bp, *src; > arglist alist; > > memset(&alist, '\0', sizeof(alist)); > @@ -572,20 +529,10 @@ > *src++ = 0; > if (*src == 0) > src = "."; > - if ((host = strrchr(argv[i], '@')) == NULL) { > - host = argv[i]; > - suser = NULL; > - } else { > - *host++ = 0; > - suser = argv[i]; > - if (*suser == '\0') > - suser = pwd->pw_name; > - } > - host = cleanhostname(host); > len = strlen(src) + CMDNEEDS + 20; > bp = xmalloc(len); > (void) snprintf(bp, len, "%s -f %s", cmd, src); > - if (do_cmd(host, suser, bp, &remin, &remout, argc) < 0) { > + if (do_cmd(argv[i], bp, &remin, &remout, argc) < 0) { > (void) xfree(bp); > ++errs; > continue; > @@ -1190,36 +1137,6 @@ > killchild(0); > } > > -int > -okname(char *cp0) > -{ > - int c; > - char *cp; > - > - cp = cp0; > - do { > - c = (int)*cp; > - if (c & 0200) > - goto bad; > - if (!isalpha(c) && !isdigit(c)) { > - switch (c) { > - case '\'': > - case '"': > - case '`': > - case ' ': > - case '#': > - goto bad; > - default: > - break; > - } > - } > - } while (*++cp); > - return (1); > - > -bad: fprintf(stderr, "%s: invalid user name\n", cp0); > - return (0); > -} > - > BUF * > allocbuf(BUF *bp, int fd, int blksize) > { > --- a/scpmisc.c > +++ b/scpmisc.c > @@ -104,16 +104,6 @@ > } > > char * > -cleanhostname(char *host) > -{ > - if (*host == '[' && host[strlen(host) - 1] == ']') { > - host[strlen(host) - 1] = '\0'; > - return (host + 1); > - } else > - return host; > -} > - > -char * > colon(char *cp) > { > int flag = 0; > @@ -165,26 +155,6 @@ > } > > void > -replacearg(arglist *args, u_int which, char *fmt, ...) > -{ > - va_list ap; > - char *cp; > - int r; > - > - va_start(ap, fmt); > - r = vasprintf(&cp, fmt, ap); > - va_end(ap); > - if (r == -1) > - fatal("replacearg: argument too long"); > - > - if (which >= args->num) > - fatal("replacearg: tried to replace invalid arg %d >= %d", > - which, args->num); > - xfree(args->list[which]); > - args->list[which] = cp; > -} > - > -void > freeargs(arglist *args) > { > u_int i; > @@ -223,6 +193,7 @@ > va_start(args, fmt); > vfprintf(stderr, fmt, args); > va_end(args); > + fputc('\n', stderr); > exit(255); > } > > --- a/scpmisc.h > +++ b/scpmisc.h > @@ -21,7 +21,6 @@ > void unset_nonblock(int); > void set_nodelay(int); > int a2port(const char *); > -char *cleanhostname(char *); > char *colon(char *); > long convtime(const char *); > > @@ -34,7 +33,6 @@ > int nalloc; > }; > void addargs(arglist *, char *, ...); > -void replacearg(arglist *, u_int, char *, ...); > void freeargs(arglist *); > > /* from xmalloc.h */ From matt at ucc.asn.au Tue Dec 29 23:42:48 2015 From: matt at ucc.asn.au (Matt Johnston) Date: Tue, 29 Dec 2015 23:42:48 +0800 Subject: Single-address space, no processes? In-Reply-To: <567AA21F.7090703@embedded-brains.de> References: <567AA21F.7090703@embedded-brains.de> Message-ID: <82A00228-4A7F-4F22-95AD-7DAAF0B09B3F@ucc.asn.au> Hi Sebastian, Dropbear probably won't fit without some modifications, though likely wouldn't be that hard to get working with only a single connection at a time. There's a global state variable, and may be a few other assumptions that the vforked child is going to exec() (like uClinux). Recent releases have better memory cleanup on exit, though there may still be some circumstances where heap memory is leaked - the code was originally written assuming exit() would clean it up. Cheers, Matt > On Wed 23/12/2015, at 9:31 pm, Sebastian Huber wrote: > > Hello, > > I would like to use SSH to do some remote administration for an application running with the RTEMS real-time operating system. In this environment we have no virtual memory and user and kernel space separation. So this is like an application running in kernel mode only. Is it in possible to use the dropbear SSH server in such an environment? A quick grep showed that fork() is used for example to spawn new processes. Would it be possible to spawn a new thread instead (e.g. via pthread_create())? > > -- > Sebastian Huber, embedded brains GmbH > > Address : Dornierstr. 4, D-82178 Puchheim, Germany > Phone : +49 89 189 47 41-16 > Fax : +49 89 189 47 41-09 > E-Mail : sebastian.huber at embedded-brains.de > PGP : Public key available on request. > > Diese Nachricht ist keine gesch?ftliche Mitteilung im Sinne des EHUG. > From guilhem at fripost.org Wed Dec 30 10:50:34 2015 From: guilhem at fripost.org (Guilhem Moulin) Date: Wed, 30 Dec 2015 03:50:34 +0100 Subject: Syscall based entropy Message-ID: <20151230025034.GA27366@localhost.localdomain> Hi there, N. Heninger, Z. Durumeric, E. Wustrow and A. Halderman have shown [0] that a low entropy pool (for instance early in the boot process) during key generation can lead to predictable keys. (Worse, for DSA this can lead to the exposure of the private host key material, but dropbear now mitigates against this in dss.c:buf_put_dss_sign.) Even when strong host keys have been generated using a properly seeded entropy pool, I guess the early boot issue can bite again and yield predictable ephemeral keys used in the Diffie-Hellman key exchange. (This looks especially bad when the server is installed in the initramfs image to unlock the encrypted root partition for instance, as the server is started very early in the initramfs stage.) The problem arise because reading from /dev/urandom doesn't block when the entropy pool hasn't been properly initialized. As a workaround, dbrandom.c:seedrandom implements its own logic to try to properly seed the pool. I wonder if you have considered to use getrandom(2) [1] on Linux ?3.17? AFAICT it has the semantics one would naively expect from reading from /dev/urandom. I could provide with a patch if there is interest. (OpenBSD has its own getentropy() syscall, but I don't have access to an OpenBSD box though.) Cheers, -- Guilhem. [0] https://factorable.net/paper.html [1] http://man7.org/linux/man-pages/man2/getrandom.2.html https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=c6e9d6f38894798696f23c8084ca7edbf16ee895 -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available Url : http://lists.ucc.gu.uwa.edu.au/pipermail/dropbear/attachments/20151230/473e9112/attachment.sig From matt at ucc.asn.au Wed Dec 30 22:08:14 2015 From: matt at ucc.asn.au (Matt Johnston) Date: Wed, 30 Dec 2015 22:08:14 +0800 Subject: Syscall based entropy In-Reply-To: <20151230025034.GA27366@localhost.localdomain> References: <20151230025034.GA27366@localhost.localdomain> Message-ID: <20151230140814.GE27198@ucc.gu.uwa.edu.au> On Wed, Dec 30, 2015 at 03:50:34AM +0100, Guilhem Moulin wrote: > The problem arise because reading from /dev/urandom doesn't block when > the entropy pool hasn't been properly initialized. As a workaround, > dbrandom.c:seedrandom implements its own logic to try to properly seed > the pool. I wonder if you have considered to use getrandom(2) [1] on > Linux ?3.17? AFAICT it has the semantics one would naively expect from > reading from /dev/urandom. I could provide with a patch if there is > interest. (OpenBSD has its own getentropy() syscall, but I don't have > access to an OpenBSD box though.) Hi Guilhem, Using getrandom() is on my todo list - I'd be glad to take a patch. I think the best behaviour would be to call getrandom() on urandom with GRND_NONBLOCK in a loop printing a warning to dropbear_log() if it is blocking (not yet initialised) and keep waiting. dbrandom.c process_file() already has some logic like that for reading from /dev/[u]random with select(). The code would need to fallback if getrandom() isn't a valid syscall - perhaps it should just use syscall(), I'm not sure how widespread getrandom() support is in glibc/uclibc/musl - I'm sure there are systems with kernels newer than their libc. See the code for clock_gettime() in dbutil.c as a similar situation. Dropbear already feeds the private keys into the random pool, so the risk from a bad boot-time RNG is reduced, at leat for the server. The extra sources in seedrandom() are purely opportunistic - better than nothing, though really it would be best if /dev/urandom blocked at boot until it's seeded (like getrandom()). Cheers, Matt From guilhem at fripost.org Wed Dec 30 22:50:22 2015 From: guilhem at fripost.org (Guilhem Moulin) Date: Wed, 30 Dec 2015 15:50:22 +0100 Subject: Syscall based entropy In-Reply-To: <20151230140814.GE27198@ucc.gu.uwa.edu.au> References: <20151230025034.GA27366@localhost.localdomain> <20151230140814.GE27198@ucc.gu.uwa.edu.au> Message-ID: <20151230145022.GA28604@localhost.localdomain> On Wed, 30 Dec 2015 at 22:08:14 +0800, Matt Johnston wrote: > Using getrandom() is on my todo list - I'd be glad to take a > patch. Awesome! I most likely won't have time to work on this during the next couple of weeks, but I'll have a look at some point if you have not done so already ;-) > I think the best behaviour would be to call > getrandom() on urandom with GRND_NONBLOCK in a loop > printing a warning to dropbear_log() if it is blocking (not > yet initialised) and keep waiting. This is exactly what I've seen done elsewhere :-) I'm curious of the possibility of an infinite loop though, but there is only one way to find out how long one has to wait in practice ;-) I'm not familiar with how the kernel fills its entropy pool, but I would hope it can use TCP packets once network has been configured and a client tries to speak with the SSH port, even when there is nothing listening on that port yet. > The extra sources in seedrandom() are purely opportunistic - > better than nothing, though really it would be best if > /dev/urandom blocked at boot until it's seeded (like getrandom()). Yup -- Guilhem. -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available Url : http://lists.ucc.gu.uwa.edu.au/pipermail/dropbear/attachments/20151230/94eca91a/attachment.sig From nsz at port70.net Wed Dec 30 23:46:22 2015 From: nsz at port70.net (Szabolcs Nagy) Date: Wed, 30 Dec 2015 16:46:22 +0100 Subject: Syscall based entropy In-Reply-To: <20151230140814.GE27198@ucc.gu.uwa.edu.au> References: <20151230025034.GA27366@localhost.localdomain> <20151230140814.GE27198@ucc.gu.uwa.edu.au> Message-ID: <20151230154621.GZ23362@port70.net> * Matt Johnston [2015-12-30 22:08:14 +0800]: > patch. I think the best behaviour would be to call > getrandom() on urandom with GRND_NONBLOCK in a loop > printing a warning to dropbear_log() if it is blocking (not > yet initialised) and keep waiting. dbrandom.c process_file() > already has some logic like that for reading from > /dev/[u]random with select(). The code would need to > fallback if getrandom() isn't a valid syscall - perhaps it > should just use syscall(), I'm not sure how widespread > getrandom() support is in glibc/uclibc/musl - I'm sure there > are systems with kernels newer than their libc. there is no getrandom libc api yet (e.g. see libc-alpha discussions: https://sourceware.org/ml/libc-alpha/2015-11/msg00373.html ) and the syscall number is not yet allocated for some target archs (e.g. sh). using raw syscall(2) can be problematic (e.g. on x32) and it adds a kernel header dependency (for the flags). there should be both compile-time and runtime checks (ifdef SYS_getrandom and check for ENOSYS). so there are some caveats, but otherwise it would be a useful addition. > See the code for clock_gettime() in dbutil.c as a similar > situation. using SYS_clock_gettime is broken: it turns a performance critical vdso call into a slow syscall. if you want to support old glibc then just add -lrt if it is needed, but i think modern systems should not be pessimized. > Dropbear already feeds the private keys into the random > pool, so the risk from a bad boot-time RNG is reduced, at > leat for the server. > The extra sources in seedrandom() are purely opportunistic - > better than nothing, though really it would be best if > /dev/urandom blocked at boot until it's seeded (like getrandom()). > > Cheers, > Matt