openwrt / luci

LuCI - OpenWrt Configuration Interface
Apache License 2.0
6.3k stars 2.51k forks source link

`luci-app-xinetd:` enhancement: add the `bind` option #7116

Closed grifos closed 4 months ago

grifos commented 5 months ago

Enhancement:

I'd like to propose a possible enhancement to the luci-app-xinetd: adding the bind option to have the service only listening on one specific IP address instead of 0.0.0.0

https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/4/html/reference_guide/s3-tcpwrappers-xinetd-config-redirection

I've tested it by manually adding it to /etc/config/xinetd and it works, see below. I'd suggest adding it just above the Redirect option.

Unfortunately I'm not a developer so I wouldn't know what to put on a pull request.

cat /etc/config/xinetd | grep bind

option bind '192.168.1.1'

cat /var/run/xinetd.conf | grep bind

    bind = 192.168.1.1

netstat -l -n | grep 6566

tcp        0      0 192.168.1.1:6566         0.0.0.0:*               LISTEN

Additional Information:

HARDWARE='Belkin RT1800'
DISTRIB_ID='OpenWrt'
DISTRIB_RELEASE='SNAPSHOT'
DISTRIB_REVISION='r26274-752f6bf64e'
DISTRIB_TARGET='ramips/mt7621'
DISTRIB_ARCH='mipsel_24kc'
DISTRIB_DESCRIPTION='OpenWrt SNAPSHOT r26274-752f6bf64e'
DISTRIB_TAINTS=''
systemcrash commented 4 months ago

Only listening? Or listening on a specific IP only? ;)

If the file is /etc/config/xinetd it's already not in luci - but whichever repo within which that init script resides.

grifos commented 4 months ago

Hi @systemcrash

Respectfully, I think there is a misunderstanding there, as /etc/config/xinetd is exactly what the luci-app-xinetd modifies with its configuration changes. Please install it on your system (if you haven't already) and see it for yourself.

For reference, in 2022 there was another request for the redirect option, which was then implemented: https://github.com/openwrt/luci/issues/6151

systemcrash commented 4 months ago

And if you look in that issue, into which repo do the actual fixes for that issue go in?

grifos commented 4 months ago

So should I have opened this in packages? But then how is the luci app going to be updated to include the bind option field? The xinetd package itself already includes the bind option, as shown in my OP.

systemcrash commented 4 months ago

the packages repo is lower down in the stack - and is a pre-requisite for the luci app. When a feature goes in there, it's trivial to add to luci, but such functionality must exist in the xintd package. Nothing happens if we add it to luci alone...

The discussion as to whether that's possible and feasible should happen there. Because luci is just an interface to what already exists. Once that's done... we can re-open this if necessary.

systemcrash commented 4 months ago

@jow- @feckert @hnyman please move to packages repo

grifos commented 4 months ago

Yes, but that's my point. The bind option already exists on the xinetd package, it's only missing on the luci-app-xinetd

It works perfectly if added manually to /etc/config/xinetd

systemcrash commented 4 months ago

Please show me where bind is handled in the init script.

grifos commented 4 months ago

OK so the init script belongs to packages, now I get it, that's what's missing from the package itself, and after that's done the luci-app-xinetd can be updated to reflect. Thanks for the clarification and patience, I'm fairly new to this. I can open the issue on packages myself if you prefer.

systemcrash commented 4 months ago

Yes please. I can't move this ticket (I've only access within this repo), but I can help when the bind thing lands in the init script.

grifos commented 4 months ago

Will do tomorrow am. Thanks a lot Paul, much appreciated.

systemcrash commented 4 months ago

Can you show me your uci config, and the resulting xinetd.conf? If no special handling is required, it seems like we could just add it to luci. I just don't know how the init script works until I spend time to understand it.

grifos commented 4 months ago

Here it is. I've redacted some irrelevant bits.

BTW, TBH this isn't worth too much of your time as developers as it works fine if just added manually to the config. I've only proposed this enhancement to the luci app to save time to new users, not for myself as I was already planning to keep using the manual method ;-)

/etc/config/xinetd

config service
    option name 'ftp'
    option disable 'no'
    option flags 'IPv4'
    option type 'UNLISTED'
    option port '21'
    option protocol 'tcp'
    option socket_type 'stream'
    option user 'root'
    option wait 'yes'
    option bind '192.168.1.1'
    option server '/usr/sbin/vsftpd'
    list log_on_failure 'HOST'
    list log_on_failure 'USERID'
    list log_on_failure 'ATTEMPT'
    list log_on_success 'PID'
    list log_on_success 'HOST'
    list log_on_success 'USERID'
    list log_on_success 'EXIT'
    list log_on_success 'DURATION'

config service
    option name 'sane'
    option disable 'no'
    option flags 'IPv4'
    option type 'UNLISTED'
    option port '6566'
    option protocol 'tcp'
    option socket_type 'stream'
    option user 'root'
    option wait 'no'
    option bind '192.168.1.1'
    option server '/usr/sbin/saned'
    list log_on_failure 'HOST'
    list log_on_failure 'USERID'
    list log_on_failure 'ATTEMPT'
    list log_on_success 'PID'
    list log_on_success 'HOST'
    list log_on_success 'USERID'
    list log_on_success 'EXIT'
    list log_on_success 'DURATION'

config service
    option name 'p910nd'
    option disable 'no'
    option flags 'IPv4'
    option type 'UNLISTED'
    option port '9100'
    option protocol 'tcp'
    option socket_type 'stream'
    option server_args '-b -f /dev/usb/lp0'
    option user 'root'
    option wait 'no'
    option bind '192.168.1.1'
    list log_on_success 'PID'
    list log_on_success 'HOST'
    list log_on_success 'USERID'
    list log_on_success 'EXIT'
    list log_on_success 'DURATION'
    list log_on_failure 'HOST'
    list log_on_failure 'USERID'
    list log_on_failure 'ATTEMPT'
    option server '/usr/sbin/p910nd'

/var/run/xinetd.conf

# Auto-generated config file from /etc/config/xinetd
# Do not edit, changes to this file will be lost on upgrades

defaults
{

}

includedir /etc/xinetd.d
includedir /tmp/xinetd.d

service ftp
{
    disable = no
    flags = IPv4
    type = UNLISTED
    port = 21
    protocol = tcp
    socket_type = stream
    user = root
    wait = yes
    bind = 192.168.1.1
    server = /usr/sbin/vsftpd
    log_on_failure = HOST USERID ATTEMPT
    log_on_success = PID HOST USERID EXIT DURATION
}

service sane
{
    disable = no
    flags = IPv4
    type = UNLISTED
    port = 6566
    protocol = tcp
    socket_type = stream
    user = root
    wait = no
    bind = 192.168.1.1
    server = /usr/sbin/saned
    log_on_failure = HOST USERID ATTEMPT
    log_on_success = PID HOST USERID EXIT DURATION
}

service p910nd
{
    disable = no
    flags = IPv4
    type = UNLISTED
    port = 9100
    protocol = tcp
    socket_type = stream
    server_args = -b -f /dev/usb/lp0
    user = root
    wait = no
    bind = 192.168.1.1
    log_on_success = PID HOST USERID EXIT DURATION
    log_on_failure = HOST USERID ATTEMPT
    server = /usr/sbin/p910nd
}
systemcrash commented 4 months ago

Test my branch: see whether using the GUI does what you want. Replace the xinetd.js on your device with mine. If it all goes tits-up, just uninstall, reinstall luci-app-xinetd.

grifos commented 4 months ago

I've replaced /www/luci-static/resources/view/xinetd/xinetd.js with the one from your repository and I got ReferenceError: network is not defined on the luci app. I've put back the original file and it's working again.

systemcrash commented 4 months ago

Sorry, forgot the import. Try again please.

grifos commented 4 months ago

Now getting ReferenceError: so is not defined

systemcrash commented 4 months ago

OK, please try once more.

grifos commented 4 months ago

Yes that works fine, thank you. I've tested it with various IP addresses and also with unspecified and the setting has always been respected, as confirmed by netstat -l

Since I'm quite new to this, I'm also happy that my OP was correct in the end. Also, I'm sorry for my English, it isn't my first nor my second language but I try my best with it.

systemcrash commented 4 months ago

OK - good to know. Problem resolved. The weird thing with this app was that it fell outside of the normal start-up script properties - it seems all are inherently handled in a key/value fashion, with a few exceptions. And bind conveniently fell within the k/v structure.