ipoe_server: T6872: Add the ability to configure LUA scripts and username #4196

natali-rs1985 · 2024-11-18T11:21:39Z

Change Summary

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Code style update (formatting, renaming)
Refactoring (no functional changes)
Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
Other (please describe):

Related Task(s)

https://vyos.dev/T6872

Related PR(s)

Component(s) name

service ipoe-server

Proposed changes

added ability to configure username with LUA script

set service ipoe-server lua-file <path_to_lua_file>
set service ipoe-server interface eth1 lua-username <username_func>

Also changed systemctl action from reload-or-restart to restart because accel-ppp doesn't apply changes to the configuration with reload-or-restart action

How to test

Example of the lua file:

vyos@vyos# cat /config/ipoe.lua
#!lua
function username_func(pkt)
    local username=pkt:hwaddr()
    return username
end

Config:

set service ipoe-server authentication mode 'radius'
set service ipoe-server authentication radius server 127.0.0.1 key 'vyos-secret'
set service ipoe-server client-ip-pool POOL range '192.168.10.5-192.168.10.100'
set service ipoe-server default-pool 'POOL'
set service ipoe-server gateway-address '192.168.10.1/24'
set service ipoe-server interface eth1 mode 'l2'
set service ipoe-server interface eth1 network 'shared'
set service ipoe-server name-server '1.1.1.1'
set service ipoe-server name-server '198.168.10.1'

set service ipoe-server interface eth1 lua-username 'username_func'
set service ipoe-server lua-file '/config/scripts/ipoe.lua'

Logs

Nov 15 20:30:18 vyos accel-ipoe[18180]: ipoe0:: send [RADIUS(1) Access-Request id=1 <User-Name "0c:dd:ab:38:00:01"> <NAS-Port 352> <NAS-Port-Id "ipoe0"> <NAS-Port-Type Ethernet> <Calling-Station-Id "0c:dd:ab:38:00:01"> <Called-Station-Id "eth1"> <User-Password 0xa6b74758d04d3f60be39e9c349fdbc8654fb4a586e8e5859b7c72c9e82937320>]
Nov 15 20:30:18 vyos accel-ipoe[18180]: ipoe0:: recv [RADIUS(1) Access-Accept id=1 <Service-Type Framed-User> <Framed-Pool "POOL"> <Stateful-IPv6-Address-Pool "IPv6-POOL"> <Delegated-IPv6-Prefix-Pool "IPv6-POOL"> <Framed-Protocol PPP>]
Nov 15 20:30:18 vyos accel-ipoe[18180]: ipoe0:0c:dd:ab:38:00:01: 0c:dd:ab:38:00:01: authentication succeeded
Nov 15 20:30:18 vyos accel-ipoe[18180]: ipoe0:0c:dd:ab:38:00:01: ipoe: no free IPv6 address
Nov 15 20:30:18 vyos accel-ipoe[18180]: ipoe0:0c:dd:ab:38:00:01: send [RADIUS(1) Accounting-Request id=1 <User-Name "0c:dd:ab:38:00:01"> <NAS-Port 352> <NAS-Port-Id "ipoe0"> <NAS-Port-Type Ethernet> <Calling-Station-Id "0c:dd:ab:38:00:01"> <Called-Station-Id "eth1"> <Acct-Status-Type Start> <Acct-Authentic RADIUS> <Acct-Session-Id "b2e7d371f7638e00"> <Acct-Session-Time 0> <Acct-Input-Octets 0> <Acct-Output-Octets 0> <Acct-Input-Packets 0> <Acct-Output-Packets 0> <Acct-Input-Gigawords 0> <Acct-Output-Gigawords 0> <Framed-IP-Address 192.168.10.5>]
Nov 15 20:30:18 vyos accel-ipoe[18180]: ipoe0:0c:dd:ab:38:00:01: recv [RADIUS(1) Accounting-Response id=1]
Nov 15 20:30:18 vyos accel-ipoe[18180]: ipoe0:0c:dd:ab:38:00:01: ipoe: session started

Smoketest result

vyos@vyos# python3 /usr/libexec/vyos/tests/smoke/cli/test_service_ipoe-server.py
test_accel_ipv4_pool (__main__.TestServiceIPoEServer.test_accel_ipv4_pool) ... ok
test_accel_ipv6_pool (__main__.TestServiceIPoEServer.test_accel_ipv6_pool) ... ok
test_accel_limits (__main__.TestServiceIPoEServer.test_accel_limits) ... ok
test_accel_local_authentication (__main__.TestServiceIPoEServer.test_accel_local_authentication) ... ok
test_accel_log_level (__main__.TestServiceIPoEServer.test_accel_log_level) ... ok
test_accel_name_servers (__main__.TestServiceIPoEServer.test_accel_name_servers) ... ok
test_accel_next_pool (__main__.TestServiceIPoEServer.test_accel_next_pool) ... ok
test_accel_ppp_options (__main__.TestServiceIPoEServer.test_accel_ppp_options) ... skipped 'PPP is not a part of IPoE'
test_accel_radius_authentication (__main__.TestServiceIPoEServer.test_accel_radius_authentication) ... ok
test_accel_shaper (__main__.TestServiceIPoEServer.test_accel_shaper) ... ok
test_accel_snmp (__main__.TestServiceIPoEServer.test_accel_snmp) ... ok
test_accel_wins_server (__main__.TestServiceIPoEServer.test_accel_wins_server) ... skipped 'WINS server is not used in IPoE'
test_ipoe_server_vlan (__main__.TestServiceIPoEServer.test_ipoe_server_vlan) ... ok

----------------------------------------------------------------------
Ran 13 tests in 123.136s

OK (skipped=2)

Checklist:

I have read the CONTRIBUTING document
I have linked this PR to one or more Phabricator Task(s)
I have run the components SMOKETESTS if applicable
My commit headlines contain a valid Task id
My change requires a change to the documentation
I have updated the documentation accordingly

github-actions · 2024-11-18T11:21:59Z

👍
No issues in PR Title / Commit Title

dmbaturin · 2024-11-19T10:16:53Z

interface-definitions/service_ipoe-server.xml.in

+                  <help>Username function</help>
+                  <valueHelp>
+                    <format>txt</format>
+                    <description>Name of function in lua file to construct username</description>


Suggested change

<description>Name of function in lua file to construct username</description>

<description>Name of the function in the Lua file to construct usernames with</description>

"Lua" is a proper name and should be capitalized.

dmbaturin · 2024-11-19T10:17:10Z

interface-definitions/service_ipoe-server.xml.in

              #include <include/accel-ppp/vlan.xml.i>
              #include <include/accel-ppp/vlan-mon.xml.i>
            </children>
          </tagNode>
+          <leafNode name="lua-file">
+            <properties>
+              <help>File containing lua function for create username</help>


Suggested change

<help>File containing lua function for create username</help>

<help>Lua script file for constructing user names</help>

dmbaturin · 2024-11-19T10:17:53Z

interface-definitions/service_ipoe-server.xml.in

+              <help>File containing lua function for create username</help>
+              <valueHelp>
+                <format>filename</format>
+                <description>File with LUA script</description>


Suggested change

<description>File with LUA script</description>

<description>File with Lua script</description>

"Lua" is not an acronym, it's a Portugese word that means "the moon".

dmbaturin · 2024-11-19T10:18:17Z

src/conf_mode/service_ipoe-server.py

-                              'use "client-ip-pool" instead!')
-        if 'vlan_mon' in iface_config and not 'vlan' in iface_config:
+            raise ConfigError(
+                'Option "client-subnet" and "vlan" are mutually exclusive, '


Suggested change

'Option "client-subnet" and "vlan" are mutually exclusive, '

'Options "client-subnet" and "vlan" are mutually exclusive, '

Missed plural.

dmbaturin · 2024-11-19T10:18:45Z

src/conf_mode/service_ipoe-server.py

+                raise ConfigError(f'File {ipoe["lua_file"]} does not exist')
+            if dict_search('authentication.mode', ipoe) != 'radius':
+                raise ConfigError(
+                    'Can configure username with LUA script only for RADIUS authentication'


Suggested change

'Can configure username with LUA script only for RADIUS authentication'

'Can configure username with Lua script only for RADIUS authentication'

interface-definitions/service_ipoe-server.xml.in

c-po · 2024-11-20T19:54:03Z

src/conf_mode/service_ipoe-server.py

        call(f'systemctl stop {systemd_service}')
        for file in [ipoe_conf, ipoe_chap_secrets]:
            if os.path.exists(file):
                os.unlink(file)

        return None

-    call(f'systemctl reload-or-restart {systemd_service}')
+    call(f'systemctl restart {systemd_service}')


Why force a restart? reload-or-restart tries if daemon soft-reload is implemented and if not it will hard-restart the daemon.

Accel-ppp does not do it correctly (not implemented in accel-pppd)
Most of the changes required restarting the service

Its restart doesn't interrupt user sessions, or does it?

Its restart doesn't interrupt user sessions, or does it?

It will drop user sessions
otherwise, you have to drop the whole IPoE config or restart manually

dmbaturin

Looks good to me now. I think the motivation for hard restart should be documented in the comments, so that no one wonders about it in the future — or, when accel-ppp implements a soft reload, can spot it as untrue and change it.

…name

github-actions · 2024-11-21T13:30:03Z

CI integration 👍 passed!

Details

CI logs

CLI Smoketests (no interfaces) 👍 passed
CLI Smoketests (interfaces only) 👍 passed
Config tests 👍 passed
RAID1 tests 👍 passed
TPM tests 👍 passed

natali-rs1985 requested a review from a team as a code owner November 18, 2024 11:21

natali-rs1985 requested review from dmbaturin, sarthurdev, zdc, jestabro, sever-sever and nicolas-fort November 18, 2024 11:21

natali-rs1985 force-pushed the T6872 branch from aff686a to 8077f7e Compare November 18, 2024 11:22

github-actions bot added the current label Nov 18, 2024

github-actions bot assigned natali-rs1985 Nov 18, 2024

natali-rs1985 force-pushed the T6872 branch from 8077f7e to 9c0c8d5 Compare November 18, 2024 12:52

sever-sever approved these changes Nov 18, 2024

View reviewed changes

dmbaturin requested changes Nov 19, 2024

View reviewed changes

natali-rs1985 force-pushed the T6872 branch from 9c0c8d5 to 65cc6fa Compare November 20, 2024 10:10

sever-sever requested a review from dmbaturin November 20, 2024 11:30

c-po requested changes Nov 20, 2024

View reviewed changes

dmbaturin approved these changes Nov 21, 2024

View reviewed changes

ipoe_server: T6872: Add the ability to configure LUA scripts and user…

e8e72e2

…name

natali-rs1985 force-pushed the T6872 branch from 613cb4e to e8e72e2 Compare November 21, 2024 12:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ipoe_server: T6872: Add the ability to configure LUA scripts and username #4196

ipoe_server: T6872: Add the ability to configure LUA scripts and username #4196

natali-rs1985 commented Nov 18, 2024 •

edited

Loading

github-actions bot commented Nov 18, 2024 •

edited

Loading

dmbaturin Nov 19, 2024

dmbaturin Nov 19, 2024

dmbaturin Nov 19, 2024

dmbaturin Nov 19, 2024

dmbaturin Nov 19, 2024

c-po Nov 20, 2024

sever-sever Nov 20, 2024 •

edited

Loading

dmbaturin Nov 21, 2024

sever-sever Nov 22, 2024 •

edited

Loading

dmbaturin left a comment

github-actions bot commented Nov 21, 2024

	<description>Name of function in lua file to construct username</description>
	<description>Name of the function in the Lua file to construct usernames with</description>

	<help>File containing lua function for create username</help>
	<help>Lua script file for constructing user names</help>

	<description>File with LUA script</description>
	<description>File with Lua script</description>

	'Option "client-subnet" and "vlan" are mutually exclusive, '
	'Options "client-subnet" and "vlan" are mutually exclusive, '

	'Can configure username with LUA script only for RADIUS authentication'
	'Can configure username with Lua script only for RADIUS authentication'

ipoe_server: T6872: Add the ability to configure LUA scripts and username #4196

Are you sure you want to change the base?

ipoe_server: T6872: Add the ability to configure LUA scripts and username #4196

Conversation

natali-rs1985 commented Nov 18, 2024 • edited Loading

Change Summary

Types of changes

Related Task(s)

Related PR(s)

Component(s) name

Proposed changes

How to test

Smoketest result

Checklist:

github-actions bot commented Nov 18, 2024 • edited Loading

dmbaturin Nov 19, 2024

Choose a reason for hiding this comment

dmbaturin Nov 19, 2024

Choose a reason for hiding this comment

dmbaturin Nov 19, 2024

Choose a reason for hiding this comment

dmbaturin Nov 19, 2024

Choose a reason for hiding this comment

dmbaturin Nov 19, 2024

Choose a reason for hiding this comment

c-po Nov 20, 2024

Choose a reason for hiding this comment

sever-sever Nov 20, 2024 • edited Loading

Choose a reason for hiding this comment

dmbaturin Nov 21, 2024

Choose a reason for hiding this comment

sever-sever Nov 22, 2024 • edited Loading

Choose a reason for hiding this comment

dmbaturin left a comment

Choose a reason for hiding this comment

github-actions bot commented Nov 21, 2024

Details

natali-rs1985 commented Nov 18, 2024 •

edited

Loading

github-actions bot commented Nov 18, 2024 •

edited

Loading

sever-sever Nov 20, 2024 •

edited

Loading

sever-sever Nov 22, 2024 •

edited

Loading