Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

main/busybox: enable telnetd #1092

Merged
merged 1 commit into from
Jun 7, 2017

Conversation

ollieparanoid
Copy link
Contributor

@ollieparanoid ollieparanoid commented Mar 25, 2017

I'd like to have telnetd for debugging embedded devices via Ethernet over USB.
EDIT: Forgot to say, that I have built the package with this configuration and tested the telnetd, and it worked.

@vakartel
Copy link
Contributor

IMHO It's a wrong package to add telnetd. It would installed for all by default, but nobody need it. You can remade and rename "community/inetutils-syslogd" to reuse all it functionality not only syslogd.

@jirutka
Copy link
Member

jirutka commented Mar 25, 2017

I agree with @vakartel about not enabling telnet in busybox. I think that it'd be better to create package for e.g. BSD telnet implementation.

@jirutka jirutka closed this Mar 25, 2017
@ollieparanoid
Copy link
Contributor Author

ollieparanoid commented Mar 25, 2017

Thank you for your feedback, @vakartel and @jirutka.

I have some comments though, and hope I can change your mind.

You can remade and rename "community/inetutils-syslogd" to reuse all it functionality not only syslogd.

I think that it'd be better to create package for e.g. BSD telnet implementation.

The thing is, that I want to have the telnetd in my initramfs for debugging an embedded device over network. So it should be as small as possible, and should not have any login code for example and not introduce additional dependencies, such as an inetd or require an /etc/passwd file. Just drop the connecting client to a root shell.

To be more precise, I am playing with a native Alpine Linux installation on an Android phone (of course I'll share if anything usable comes out of this). There is an init script, that works with busybox' telnetd and I'd rather use it as is without hacking in another telnetd and making the resulting initramfs bigger.

It would installed for all by default, but nobody need it.

"Nobody need it" could also be said about the telnet client - I guess the usage is as widespread as the usage of the telnetd. Yet it is enabled in Alpine Linux' busybox config.

Including telnetd increases the busybox binary on armhf by exactly 4096 bytes, from 955912 to 960008. So we only have an increase of 0.4% (!) of the total executable size.

So... one more time, could you please accept the PR?
If not, I'll respect the decision and try to use another telnetd.

@vakartel
Copy link
Contributor

vakartel commented Mar 27, 2017

"Nobody need it" could also be said about the telnet client - I guess the usage is as widespread as the usage of the telnetd.
Yet it is enabled in Alpine Linux' busybox config.

telnet client is not only to connect to telnetd server. It's often used to make a testing connections to non-ssl tcp servers like smtp, pop3, imap, http etc.

for a very simple telnet client/server emulation there is nc (netcat) in busybox.
at server side run: nc -l 1234
at client side: nc server.host 1234
...and voila :)

@ollieparanoid
Copy link
Contributor Author

ollieparanoid commented Mar 27, 2017

Good idea with netcat @vakartel! I will use something like the following then:

#!/bin/sh
nc -lk -p 23 -e sh -c 'sh -i 2>&1'

@vakartel
Copy link
Contributor

Glad you like it.

@ollieparanoid
Copy link
Contributor Author

ollieparanoid commented Apr 11, 2017

Turns out, netcat has too many limitations compared to busybox' telnetd (problems with stderr - redirection as shown above does not work in practice; can't type in passwords without being visible on the screen with a normal telnet client etc.).
So I have packaged busybox-telnetd as standalone package (with its own APKBUILD and minimal busyboxconfig) and it works great for me.

Does it make sense to make a pull-request for that, or will it be declined?

@kaniini
Copy link
Contributor

kaniini commented Apr 12, 2017

i'm okay with adding telnetd if there is an actual usecase for it. it sounds like there is to me.

@ncopa what do you think

@kaniini kaniini reopened this Apr 12, 2017
@ncopa
Copy link
Contributor

ncopa commented Apr 12, 2017

Im ok with a separate package for it. I would prefer not enable it in the default busybox binary since I don´t want to tempt people to use it instead of ssh. Maybe we should also move out telnet client?

@kaniini
Copy link
Contributor

kaniini commented Apr 12, 2017

@ncopa that seems fine to me.

@ollieparanoid can you submit a PR that moves telnet and telnetd to it's own busybox binary? :)

@ollieparanoid
Copy link
Contributor Author

@kaniini: Sure, I will do that within the next week.

ollieparanoid pushed a commit to ollieparanoid/aports that referenced this pull request Apr 19, 2017
@ollieparanoid
Copy link
Contributor Author

Updated :)

@ollieparanoid
Copy link
Contributor Author

@kaniini , @ncopa is there anything I can do to speed this up? As I see it, it only needs a review by you guys now.

@clandmeter
Copy link
Member

creating multiple version of busybox to prevent users from using unsafe daemons/clients sounds plausible to me. In the end we will have the same PR's for ftpd, httpd and all insecure clients as well?
If busybox is our main system tool maybe we should remove all of those extra features and move them into a separate package called busybox-extras and have appropriate install files to generate the symlinks like we do already.

@kaniini
Copy link
Contributor

kaniini commented May 1, 2017

@clandmeter i agree, that would be quite nice. but we can do it for 3.7.

@clandmeter
Copy link
Member

We can do it post 3.6, should probably make a list of all tools that are applicable to -extras.

@ollieparanoid can you work on this (including matching install file) so we can merge it after we branch 3.6?

@ollieparanoid
Copy link
Contributor Author

I like the 'busybox-extras' idea. What would you think about having the 'busybox-extras' package replace the normal 'busybox' package, once it is installed (-> make it conflict with the busybox package)? So the users would only have one binary, even if the additional features are needed.

If you guys agree, I could directly work on the 'busybox-extras' subpackage (including the install file).

It would be nice if you could provide a list of busybox widgets, which should only be in extras, otherwise I'd use this list:

  • telnet
  • telnetd
  • ftpd
  • ftpget
  • ftpput
  • httpd

@kaniini
Copy link
Contributor

kaniini commented May 5, 2017

@ollieparanoid sounds good to me

@ollieparanoid
Copy link
Contributor Author

ollieparanoid commented May 5, 2017

Here is the update. I have also moved tftp to busybox-extras, and added tftpd there. The install files take care of properly removing symlinks now (whenever necessary).

Test output of upgrading/installing busybox and busybox-extras, and probing for telnet:
https://gist.github.com/ollieparanoid/40c94e1c0e51fdadb4ece40052fe509a

Removing busybox-extras to re-install busybox is a bit unintuitive, so I have written the commands in the post-install of busybox-extras. (I think, that this could be simplified in apk: when running apk del busybox-extras: it could automatically install busybox, as it is still needed by other packages. Not as some dirty hack, but as general rule for provides packages.) Anyway, this package really has the benefit of providing telnetd from busybox and whatever other people need can easily be added to busybox-extras now, so I'd like to see this PR merged even though the uninstallation is not as smooth as it could be yet (but as mentioned, it is described in the post-install, so it really should not be a problem for users to figure it out).

@ollieparanoid
Copy link
Contributor Author

Could someone review/merge this please? @kaniini @clandmeter @ncopa

@ollieparanoid
Copy link
Contributor Author

If I understood @clandmeter right, this can go to testing after 3.6 has been branched.
Is that right, and if so, when will 3.6 be branched?

Anything more left for me to do here?
I just want to make sure, that this PR finds its way into Alpine.
Thanks.

@ollieparanoid
Copy link
Contributor Author

@vakartel @jirutka @kaniini @clandmeter @ncopa

3.6 is out, so let's talk about this PR again. I did as @clandmeter asked above:

If busybox is our main system tool maybe we should remove all of those extra features and move them into a separate package called busybox-extras and have appropriate install files to generate the symlinks like we do already. [...]
@ollieparanoid can you work on this (including matching install file) so we can merge it after we branch 3.6?

My comment from one month ago shows the workflow, that one would have with installing busybox-extras.

Please tell me, if this is good enough, and if you would accept the PR if I rebased it.

I'd like to contribute more, but not hearing from you for so long makes it kind of hard... would it be better if I set up a bouncer and joined IRC?

Thanks.

@ncopa
Copy link
Contributor

ncopa commented Jun 6, 2017

@ollieparanoid What do you think about building a /bin/busybox-extras binary with the "extras" applets only. the trigger would set up the symlinks to point to /bin/busybox-extras.

That way we make the uninstall as simple as apk del busybox-extras.

@ollieparanoid
Copy link
Contributor Author

I'd rather have busybox-extras replace busybox, so we don't have the code duplication. But as you pointed out in IRC, the uninstallation isn't intuitive and should be avoided.

I will rebase the code and make it work like you suggested (two binaries: busybox and busybox-extras). (The upside is, that for my initramfs usecase, the redundant data shouldn't matter much, because of compression.)

Thank you!

The following programs have been moved from busybox to -extras:
ftpget, ftpput, telnet, tftp, ftpd, httpd

New programs in -extras: telnetd, tftpd
@ncopa
Copy link
Contributor

ncopa commented Jun 7, 2017

this is nice. very nice. Thank you 👍

@algitbot
Copy link

algitbot commented Jun 7, 2017

Merged in 23f4c6b by @ollieparanoid. Thanks for your contribution!

(This pull request has been closed automatically by GitHub PR Closer. If you think that it’s not resolved yet, please add a comment.)

@algitbot algitbot merged commit 23f4c6b into alpinelinux:master Jun 7, 2017
@ollieparanoid ollieparanoid deleted the busybox-telnet branch August 4, 2017 15:02
samueldr pushed a commit to samueldr/pmaports that referenced this pull request Nov 17, 2018
Pull request, that got merged:
alpinelinux/aports#1092

Please note, that you can't directly upgrade from postmarketOS "busybox-extras" to the upstreamed version.
To upgrade properly, do the following:
* delete your self-compiled busybox* packages:
	`sudo rm ~/.local/var/pmbootstrap/packages/armhf/busybox*`
* zap all your armhf chroots with:
	`./pmbootstrap.py zap`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants