Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions net/simple-captive-portal/Makefile
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
include $(TOPDIR)/rules.mk

PKG_NAME:=simple-captive-portal
PKG_VERSION:=2025.06.22
PKG_VERSION:=2026.01.14
PKG_RELEASE:=1

PKG_MAINTAINER:=Etienne CHAMPETIER <champetier.etienne@gmail.com>
Expand All @@ -23,12 +23,11 @@ endef
define Package/simple-captive-portal/install
$(INSTALL_DIR) $(1)/etc/config
$(INSTALL_CONF) ./files/etc/config/simple-captive-portal $(1)/etc/config/simple-captive-portal
$(INSTALL_DIR) $(1)/etc/hotplug.d/net/
$(INSTALL_DATA) ./files/etc/hotplug.d/net/00-simple-captive-portal $(1)/etc/hotplug.d/net/00-simple-captive-portal
$(INSTALL_DIR) $(1)/etc/init.d
$(INSTALL_BIN) ./files/etc/init.d/simple-captive-portal $(1)/etc/init.d/simple-captive-portal
$(INSTALL_DIR) $(1)/etc/simple-captive-portal/
$(INSTALL_DATA) ./files/etc/simple-captive-portal/index.html $(1)/etc/simple-captive-portal/index.html
$(INSTALL_DATA) ./files/etc/simple-captive-portal/connected.html $(1)/etc/simple-captive-portal/connected.html
$(INSTALL_DIR) $(1)/usr/share/simple-captive-portal/
$(INSTALL_DATA) ./files/usr/share/simple-captive-portal/capabilities.json $(1)/usr/share/simple-captive-portal/capabilities.json
$(INSTALL_DATA) ./files/usr/share/simple-captive-portal/portal.lua $(1)/usr/share/simple-captive-portal/portal.lua
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
config simple-captive-portal main
#option interface guest
#list interface guest
#option port_redirect 8888
#option port_portal 8889
#option timeout 86400
#list exempt '01:23:45:67:89:0A'

This file was deleted.

65 changes: 45 additions & 20 deletions net/simple-captive-portal/files/etc/init.d/simple-captive-portal
Original file line number Diff line number Diff line change
@@ -1,14 +1,28 @@
#!/bin/sh /etc/rc.common
START=10
USE_PROCD=1
EXTRA_COMMANDS='firewall'
EXTRA_COMMANDS='flush_guests'

firewall() {
local INTF PORT_REDIRECT TIMEOUT
add_exempt() {
[ -z "${1}" ] || /usr/sbin/nft add element inet simple-captive-portal exempt_macs { "${1}" }
}

add_interface() {
[ -z "${1}" ] || /usr/sbin/nft add element inet simple-captive-portal portal_ifs { "${1}" }
}

service_triggers() {
procd_add_reload_trigger "simple-captive-portal"
}

start_service() {
local INTF INTF_COUNT PORT_REDIRECT PORT_PORTAL TIMEOUT
config_load "simple-captive-portal"
config_get INTF main interface
[ -z "${INTF}" ] && exit 0
config_get INTF_COUNT main interface_LENGTH
config_get PORT_REDIRECT main port_redirect 8888
config_get PORT_PORTAL main port_portal 8889
config_get TIMEOUT main timeout 86400

/usr/sbin/nft -f- <<EOF
Expand All @@ -20,33 +34,35 @@ table inet simple-captive-portal {
timeout ${TIMEOUT}s
}

set exempt_macs {
type ether_addr
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should be able to add the members of the set directly here, and move the flush set inet simple-captive-portal exempt_macs with the other flush so we call nft once, and the change is atomic.
Here when you reconfigure there is a small window of time in which the set is empty, so all traffic is going to be rejected.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I also wasn't thrilled about losing the atomicity here. This definitely needs to be addressed then.

I could use something like this to generate the list, but this assumes no exempt option can contain a space:

local EXEMPT
config_get EXEMPT main exempt
[ -z "${EXEMPT}" ] || EXEMPT="\"$(echo "${EXEMPT}" | sed 's/ /", "/g' )\""

I'm not aware of any ether_addr values that would contain a space, but I wasn't 100% sure.

I also considered using an alternative method based on config_list_foreach, a global variable, and a couple string builder functions that would preserve spaces, but it was really gross and I didn't think it would review well, haha.

There's also the case where there are no exempt options configured, so the add element needs to be done conditionally since you can't pass an empty string.

Having flush ... exempt_macs after the table definition avoids the case during first boot where the exempt_macs set does not exist yet. Defining it quickly beforehand isn't as simple as it is for the table.

I have a couple ideas depending on whether the spaces issue is relevant or not. Please advise and I'll get it figured out and updated. :)

}

set portal_ifs {
type ifname
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment than on first commit, add the interfaces in the set right away

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will likely have the same solution as exempt_macs.
I'm pretty sure ifname names can't have spaces in them, right?

}

chain prerouting {
type nat hook prerouting priority mangle; policy drop;
iif != ${INTF} accept
iifname != @portal_ifs accept
ether saddr @exempt_macs accept
ether saddr @guest_macs accept
tcp dport 80 redirect to ${PORT_REDIRECT}
fib daddr . iif type { local, broadcast, multicast } accept
reject
}
}
flush set inet simple-captive-portal exempt_macs
flush set inet simple-captive-portal portal_ifs
EOF
}
config_list_foreach main exempt add_exempt

boot() {
BOOT=1
start "$@"
}

start_service() {
# firewall() called by hotplug on boot
[ -z "${BOOT}" ] && firewall

local INTF PORT_REDIRECT PORT_PORTAL
config_load "simple-captive-portal"
config_get INTF main interface
[ -z "${INTF}" ] && exit 0
config_get PORT_REDIRECT main port_redirect 8888
config_get PORT_PORTAL main port_portal 8889
# Detect whether the interface is an option or a list
if [ -n "${INTF_COUNT}" ]; then
config_list_foreach main interface add_interface
else
add_interface "${INTF}"
fi

procd_open_instance redirect
procd_set_param command /usr/sbin/uhttpd -f -c /dev/null -k0 -h /etc/simple-captive-portal/ -l / -L /usr/share/simple-captive-portal/redirect.lua -p "${PORT_REDIRECT}"
Expand Down Expand Up @@ -79,3 +95,12 @@ start_service() {
procd_set_param stderr 1
procd_close_instance
}

stop_service() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we just implement stop, then I think restart/reload are implemented as stop/start,
I don't think we want to flush the guests / temporarily open on restart/reload

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that is the case. I wasn't happy about the tiny gap during restart either.

Stopping the firewall blocking on stop was intended to keep the state of the firewall and uhttpd in sync with each other. I experimented with adding a pause command that would stop the blocking, but it left uhttpd running, and calling start/stop/pause in different ways could end up getting the system into some confusing states.

Before, the only way to stop the blocking was to remove the interface from the config and reboot. With the other changes in this PR, you can remove interfaces and reload quickly. If it's not an issue that the firewall can be blocking and the login page is unavailable, then this addition of stop probably isn't necessary.

# Delete the rule that blocks traffic
/usr/sbin/nft delete chain inet simple-captive-portal prerouting &> /dev/null
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no reason to silence errors here

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the stop command is run when the service is already stopped, the following error message is printed:

Error: No such file or directory; did you mean chain 'prerouting' in table inet 'fw4'?
delete chain inet simple-captive-portal prerouting

I was concerned someone might see this message and attempt to delete the prerouting chain on fw4, so I opted to silence it, heh.

}

flush_guests() {
/usr/sbin/nft flush set inet simple-captive-portal guest_macs
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Simple captive portal</title>
</head>
<body style="text-align:center">
<h1>Free Wifi</h1>
<p>You are now connected. You may close this page.</p>
</body>
</html>
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,24 @@ function handle_request(env)
return
end

uhttpd.send("Status: 200 OK\r\n")
if string.find(env.SERVER_ADDR, ":") == nil then
url = "http://" .. env.SERVER_ADDR .. ":" .. env.SERVER_PORT .. "/connected.html"
else
url = "http://[" .. env.SERVER_ADDR .. "]:" .. env.SERVER_PORT .. "/connected.html"
end

body = "<!DOCTYPE html>\r\n" ..
"<html lang=\"en\"><head>\r\n" ..
"<title>Connected</title>\r\n" ..
"</head><body>\r\n"..
"<p>You are now connected. You may close this page.</p>\r\n" ..
"</body></html>\r\n"
Comment on lines +29 to +34
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not tested:

    body = [[
<!DOCTYPE html>
<html lang="en"><head>
<title>Connected</title>
</head><body>
<p>You are now connected. You may close this page.</p>
</body></html>]]


uhttpd.send("Status: 302 Found\r\n")
uhttpd.send("Server: simple-captive-portal\r\n")
uhttpd.send("Content-Type: text/plain\r\n\r\n")
uhttpd.send("You now have internet access\n")
uhttpd.send("Location: " .. url .. "\r\n")
uhttpd.send("Cache-Control: no-cache\r\n")
uhttpd.send("Content-Type: text/html\r\n")
uhttpd.send("Content-Length: " .. string.len(body) .. "\r\n\r\n")
uhttpd.send(body)
end
Original file line number Diff line number Diff line change
@@ -1,13 +1,27 @@
port_portal = os.getenv("PORT_PORTAL")

function handle_request(env)
uhttpd.send("Status: 302 Found\r\n")
uhttpd.send("Server: simple-captive-portal\r\n")
if string.find(env.SERVER_ADDR, ":") == nil then
uhttpd.send("Location: http://" .. env.SERVER_ADDR .. ":" .. port_portal .. "/\r\n")
url = "http://" .. env.SERVER_ADDR .. ":" .. port_portal .. "/"
else
uhttpd.send("Location: http://[" .. env.SERVER_ADDR .. "]:" .. port_portal .. "/\r\n")
url = "http://[" .. env.SERVER_ADDR .. "]:" .. port_portal .. "/"
end

body = "<!DOCTYPE html>\r\n" ..
"<html lang=\"en\"><head>\r\n" ..
"<title>Redirecting to login</title>\r\n" ..
"</head><body>\r\n"..
"<p>Redirecting to login page...</p>\r\n" ..
"<p><a href=\"" .. url .. "\">" ..
"Click here if the page does not automatically redirect." ..
"</a></p>\r\n" ..
"</body></html>\r\n"
Comment on lines +10 to +18
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never seen a client that doesn't follow 302, can you share the one you saw ?
In any case this is a good addition, but no need for \r\n for the body, \n is fine.

Not tested:

    body = string.format([[
<!DOCTYPE html>
<html lang="en"><head>
<title>Redirecting to the captive portal</title>
</head><body>
<p>Redirecting to the captive portal...</p>
<p><a href="%s">Click here if the page does not automatically redirect.</a></p>
</body></html>]], url)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't seen one in quite a while now that I think of it, but I always try to add guard links to redirects when I can. Old habits die hard, heh.

I'm not thrilled that the HTML is hard-coded when other pages are configurable, but I don't know enough about how uhttpd/LUA works to safely attempt loading and templating something from /etc/simple-captive-portal, and hardly anyone will see this page anyway.


uhttpd.send("Status: 302 Found\r\n")
uhttpd.send("Server: simple-captive-portal\r\n")
uhttpd.send("Location: " .. url .. "\r\n")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't make a huge difference, but I think you can do

uhttpd.send("Location: ", url, "\r\n")

uhttpd.send("Cache-Control: no-cache\r\n")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a potential bug here (already existing), we should send Connection: close to be sure the hijacked http connection is closed and not reused.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, good catch, thanks. I'll add it in the next push. 👍

uhttpd.send("Content-Length: 0\r\n\r\n")
uhttpd.send("Content-Type: text/html\r\n")
uhttpd.send("Content-Length: " .. string.len(body) .. "\r\n\r\n")
uhttpd.send(body)
end