[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Re: [PATCH 2/4] Revert 2aeb201 "libbb: new option FEATURE_ETC_SERVICES"
From:       Sergey Ponomarev <stokito () gmail ! com>
Date:       2021-09-09 22:09:19
Message-ID: CADR0UcXYUV3WxBG38Zet4kBLKnYDnjjfrZeFqNLX9Y63b+gvog () mail ! gmail ! com
[Download RAW message or body]

And nobody will use the FEATURE_ETC_SERVICES because it's useless and
potentially breaks clients.
FTP is always 21 no matter what is written in /etc/services. So let's
just inline it as it's made in GNU wget.
We have two cases:
1. "Well known port" i.e. that we know for sure in compile time FTP,
HTTP e.g. wget http://example.com
2. Port that may differ i.e. that comes from user input: inetd conf. A
user may input any port name or port number.
E.g. telnet alternate-http example.com http

This commit is about 1

On Thu, 9 Sept 2021 at 22:44, Denys Vlasenko <vda.linux@googlemail.com> wrote:
> 
> Why?
> Just do not select FEATURE_ETC_SERVICES.
> 
> On Wed, Aug 25, 2021 at 10:14 PM Sergey Ponomarev <stokito@gmail.com> wrote:
> > 
> > Don't lookup /etc/services for well known ports.
> > In the commit  was added a new function bb_lookup_std_port().
> > The function is in fact not needed and we may assume that it is always disabled.
> > Nobody changes ports in the /etc/services i.e. if we know a port it will always \
> > remain the same. The file can be used only by telnet client and /etc/inetd.conf \
> > configuration because they may use port names. You may want to change port \
> > numbers there only to configure a daemon. But this doesn't make sense because it \
> > will break a client when it will try to call an external daemon which remains on \
> > standard port. 
> > For example if you change http to 8080 to make httpd use the port then this will \
> > break wget if it tries to call an external http url. So clients like wget must \
> > not use port numbers from /etc/services. Even if we remove the lookup from \
> > clients we still can't change the port because inted and telnet may do need to \
> > make the lookup. I.e. they are clients which don't know for sure which port must \
> > be used. 
> > Signed-off-by: Sergey Ponomarev <stokito@gmail.com>
> > ---
> > include/libbb.h          |  5 -----
> > libbb/Config.src         | 12 ------------
> > networking/isrv_identd.c |  3 +--
> > networking/telnet.c      |  3 +--
> > networking/wget.c        |  8 ++++----
> > util-linux/rdate.c       |  2 +-
> > 6 files changed, 7 insertions(+), 26 deletions(-)
> > 
> > diff --git a/include/libbb.h b/include/libbb.h
> > index 7d6ab4a93..e000ed8b0 100644
> > --- a/include/libbb.h
> > +++ b/include/libbb.h
> > @@ -722,11 +722,6 @@ int setsockopt_bindtodevice(int fd, const char *iface) \
> > FAST_FUNC; int bb_getsockname(int sockfd, void *addr, socklen_t addrlen) \
> > FAST_FUNC; /* NB: returns port in host byte order */
> > unsigned bb_lookup_port(const char *port, const char *protocol, unsigned \
> >                 default_port) FAST_FUNC;
> > -#if ENABLE_FEATURE_ETC_SERVICES
> > -# define bb_lookup_std_port(portstr, protocol, portnum) bb_lookup_port(portstr, \
> >                 protocol, portnum)
> > -#else
> > -# define bb_lookup_std_port(portstr, protocol, portnum) (portnum)
> > -#endif
> > typedef struct len_and_sockaddr {
> > socklen_t len;
> > union {
> > diff --git a/libbb/Config.src b/libbb/Config.src
> > index f97de8ef7..886725704 100644
> > --- a/libbb/Config.src
> > +++ b/libbb/Config.src
> > @@ -76,18 +76,6 @@ config FEATURE_ETC_NETWORKS
> > a rarely used feature which allows you to use names
> > instead of IP/mask pairs in route command.
> > 
> > -config FEATURE_ETC_SERVICES
> > -       bool "Consult /etc/services even for well-known ports"
> > -       default n
> > -       help
> > -       Look up e.g. "telnet" and "http" in /etc/services file
> > -       instead of assuming ports 23 and 80.
> > -       This is almost never necessary (everybody uses standard ports),
> > -       and it makes sense to avoid reading this file.
> > -       If you disable this option, in the cases where port is explicitly
> > -       specified as a service name (e.g. "telnet HOST PORTNAME"),
> > -       it will still be looked up in /etc/services.
> > -
> > config FEATURE_EDITING
> > bool "Command line editing"
> > default y
> > diff --git a/networking/isrv_identd.c b/networking/isrv_identd.c
> > index f564d604a..5bacfde94 100644
> > --- a/networking/isrv_identd.c
> > +++ b/networking/isrv_identd.c
> > @@ -158,8 +158,7 @@ int fakeidentd_main(int argc UNUSED_PARAM, char **argv)
> > 
> > fd = 0;
> > if (!(opt & OPT_inetdwait)) {
> > -               fd = create_and_bind_stream_or_die(bind_address,
> > -                               bb_lookup_std_port("identd", "tcp", 113));
> > +               fd = create_and_bind_stream_or_die(bind_address, 113);
> > xlisten(fd, 5);
> > }
> > 
> > diff --git a/networking/telnet.c b/networking/telnet.c
> > index 7a0253525..dc088721b 100644
> > --- a/networking/telnet.c
> > +++ b/networking/telnet.c
> > @@ -649,8 +649,7 @@ int telnet_main(int argc UNUSED_PARAM, char **argv)
> > if (!*argv)
> > bb_show_usage();
> > host = *argv++;
> > -       port = *argv ? bb_lookup_port(*argv++, "tcp", 23)
> > -               : bb_lookup_std_port("telnet", "tcp", 23);
> > +       port = *argv ? bb_lookup_port(*argv++, "tcp", 23) : 23;
> > if (*argv) /* extra params?? */
> > bb_show_usage();
> > 
> > diff --git a/networking/wget.c b/networking/wget.c
> > index 6a9604421..46c470c02 100644
> > --- a/networking/wget.c
> > +++ b/networking/wget.c
> > @@ -534,19 +534,19 @@ static void parse_url(const char *src_url, struct host_info \
> > *h) h->host = p + 3;
> > #if ENABLE_FEATURE_WGET_FTP
> > if (strcmp(url, P_FTP) == 0) {
> > -                       h->port = bb_lookup_std_port(P_FTP, "tcp", 21);
> > +                       h->port = 21;
> > h->protocol = P_FTP;
> > } else
> > #endif
> > #if FTPS_SUPPORTED
> > if (strcmp(url, P_FTPS) == 0) {
> > -                       h->port = bb_lookup_std_port(P_FTPS, "tcp", 990);
> > +                       h->port = 990;
> > h->protocol = P_FTPS;
> > } else
> > #endif
> > #if SSL_SUPPORTED
> > if (strcmp(url, P_HTTPS) == 0) {
> > -                       h->port = bb_lookup_std_port(P_HTTPS, "tcp", 443);
> > +                       h->port = 443;
> > h->protocol = P_HTTPS;
> > } else
> > #endif
> > @@ -560,7 +560,7 @@ static void parse_url(const char *src_url, struct host_info \
> > *h) // GNU wget is user-friendly and falls back to http://
> > h->host = url;
> > http:
> > -               h->port = bb_lookup_std_port(P_HTTP, "tcp", 80);
> > +               h->port = 80;
> > }
> > 
> > // FYI:
> > diff --git a/util-linux/rdate.c b/util-linux/rdate.c
> > index 9b80141c9..68d9a7a46 100644
> > --- a/util-linux/rdate.c
> > +++ b/util-linux/rdate.c
> > @@ -45,7 +45,7 @@ static time_t askremotedate(const char *host)
> > alarm(10);
> > signal(SIGALRM, socket_timeout);
> > 
> > -       fd = create_and_connect_stream_or_die(host, bb_lookup_std_port("time", \
> > "tcp", 37)); +       fd = create_and_connect_stream_or_die(host, 37);
> > 
> > if (safe_read(fd, &nett, 4) != 4)    /* read time from server */
> > bb_error_msg_and_die("%s: %s", host, "short read");
> > --
> > 2.30.2
> > 
> > _______________________________________________
> > busybox mailing list
> > busybox@busybox.net
> > http://lists.busybox.net/mailman/listinfo/busybox



-- 
Sergey Ponomarev, skype:stokito
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic