Skip to content
New issue

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

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

Already on GitHub? # to your account

mesh-vpn: fully abstract VPN methods #2102

Merged
merged 1 commit into from
May 26, 2021

Conversation

blocktrron
Copy link
Member

This fully abstracts VPN methods, making gluon-mesh-vpn-fastd and
gluon-mesh-vpn-tunneldigger completely self-contained.

Provide a LUA interface for generic interacting with VPN methods in
gluon-mesh-vpn-core and web packages.

This also adds the ability to install tunneldigger and fastd to the same
image, selecting the VPN method based on the selected domain.

@neocturne
Copy link
Member

I see a few conceptual issues that need to be discuss:

  • Should the chosen VPN implementation only be a matter of site/domain config, or should is be possible for the user to change this (for example turning the security/performance mode of gluon-web-mesh-vpn-fastd into a generic switch hat selects between fastd and tunneldigger)?
  • There are a few places in this PR that query a specific method whether it is enabled. This doesn't make sense to me - VPN enable status should be a single value in /etc/config/gluon. The enabled flags of the individual VPN implementation config files are dependent values that are populated based on the global enable status and the selected VPN implementation; they should only be set, but never be queried by Gluon's scripts (except for migration purposes, in case the global enable status doesn't exist yet)

@blocktrron
Copy link
Member Author

@NeoRaider

For the first point, I would propose to leave it to the site configuration for now. The goal of this PR is not to add this functionality but to remove the core VPN package dependencies on the specific VPN implementation packages.

Your second remark is correct. I've simply didn't correctly interpret the context and it's goal for which the specific VPN states are extracted. I'll fix that.

@blocktrron blocktrron force-pushed the mesh-vpn-abstraction branch 2 times, most recently from 8fff333 to d84c0a1 Compare September 9, 2020 04:07
@rotanid rotanid added this to the 2020.3 milestone Sep 13, 2020
@rotanid rotanid requested a review from neocturne October 19, 2020 21:02
end
uci:save('tunneldigger')
Copy link
Member

Choose a reason for hiding this comment

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

You are removing the code that disables the unavailable VPNs, this is not correct.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I wrote the code this way so that even VPNs that are not installed anymore get disabled (for the case when for example fastd is installed, but gluon-mesh-vpn-fastd isn't).

What we really want is a generic facility that allows removed packages to clean up after themselves on firmware upgrades, that way we wouldn't need a full list of packages that could exist. I have a few ideas how to solve this...

@rotanid rotanid added the 2. status: waiting-on-author Waiting on some action from the author label Oct 24, 2020
@CodeFetch
Copy link
Contributor

@NeoRaider

Should the chosen VPN implementation only be a matter of site/domain config, or should is be possible for the user to change this (for example turning the security/performance mode of gluon-web-mesh-vpn-fastd into a generic switch hat selects between fastd and tunneldigger)?

I've been working on IPSec using L2TP where I'd like to be able to let the user configure the encryption this way. Please make it as generic as possible or we will rip it out again, later.

AiyionPrime added a commit to AiyionPrime/gluon that referenced this pull request Apr 24, 2021
AiyionPrime added a commit to AiyionPrime/gluon that referenced this pull request Apr 24, 2021
AiyionPrime added a commit to AiyionPrime/gluon that referenced this pull request Apr 24, 2021
@mweinelt mweinelt mentioned this pull request Apr 27, 2021
2 tasks
@blocktrron blocktrron force-pushed the mesh-vpn-abstraction branch 4 times, most recently from 3688b9c to fae8219 Compare April 28, 2021 21:35
@blocktrron
Copy link
Member Author

Fixed all outstanding comments.

Regarding the teardown of old VPN methods - I tend to leave it as - is for now. This might not be perfect, but we remain consistent in the behavior.

For the future, we can have a teardown manager which manages teardown in case a package does no exist anymore. The goal of allowing a third-party VPN method from a community feed is still possible this way, they have to take care of teardown themselves though.

@AiyionPrime Not build / ru ntested yet although the linter is happy.

@AiyionPrime
Copy link
Member

Thanks!
I just compiled and installed it on one of my cudy wr1000 (ramips-mt76x8).
http://[2001:678:978:114:b64b:d6ff:fe22:4ebc]/cgi-bin/status

You five got access to it, welcome to my kitchen; obviously leave the rest of the network untouched.

@blocktrron blocktrron force-pushed the mesh-vpn-abstraction branch from fae8219 to 251d550 Compare April 29, 2021 23:22
@blocktrron blocktrron force-pushed the mesh-vpn-abstraction branch from e4b5414 to ef5fe30 Compare May 17, 2021 14:31
@github-actions github-actions bot added 3. topic: config-mode This is about the configuration mode 3. topic: fastd 3. topic: multidomain 3. topic: package Topic: Gluon Packages 3. topic: tunneldigger This is about tunneldigger, a L2TP brokering solution labels May 17, 2021
@blocktrron
Copy link
Member Author

Addressed comments and rebased on #2223

@blocktrron blocktrron force-pushed the mesh-vpn-abstraction branch from ef5fe30 to fca24cc Compare May 19, 2021 11:42
@AiyionPrime
Copy link
Member

There are currently 17 occurrences of proto, are they meant to stay?

@blocktrron blocktrron force-pushed the mesh-vpn-abstraction branch 2 times, most recently from 330ed59 to 4d737b8 Compare May 19, 2021 13:44
@AiyionPrime
Copy link
Member

Looks better, I'll test later, when the dust has settled a little.

@blocktrron
Copy link
Member Author

Works from my testing.

@AiyionPrime
Copy link
Member

Compiles and gluon-reconfigure does it's job. I'll close blocktrron#6 then.

Copy link
Member

@AiyionPrime AiyionPrime left a comment

Choose a reason for hiding this comment

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

Just reread the code once more.

if meshvpn_enabled then
pubkey = util.trim(util.exec('/etc/init.d/fastd show_key mesh_vpn'))
if has_vpn and vpn.enabled() then
local _, active_vpn = vpn.get_active_provider()

This comment was marked as resolved.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes

Copy link
Member

Choose a reason for hiding this comment

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

resolved in 98a1c19

end

function M.uci_sections()
return {'tunneldigger'}

This comment was marked as resolved.

Copy link
Member Author

Choose a reason for hiding this comment

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

These methods are now obsolete anyways, as uci::commit is called globally on reconfigure anyways. I removed them now.

Copy link
Member

Choose a reason for hiding this comment

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

resolved in 98a1c19

pubkey = util.trim(util.exec("/etc/init.d/fastd show_key mesh_vpn"))
if vpn.enabled() then
local _, active_vpn = vpn.get_active_provider()
pubkey = active_vpn.public_key()

This comment was marked as resolved.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes

Copy link
Member

Choose a reason for hiding this comment

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

resolved in 98a1c19

local out = {}

for _, v in ipairs(util.glob('/lib/gluon/mesh-vpn/provider/*')) do
table.insert(out, v:match('([^/]+)$'))

This comment was marked as resolved.

This comment was marked as resolved.

@blocktrron blocktrron force-pushed the mesh-vpn-abstraction branch from 4d737b8 to 3ee6a02 Compare May 22, 2021 04:53
This fully abstracts VPN methods, making gluon-mesh-vpn-fastd and
gluon-mesh-vpn-tunneldigger completely self-contained.

Provide a LUA interface for generic interacting with VPN methods in
gluon-mesh-vpn-core and web packages.

This also adds the ability to install tunneldigger and fastd to the same
image, selecting the VPN method based on the selected domain.

Signed-off-by: David Bauer <mail@david-bauer.net>
@blocktrron blocktrron force-pushed the mesh-vpn-abstraction branch from 3ee6a02 to 98a1c19 Compare May 22, 2021 04:54
Copy link
Member

@AiyionPrime AiyionPrime left a comment

Choose a reason for hiding this comment

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

Looks good to me.
@NeoRaider or @mweinelt may you review once more?

@AiyionPrime
Copy link
Member

I compiled the code before the weekend; and just did so again. It's deployed on http://meshvpnabstracttest.n.ffh.zone/cgi-bin/status.
You all still should have access to it.

@blocktrron
Copy link
Member Author

Verified initial setup & upgrades are working in a fastd environment.

Let's do this.

@blocktrron blocktrron merged commit 71c3b83 into freifunk-gluon:master May 26, 2021
@blocktrron blocktrron deleted the mesh-vpn-abstraction branch May 26, 2021 19:15
herbetom added a commit to herbetom/gluon that referenced this pull request Dec 18, 2023
The Gluon Tunneldigger protocol handler hasn't received the care and
attention a core package, especially a VPN protocol, should.

Due to the works in freifunk-gluon#2102 it's no longer required to maintain
VPN handlers in the Gluon Core and they can be instead included via a
package feed.

Users who wish to continue using a supported core Gluon package are
encouraged to take a look at fastd in conjunction with it's null@l2tp method.
It offers roughly the same performance (it's using the same principle
of operation) and supports connections via IPv6 as a bonus.

In general it should be possible to change the VPN used protocol
asynchronously with a normal update.

If Gluon users wish to continue using tunneldigger the recomendation
is to participate in the development of tunneldigger inside our
community-packages.
herbetom added a commit to herbetom/gluon that referenced this pull request Dec 21, 2023
The Gluon Tunneldigger protocol handler hasn't received the care and
attention a core package, especially a VPN protocol, should.

Due to the works in freifunk-gluon#2102 it's no longer required to maintain
VPN handlers in the Gluon Core and they can be instead included via a
package feed.

Users who wish to continue using a supported core Gluon package are
encouraged to take a look at fastd in conjunction with it's null@l2tp method.
It offers roughly the same performance (it's using the same principle
of operation) and supports connections via IPv6 as a bonus.

In general it should be possible to change the VPN used protocol
asynchronously with a normal update.

If Gluon users wish to continue using tunneldigger the recomendation
is to participate in the development of tunneldigger inside our
community-packages.
herbetom added a commit to herbetom/gluon that referenced this pull request Dec 29, 2023
The Gluon Tunneldigger protocol handler hasn't received the care and
attention a core package, especially a VPN protocol, should.

Due to the works in freifunk-gluon#2102 it's no longer required to maintain
VPN handlers in the Gluon Core and they can be instead included via a
package feed.

Users who wish to continue using a supported core Gluon package are
encouraged to take a look at fastd in conjunction with it's null@l2tp method.
It offers roughly the same performance (it's using the same principle
of operation) and supports connections via IPv6 as a bonus.

In general it should be possible to change the VPN used protocol
asynchronously with a normal update.

If Gluon users wish to continue using tunneldigger the recomendation
is to participate in the development of tunneldigger inside our
community-packages.
blocktrron pushed a commit that referenced this pull request Jan 5, 2024
The Gluon Tunneldigger protocol handler hasn't received the care and
attention a core package, especially a VPN protocol, should.

Due to the works in #2102 it's no longer required to maintain
VPN handlers in the Gluon Core and they can be instead included via a
package feed.

Users who wish to continue using a supported core Gluon package are
encouraged to take a look at fastd in conjunction with it's null@l2tp method.
It offers roughly the same performance (it's using the same principle
of operation) and supports connections via IPv6 as a bonus.

In general it should be possible to change the VPN used protocol
asynchronously with a normal update.

If Gluon users wish to continue using tunneldigger the recomendation
is to participate in the development of tunneldigger inside our
community-packages.
misanthropos pushed a commit to misanthropos/gluon that referenced this pull request Apr 29, 2024
The Gluon Tunneldigger protocol handler hasn't received the care and
attention a core package, especially a VPN protocol, should.

Due to the works in freifunk-gluon#2102 it's no longer required to maintain
VPN handlers in the Gluon Core and they can be instead included via a
package feed.

Users who wish to continue using a supported core Gluon package are
encouraged to take a look at fastd in conjunction with it's null@l2tp method.
It offers roughly the same performance (it's using the same principle
of operation) and supports connections via IPv6 as a bonus.

In general it should be possible to change the VPN used protocol
asynchronously with a normal update.

If Gluon users wish to continue using tunneldigger the recomendation
is to participate in the development of tunneldigger inside our
community-packages.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
0. type: enhancement The changeset is an enhancement 2. status: waiting-on-author Waiting on some action from the author 3. topic: config-mode This is about the configuration mode 3. topic: fastd 3. topic: package Topic: Gluon Packages 3. topic: tunneldigger This is about tunneldigger, a L2TP brokering solution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants