shadow-maint / shadow

Upstream shadow tree
Other
287 stars 227 forks source link

src/login.c, src/login_nopam.c: Remove support for remote login (telndetd; innetgr(3)) #1022

Closed alejandro-colomar closed 3 hours ago

alejandro-colomar commented 4 weeks ago

Revisions:

v1b - Reviewed-by: @dancrossnyc - Add @dancrossnyc 's comment to the commit message. () - Link to the PR. ``` $ git range-diff alx/master gh/telnetd telnetd 1: 337ad270 ! 1: 55c5a072 src/login.c: Remove support for telnetd in login(1), that is, remove the '-h' flag @@ Commit message Hardcode the hostname to "". + Dan Cross wrote: + > Note that this change means that comparisons against the hostname in + > login.access no longer apply for names other than whatever has been + > assigned to the local system. For the axspawn program from the AX.25 + > suite, this applies to the protocol as well. + + Link: + Reviewed-by: Dan Cross Signed-off-by: Alejandro Colomar ## src/login.c ## 2: 93c43b74 ! 2: e3a4c1ec src/login.c: The only reason is PW_LOGIN @@ Metadata ## Commit message ## src/login.c: The only reason is PW_LOGIN + Link: + Reviewed-by: Dan Cross Signed-off-by: Alejandro Colomar ## src/login.c ## 3: 2b8e3669 ! 3: cd4f2cfe lib/, src/login.c: hostname is always "" @@ Commit message This is true since a few commits ago we removed support for rlogind and telndetd in login(1), removing -r and -h. + Link: + Reviewed-by: Dan Cross Signed-off-by: Alejandro Colomar ## lib/log.c ## 4: 0d28804f ! 4: 7e80f6ba src/login_nopam.c: Remove support for innetgr(3) @@ Metadata ## Commit message ## src/login_nopam.c: Remove support for innetgr(3) + Link: + Reviewed-by: Dan Cross Signed-off-by: Alejandro Colomar ## configure.ac ## 5: 17dbf39b ! 5: 595c3f3b src/login_nopam.c: Remove support for network hostnames @@ Commit message We dropped support for remote login recently, so we should reject network hostnames. + Link: + Reviewed-by: Dan Cross Signed-off-by: Alejandro Colomar ## src/login_nopam.c ## ```
v1c - Rebase ``` $ git range-diff alx/master..gh/telnetd shadow/master..telnetd 1: 55c5a072 = 1: ddabc1e0 src/login.c: Remove support for telnetd in login(1), that is, remove the '-h' flag 2: e3a4c1ec = 2: 43b8d123 src/login.c: The only reason is PW_LOGIN 3: cd4f2cfe = 3: b112ffb6 lib/, src/login.c: hostname is always "" 4: 7e80f6ba = 4: 96754035 src/login_nopam.c: Remove support for innetgr(3) 5: 595c3f3b = 5: 7273d687 src/login_nopam.c: Remove support for network hostnames ```
v1d - Rebase ``` $ git range-diff master..gh/telnetd shadow/master..telnetd 1: ddabc1e0 = 1: 333092e6 src/login.c: Remove support for telnetd in login(1), that is, remove the '-h' flag 2: 43b8d123 = 2: c734b2e2 src/login.c: The only reason is PW_LOGIN 3: b112ffb6 ! 3: d1dbd06b lib/, src/login.c: hostname is always "" @@ lib/utmp.c: prepare_utmp(const char *name, const char *line, const char *host, -- if ( (NULL != host) -- && ('\0' != host[0])) { -- hostname = XMALLOC(strlen(host) + 1, char); -- strcpy (hostname, host); +- if (NULL != host && '\0' != host[0]) +- hostname = xstrdup(host); #if defined(HAVE_STRUCT_UTMPX_UT_HOST) -- } else if ( (NULL != ut) -- && ('\0' != ut->ut_host[0])) { -+ if (NULL != ut && '\0' != ut->ut_host[0]) { - hostname = XMALLOC(NITEMS(ut->ut_host) + 1, char); - ZUSTR2STP(hostname, ut->ut_host); --#endif - } -+#endif +- else if (NULL != ut && '\0' != ut->ut_host[0]) ++ if (NULL != ut && '\0' != ut->ut_host[0]) + hostname = XSTRNDUP(ut->ut_host); + #endif - if (strncmp(line, "/dev/", 5) == 0) { - line += 5; @@ lib/utmp.c: setutmp(struct utmpx *ut) 4: 96754035 = 4: 8a1d3f38 src/login_nopam.c: Remove support for innetgr(3) 5: 7273d687 = 5: 01a474c2 src/login_nopam.c: Remove support for network hostnames ```
v1e - Rebase ``` $ git range-diff master..gh/telnetd shadow/master..telnetd 1: 333092e6 = 1: 1dd08490 src/login.c: Remove support for telnetd in login(1), that is, remove the '-h' flag 2: c734b2e2 = 2: d7cfb4fe src/login.c: The only reason is PW_LOGIN 3: d1dbd06b = 3: 8776c239 lib/, src/login.c: hostname is always "" 4: 8a1d3f38 = 4: 61ba2e8b src/login_nopam.c: Remove support for innetgr(3) 5: 01a474c2 = 5: d1ac2677 src/login_nopam.c: Remove support for network hostnames ```
v1f - Rebase ``` $ git range-diff alx/master..gh/telnetd shadow/master..telnetd 1: 1dd08490 = 1: 3e61cc62 src/login.c: Remove support for telnetd in login(1), that is, remove the '-h' flag 2: d7cfb4fe = 2: 8f267484 src/login.c: The only reason is PW_LOGIN 3: 8776c239 = 3: 36c5b421 lib/, src/login.c: hostname is always "" 4: 61ba2e8b ! 4: 560c86c7 src/login_nopam.c: Remove support for innetgr(3) @@ src/login_nopam.c: static char *myhostname (void) /* user_match - match a username against one token */ static bool user_match (const char *tok, const char *string) @@ src/login_nopam.c: static bool user_match (const char *tok, const char *string) - *at = '\0'; + stpcpy(at, ""); return ( user_match (tok, string) && from_match (at + 1, myhostname ())); -#if HAVE_INNETGR 5: d1ac2677 ! 5: d72ab137 src/login_nopam.c: Remove support for network hostnames @@ src/login_nopam.c -#include /* for inet_ntoa() */ #include "sizeof.h" - + #include "string/strchr/strrspn.h" @@ src/login_nopam.c: static bool list_match (char *list, const char *item, bool (*match_fn) (const ch static bool user_match (const char *tok, const char *string); static bool from_match (const char *tok, const char *string); ```
v1g - Rebase ``` $ git range-diff gh/master..gh/telnetd shadow/master..telnetd 1: 3e61cc62 = 1: f1747f09 src/login.c: Remove support for telnetd in login(1), that is, remove the '-h' flag 2: 8f267484 = 2: 5c867dc4 src/login.c: The only reason is PW_LOGIN 3: 36c5b421 = 3: f7eed6e0 lib/, src/login.c: hostname is always "" 4: 560c86c7 = 4: 92a52cfa src/login_nopam.c: Remove support for innetgr(3) 5: d72ab137 = 5: 8c5bb461 src/login_nopam.c: Remove support for network hostnames ```
v1h - Rebase ``` $ git range-diff gh/master..gh/telnetd master..telnetd 1: f1747f09 = 1: 82cf81db src/login.c: Remove support for telnetd in login(1), that is, remove the '-h' flag 2: 5c867dc4 = 2: d00cdd1d src/login.c: The only reason is PW_LOGIN 3: f7eed6e0 = 3: 04eced12 lib/, src/login.c: hostname is always "" 4: 92a52cfa ! 4: a74714bc src/login_nopam.c: Remove support for innetgr(3) @@ src/login_nopam.c: static char *myhostname (void) /* user_match - match a username against one token */ static bool user_match (const char *tok, const char *string) @@ src/login_nopam.c: static bool user_match (const char *tok, const char *string) - stpcpy(at, ""); - return ( user_match (tok, string) - && from_match (at + 1, myhostname ())); + host = stpsep(tok + 1, "@"); /* split user@host pattern */ + if (host != NULL) { + return user_match(tok, string) && from_match(host, myhostname()); -#if HAVE_INNETGR - } else if (tok[0] == '@') { /* netgroup */ - return (netgroup_match (tok + 1, NULL, string)); 5: 8c5bb461 = 5: 58050db0 src/login_nopam.c: Remove support for network hostnames ```
v1i - Rebase ``` $ git range-diff master..gh/telnetd shadow/master..telnetd 1: 82cf81db = 1: afd8af13 src/login.c: Remove support for telnetd in login(1), that is, remove the '-h' flag 2: d00cdd1d = 2: 71215b6d src/login.c: The only reason is PW_LOGIN 3: 04eced12 = 3: 629c244a lib/, src/login.c: hostname is always "" 4: a74714bc = 4: 7a262ad8 src/login_nopam.c: Remove support for innetgr(3) 5: 58050db0 = 5: 4a9e3ce7 src/login_nopam.c: Remove support for network hostnames ```
alejandro-colomar commented 4 weeks ago

@ikerexxe I'm not sure how much dead code we can remove here. Please check thoroughly.

zeha commented 2 weeks ago

I'm all for cutting dead ends, but I'll note ax25-tools still passes -h to login. Maybe that is fine, I don't know.

alejandro-colomar commented 2 weeks ago

I'm all for cutting dead ends, but I'll note ax25-tools still passes -h to login. Maybe that is fine, I don't know.

Is that project alive? I've tried finding their source code, but:

$ apt-cache show ax25-tools | grep Homepage: | awk '{print $2}' | xargs wget
--2024-06-24 21:52:08--  http://www.linux-ax25.org/wiki/Main_Page
Resolving www.linux-ax25.org (www.linux-ax25.org)... failed: Name or service not known.
wget: unable to resolve host address ‘www.linux-ax25.org’

Then went to https://www.kernel.org/doc/html/latest/networking/ax25.html which has a link to the utils at https://linux-ax25.in-berlin.de/, which is also a dead link.

zeha commented 2 weeks ago

I'm all for cutting dead ends, but I'll note ax25-tools still passes -h to login. Maybe that is fine, I don't know.

Is that project alive?

Maybe? It seems to get new versions in Debian every so often

[..] Then went to https://www.kernel.org/doc/html/latest/networking/ax25.html which has a link to the utils at https://linux-ax25.in-berlin.de/, which is also a dead link.

https://linux-ax25.in-berlin.de/cgit/ax25-tools.git/ seems to load, if slowly.

alejandro-colomar commented 2 weeks ago

I'm all for cutting dead ends, but I'll note ax25-tools still passes -h to login. Maybe that is fine, I don't know.

Can you please point to the specific line of code?

alx@debian:/tmp/tmp/ax25-tools$ grep -rn 'login.*-h'
alx@debian:/tmp/tmp/ax25-tools$ 
zeha commented 2 weeks ago

I'm all for cutting dead ends, but I'll note ax25-tools still passes -h to login. Maybe that is fine, I don't know.

Can you please point to the specific line of code?

https://sources.debian.org/src/ax25-tools/0.0.10-rc5+git20230513+d3e6d4f-2/ax25/axspawn.c/?hl=64#L1837

alejandro-colomar commented 2 weeks ago

https://lore.kernel.org/linux-hams/916671b2-10a5-49f4-9a86-d2b31d5b961c@app.fastmail.com/T/

dancrossnyc commented 2 weeks ago

Oh! One other minor thing that occurred to me... axspawn plugs the radio protocol into the hostname field via -h, and it is conceivable that someone could be matching against that in the pre-PAM login config file; thus, a user might be allowed to login via AX.25 but not via NET/ROM or something like that. With this change, I believe that is no longer possible.

For the record, I do not believe that anyone is actually doing that, but it may be worth noting in the commit.

alejandro-colomar commented 2 weeks ago

Oh! One other minor thing that occurred to me... axspawn plugs the radio protocol into the hostname field via -h, and it is conceivable that someone could be matching against that in the pre-PAM login config file; thus, a user might be allowed to login via AX.25 but not via NET/ROM or something like that. With this change, I believe that is no longer possible.

For the record, I do not believe that anyone is actually doing that, but it may be worth noting in the commit.

Thanks! Since I don't know much about those protocols, I fear I won't be able to write something clear. Would you mind sending a proposal of text to go to the commit messages (and specify to which commits they apply)? You'll probably write something more useful than I could.

dancrossnyc commented 2 weeks ago

Oh! One other minor thing that occurred to me... axspawn plugs the radio protocol into the hostname field via -h, and it is conceivable that someone could be matching against that in the pre-PAM login config file; thus, a user might be allowed to login via AX.25 but not via NET/ROM or something like that. With this change, I believe that is no longer possible. For the record, I do not believe that anyone is actually doing that, but it may be worth noting in the commit.

Thanks! Since I don't know much about those protocols, I fear I won't be able to write something clear. Would you mind sending a proposal of text to go to the commit messages (and specify to which commits they apply)? You'll probably write something more useful than I could.

Maybe something like,

"Note that this change means that comparisons against the hostname in login.access no longer apply for names other than whatever has been assigned to the local system. For the axspawn program from the AX.25 suite, this applies to the protocol as well."

woelfisch commented 1 week ago

Why do these topics always come up when I have zero time to take care of it? Anyway:

Oh! One other minor thing that occurred to me... axspawn plugs the radio protocol into the hostname field via -h, and it is conceivable that someone could be matching against that in the pre-PAM login config file; thus, a user might be allowed to login via AX.25 but not via NET/ROM or something like that. With this change, I believe that is no longer possible.

That was the original intention back then IIRC, but I used to run plain AX.25 only and I honestly don't know who is still using axspawn these days. Back when it was written, cryptographically secured communication of any kind was not allowed to even secure the login process (in Germany at least), and besides, TCP/IP over 1200 bit/s radio connections is a PITA, so SSH was not possible in many cases. Also, automated ham radio stations rarely had access to the Internet back then. Looking at the code today it should probably be rewritten to utilize the PAM stack directly, but I don't have any AX.25 setup operational right now.

The current maintainer of ax25-tools is Thomas DL9SAU (@dl9sau here on Github), if you're dropping the "-h" flag with an argument from login he may want to know about it...

alejandro-colomar commented 1 week ago

Thanks @woelfisch !

Let's wait for @dl9sau to comment.

alejandro-colomar commented 3 hours ago

The current maintainer of ax25-tools is Thomas DL9SAU (@dl9sau here on Github), if you're dropping the "-h" flag with an argument from login he may want to know about it...

He NAKed the change. I'll close this PR. It seems this feature is still needed by some users.