janmojzis / tinyssh

TinySSH is small server (less than 100000 words of code)
Creative Commons Zero v1.0 Universal
1.41k stars 75 forks source link

Option to allow anonymous SSH sessions #31

Open 1Hyena opened 6 years ago

1Hyena commented 6 years ago

It's been half a year since I implemented an option to allow SSH guest logins for TinySSH. I created a pull request but it has still not received any reviews nor has it been merged.

Is there any reason for delaying this? It would be helpful to have at least a discussion regarding this enhancement. I would prefer not to create my own fork of TinySSH as a long term solution.

itdaniher commented 6 years ago

FWIW I'd also like to see this enhancement landed. I see plenty of possibilities for an easy to embed ssh-based transport layer.

hardfalcon commented 6 years ago

This is probably out of scope for TinySSH. Quote from https://tinyssh.org/:

Features

  • easy auditable - TinySSH has less than 100000 words of code
  • simple configuration - TinySSH can’t be misconfigured
  • limited amount of features - TinySSH doesn’t have features such: SSH1 protocol, compression, …
1Hyena commented 6 years ago

@hardfalcon have you looked at the code changes?

Still easily auditable as there were trivial changes.

Configuration is still simple.

Limited amount of features? You mean TinySSH should be deliberately crippled? If you look at the changes proposed in the referred pull request it becomes blatantly obvious that with 22 lines of added code the use cases for tinySSH have increased by 50%. Do we really need another competing fork of TinySSH just because of 22 lines of code that increase the usability of TinySSH significantly?

terefang commented 2 years ago

i need to add that i have a similar use case that needs exactly the features as present in the PR above

terefang commented 2 years ago

i have looked in details onto the PR and i think this needs further discussion and engineering work.

what i have seen so far and from my own, i have identified the following features that are related:

  1. the ability to have a fallback user if user lookup fails, but authentication via public key is still performed against this fallback users key
  2. the users are trusted to being those that is signaled in ssh, disabling authentication entirely, ie. no PK authentication
  3. acting as a simple secure transport wrapper for a shell executable (ie. user remains the startup user)
  4. a variation on point 1, the fallback user (only) has forced to run a shell executable. (authentication still need to happen)
  5. regardless which user is signaled in ssh, the fallback user is used for everything
  6. regardless which user is signaled in ssh, each forced to run a shell executable instead of the user shell (authentication still need to happen)

some of the above form their own use cases, some use cases are a combination of the above.

terefang commented 2 years ago

Use Case:

i think this is the intended use case of @1Hyena for the original PR (ie. combining 6 + 5 + 2)

reasoning:

examples:

terefang commented 2 years ago

Proposed Features

if you think that some of the features might compromise you secure by default policy, the features could be made optional by wrapping them in defines

mkrsn commented 2 years ago

Anything new here? I would also love to see this feature in TinySSH.

terefang commented 2 years ago

looking at some code of the various github forks, there are some good features that could be reintroduced into the code-base.

i really dont want to hard-fork the code base, as i have no time maintaining it

janmojzis commented 2 years ago

Proposed Features

  • ability to specify a fallback user or force a specific user (-g and -G ?)
  • ability to disable authentication for only the fallback case or entirely (-a and -A ?)
  • ability to specify an executable for the fallback case (-e ?)
  • ability to specify an executable for the regular case (-E ?)
  • ability to force an executable for all cases (-F ?)
  • include the ability/workaround to run /bin/login

if you think that some of the features might compromise you secure by default policy, the features could be made optional by wrapping them in defines

Hi, I don't agree that tinyssh should have a lot of fallback options. Tinyssh is not feature-rich software, it's minimalistic software.

I would like to implement this feature very close to @1Hyena's pull request.

terefang commented 2 years ago

hmm ... how about:

but questions remain, what to do with the user:

lacking --no-auth but having -e should execute the given command in place of the normal user-account shell.

in my opinion having both --no-auth and -e should execute as the current user only, so it could be run as non-root.

this behavior would be a good compromise to implementing too many options/features

janmojzis commented 2 years ago

pull request is here: https://github.com/janmojzis/tinyssh/pull/62 User who needs unauthorized access must:

... I think it's hard to make mistake

janmojzis commented 2 years ago

And #60 #62 merged. Can anyone independently confirm that it works as expected?

eventually edit/add real-word examples to the man page https://github.com/janmojzis/tinyssh/blob/master/man/tinysshnoneauthd.8

terefang commented 2 years ago

hello

looking at the code: packet_auth.c:73 you expect that the ssh client can signal "none" authentication.

i have applied the following patch to make it work:

--- tinyssh/packet_auth.c.old   2022-01-10 17:43:34.000000000 +0100
+++ tinyssh/packet_auth.c   2022-01-10 19:16:48.563463947 +0100
@@ -70,10 +70,6 @@
         pos = packetparser_uint32(b->buf, b->len, pos, &len);       /* publickey/password/hostbased/none */
         pos = packetparser_skip(b->buf, b->len, pos, len);

-        if (str_equaln((char *)b->buf + pos - len, len, "none")) {
-            /*
-            if auth. none is enabled get the user from UID
-            */
             if (flagnoneauth) {
                 struct passwd *pw;
                 pkname = "none";
@@ -83,7 +79,8 @@
                 b->len = 0; b->buf[0] = 0;
                 goto authorized;
             }
-        }
+
+        if (str_equaln((char *)b->buf + pos - len, len, "none")) pkname = "none";
         if (str_equaln((char *)b->buf + pos - len, len, "password")) pkname = "password";
         if (str_equaln((char *)b->buf + pos - len, len, "hostbased")) pkname = "hostbased";
         if (str_equaln((char *)b->buf + pos - len, len, "publickey")) {
janmojzis commented 2 years ago

I test it like this, and works:

git clone git@github.com:janmojzis/tinyssh.git
cd tinyssh
make
useradd tinysshnoneauth
mkdir -p /home/tinysshnoneauth/
tinysshd-makekey /home/tinysshnoneauth/sshkeydir
chown -R tinysshnoneauth /home/tinysshnoneauth/sshkeydir
envuidgid tinysshnoneauth tcpserver -UHRDl0 0 2222 build/bin/tinysshnoneauthd -vv -e 'cat /etc/motd' /home/tinysshnoneauth/sshkeydir &
$ ssh -o PreferredAuthentications=none -p 2222 abcdefg@127.0.0.1

The programs included with the Debian GNU/Linux system are free software;
the exact distribution terms for each program are described in the
individual files in /usr/share/doc/*/copyright.

Debian GNU/Linux comes with ABSOLUTELY NO WARRANTY, to the extent
permitted by applicable law.
Connection to 127.0.0.1 closed.
1Hyena commented 2 years ago

I tested like this and worked as expected, thanks!

Screenshot from 2022-01-11 10-44-05

1Hyena commented 2 years ago

For the record, one may want to execute tinyssh via firejail as it would provide additional protection against possible security holes inside of tinyssh itself. The command line would be something like this:

tcpserver -v -RHl0 0.0.0.0 7022 firejail --private --shell=none --private-bin=nc,bash /opt/tinysshnoneauthd -vv -e 'nc localhost 7777' /opt/keys

terefang commented 2 years ago
$ ssh -o PreferredAuthentications=none -p 2222 abcdefg@127.0.0.1

i have tested this with some graphical ssh clients and none of them has the ability to set this option.

my patch enables those users to still log in, not having to change their favorite clients.

this is not a matter of compliance to the protocol standard, but of tolerance of unfortunate implementations.

terefang commented 1 year ago

any updates ?

janmojzis commented 1 year ago

Hi, anonymous sessions merged a year ago https://github.com/janmojzis/tinyssh/releases/tag/20220222

terefang commented 6 days ago

hello

i have checked the latest git cde and it still needs my patch to function.

the behavior has not changed that "none" authethentication actually means: ignore any authentication at all and let them silently succeed if any

code from here https://github.com/janmojzis/tinyssh/blob/f863e60d93006019855a0431ea5cd71c8f1fc173/tinyssh/packet_auth.c#L73

needs this patch:

--- tinyssh/packet_auth.c.old   2022-01-10 17:43:34.000000000 +0100
+++ tinyssh/packet_auth.c   2022-01-10 19:16:48.563463947 +0100
@@ -70,10 +70,6 @@
         pos = packetparser_uint32(b->buf, b->len, pos, &len);       /* publickey/password/hostbased/none */
         pos = packetparser_skip(b->buf, b->len, pos, len);

-        if (str_equaln((char *)b->buf + pos - len, len, "none")) {
-            /*
-            if auth. none is enabled get the user from UID
-            */
             if (flagnoneauth) {
                 struct passwd *pw;
                 pkname = "none";
@@ -83,7 +79,8 @@
                 b->len = 0; b->buf[0] = 0;
                 goto authorized;
             }
-        }
+
+        if (str_equaln((char *)b->buf + pos - len, len, "none")) pkname = "none";
         if (str_equaln((char *)b->buf + pos - len, len, "password")) pkname = "password";
         if (str_equaln((char *)b->buf + pos - len, len, "hostbased")) pkname = "hostbased";
         if (str_equaln((char *)b->buf + pos - len, len, "publickey")) {