Kodi - Open ports to allow smb/samba browsing #3503

zguithues · 2017-11-11T02:48:05Z

browsing smb/samba shares requires having ports 1025 - 65535 open.
Source: https://github.com/dnschneid/crouton/wiki/How-to-mount-network-shares-on-Chromebook-(sshfs,-cifs,-nfs-etc)

this patch grabs the ip address from wlan0 and opens the ports when the source ip is within the local subnet. if wlan0 has no ip, it does not modify iptables.

I'm not sure how many ChromeOS devices have an eth0, or any other interfaces that should be accounted for. or perhaps there is a better way to accomplish the same thing.

iptables gets reset on reboots, so it's probably not a big deal that the rule doesn't get cleared. however, i would love to clear the port forward when kodi closes, or the chroot exits.
do you have any ideas for this @dnschneid ?

browsing smb/samba shares requires having ports 1025 - 65535 open. this patch grabs the ip address from wlan0 and opens the ports when the source ip is within the local subnet.

divx118

Thanks for creating this pull request.

divx118 · 2017-11-11T13:36:38Z

host-bin/startkodi

@@ -14,5 +14,10 @@ By default, it will log into the primary user on the first chroot found.

 Options are directly passed to enter-chroot; run enter-chroot to list them."

+# Forward ports needed to browse smb shares
+if test -n "$(ip -4 -o addr show dev wlan0| awk '{split($4,a,"/");print a[1]}')"; then


This would assume you are using wlan0, you also could be using a usb ethernet adapter or another wlan device. Maybe something like ip route get 1 | awk -F 'src ' '{ split($2,a," ");print a[1];exit}'

running that command returns a "1"

i agree that we should take into consideration other devices, i just don't know how...

@zguithues hmm strange I just tried it on some different linux devices and my chromebook and that seems to work. What do you get when you run ip route get 1 for output. Maybe the awk part isn't full proof. If not, there sure will be other ways to get the used ip-address, without looking at the device.

divx118 · 2017-11-11T13:37:48Z

host-bin/startkodi

+# Forward ports needed to browse smb shares
+if test -n "$(ip -4 -o addr show dev wlan0| awk '{split($4,a,"/");print a[1]}')"; then
+	iptables -I INPUT 1 -p udp --source $(ip -4 -o addr show dev wlan0| awk '{split($4,a,"/");print a[1]}')/255.255.255.0 --dport 1025:65535 -j ACCEPT
+fi


Max line length should be 80

@divx118 method works for me, here's the output of each:

@zguithues -

chronos@localhost ~ $ ip -4 -o addr show dev wlan0| awk '{split($4,a,"/");print a[1]}' 192.168.1.225

@divx118 -

chronos@localhost ~ $ ip route get 1 1.0.0.0 via 192.168.1.1 dev wlan0 src 192.168.1.225 uid 1000 cache chronos@localhost ~ $ ip route get 1 | awk -F 'src ' '{ split($2,a," ");print a[1];exit}' 192.168.1.225

ok, i got it working @divx118. I was missing the space between the 2 " in the split($2,a," ").

chronos@localhost / $ ip route get 1 | awk -F 'src ' '{ split($2,a," ");print a[1];exit}' 192.168.2.103

i'll update the patch. thanks!

switched from: ip -4 -o addr show dev wlan0| awk '{split($4,a,"/");print a[1]}' to: ip route get 1 | awk -F 'src ' '{ split($2,a," ");print a[1];exit}' also used a variable to get line length down.

zguithues · 2017-11-12T17:46:27Z

OK, i've updated the command, and switched to using a variable so i could easily keep the line length down.

The only issue i still have with this is clearing the rule after kodi closes. I'm not sure the best way to do this, but it's not really that big of a deal...

removed the -n from the test to check whether it succeeds, rather than whether the return exists.

zguithues · 2017-11-12T17:57:38Z

ok, previously this would fail if no network exists, i changed the test, removed the -n. This seems to work from my end. please let me know if you see any other issues with the code @DennisLfromGA @divx118

thanks a bunch for your help guys!

divx118

Looks good to me except those little things I mentioned. Thanks

divx118 · 2017-11-12T18:11:45Z

host-bin/startkodi

@@ -17,7 +17,7 @@ Options are directly passed to enter-chroot; run enter-chroot to list them."
 # Forward ports needed to browse smb shares
 MYIP=$(ip route get 1 | awk -F 'src ' '{ split($2,a," ");print a[1];exit}')

-if test -n $MYIP; then
+if test $MYIP; then


if [ -n "$MYIP" ]; then should work. It also would be more consistent with the syntax elsewhere used in crouton scripts. Try to always quote strings.

divx118 · 2017-11-12T18:12:25Z

host-bin/startkodi

@@ -17,7 +17,7 @@ Options are directly passed to enter-chroot; run enter-chroot to list them."
 # Forward ports needed to browse smb shares
 MYIP=$(ip route get 1 | awk -F 'src ' '{ split($2,a," ");print a[1];exit}')

-if test -n $MYIP; then
+if test $MYIP; then
 	iptables -I INPUT 1 -p udp \
 	--source $MYIP/255.255.255.0 \


"$MYIP"/255.255.255.0 \ quotes

@divx118

thanks @divx118

zguithues · 2017-11-12T18:30:43Z

thanks @divx118 !

divx118 · 2017-11-12T20:22:54Z

host-bin/startkodi

+MYIP=$(ip route get 1 | awk -F 'src ' '{ split($2,a," ");print a[1];exit}')
+
+if [ -n "$MYIP" ]; then
+	iptables -I INPUT 1 -p udp \


Sorry I overlooked this, but indentation is standard 4 spaces (no tabs) in the crouton scripts so it should look like

if [ -n "$MYIP" ]; then iptables -I INPUT 1 -p udp \ --source "$MYIP"/255.255.255.0 \ --dport 1025:65535 -j ACCEPT fi

divx118

Great, now let's see if @dnschneid approves it too.

DennisLfromGA · 2017-11-12T21:09:42Z

LGTM2 👍

dnschneid

For reverting the iptables, maybe add a script inside the chroot that you launch as part of the enter-chroot command, which in turn forks off a subshell that traps HUP or TERM (which gets triggered by unmount-chroot; you'll have to experiment) and then resets the settings?

dnschneid · 2017-11-15T01:59:30Z

host-bin/startkodi

@@ -14,5 +14,14 @@ By default, it will log into the primary user on the first chroot found.

 Options are directly passed to enter-chroot; run enter-chroot to list them."

+# Forward ports needed to browse smb shares
+MYIP=$(ip route get 1 | awk -F 'src ' '{ split($2,a," ");print a[1];exit}')


Quote the output, and space out the awk commands. If it doesn't fit on one line, you can make the awk script span multiple lines.

My limited knowledge indicated 2 places that obviously needed spacing, did i miss anything else?

I think i added the quote properly...

Yep, that looks better.

zguithues · 2017-11-15T18:53:08Z

@dnschneid Thanks for the direction, but unfortunately that's mostly Chinese to me...

can you point me in the direction of something i can build off of?

dnschneid · 2017-11-16T20:38:36Z

Unfortunately, I don't think there's anything like it at the moment, and this feature has a weird set of requirements

root permission, regardless of which user is running kodi
needs to be triggered when the chroot unmounts
needs to only be triggered when launching kodi

I guess if we accept the caveat that you can only launch one kodi at a time, you can just get rid of the exec keyword in the startkodi script and add a trap on INT/HUP/0 to run the ip command.

zguithues · 2017-11-17T18:02:56Z

i'll poke around and see what i can come up with.

i'd be happy for suggestions/recommendations from the peanut gallery ;)

@dnschneid do you consider the removal of this iptables rule to be imperative to the merging of this patch?

RUDIVANTORRE · 2017-11-17T23:48:43Z

z

dnschneid · 2017-11-20T06:48:17Z

I agree with @RUDIVANTORRE. It should be pretty straightforward to make the change, so I think the merge can wait until you have it working.

open ports to browse smb

0343c6b

browsing smb/samba shares requires having ports 1025 - 65535 open. this patch grabs the ip address from wlan0 and opens the ports when the source ip is within the local subnet.

divx118 reviewed Nov 11, 2017

View reviewed changes

use better command to get ip address

1a5877c

switched from: ip -4 -o addr show dev wlan0| awk '{split($4,a,"/");print a[1]}' to: ip route get 1 | awk -F 'src ' '{ split($2,a," ");print a[1];exit}' also used a variable to get line length down.

change test to see if command succeeds

749acc0

removed the -n from the test to check whether it succeeds, rather than whether the return exists.

divx118 suggested changes Nov 12, 2017

View reviewed changes

improve syntax

a3c6eb8

thanks @divx118

divx118 reviewed Nov 12, 2017

View reviewed changes

fixed indentation

bbed58d

divx118 approved these changes Nov 12, 2017

View reviewed changes

dnschneid reviewed Nov 15, 2017

View reviewed changes

Fixed spacing

9f676c1

dnschneid added the stale label Dec 31, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Kodi - Open ports to allow smb/samba browsing #3503

Kodi - Open ports to allow smb/samba browsing #3503

zguithues commented Nov 11, 2017 •

edited

divx118 left a comment

divx118 Nov 11, 2017 •

edited

zguithues Nov 12, 2017

divx118 Nov 12, 2017

divx118 Nov 11, 2017

DennisLfromGA Nov 12, 2017

zguithues Nov 12, 2017

zguithues commented Nov 12, 2017

zguithues commented Nov 12, 2017

divx118 left a comment

divx118 Nov 12, 2017 •

edited

divx118 Nov 12, 2017

zguithues commented Nov 12, 2017

divx118 Nov 12, 2017

zguithues Nov 12, 2017

divx118 left a comment

DennisLfromGA commented Nov 12, 2017

dnschneid left a comment

dnschneid Nov 15, 2017

zguithues Nov 15, 2017

dnschneid Nov 16, 2017

zguithues commented Nov 15, 2017

dnschneid commented Nov 16, 2017

zguithues commented Nov 17, 2017 •

edited

RUDIVANTORRE commented Nov 17, 2017

dnschneid commented Nov 20, 2017

Kodi - Open ports to allow smb/samba browsing #3503

Are you sure you want to change the base?

Kodi - Open ports to allow smb/samba browsing #3503

Conversation

zguithues commented Nov 11, 2017 • edited

divx118 left a comment

Choose a reason for hiding this comment

divx118 Nov 11, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zguithues commented Nov 12, 2017

zguithues commented Nov 12, 2017

divx118 left a comment

Choose a reason for hiding this comment

divx118 Nov 12, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zguithues commented Nov 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

divx118 left a comment

Choose a reason for hiding this comment

DennisLfromGA commented Nov 12, 2017

dnschneid left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zguithues commented Nov 15, 2017

dnschneid commented Nov 16, 2017

zguithues commented Nov 17, 2017 • edited

RUDIVANTORRE commented Nov 17, 2017

dnschneid commented Nov 20, 2017

zguithues commented Nov 11, 2017 •

edited

divx118 Nov 11, 2017 •

edited

divx118 Nov 12, 2017 •

edited

zguithues commented Nov 17, 2017 •

edited