Skip to content

Linux IPC hardening: lantern-group authorization#338

Closed
atavism wants to merge 5 commits into
mainfrom
atavism/linux-test
Closed

Linux IPC hardening: lantern-group authorization#338
atavism wants to merge 5 commits into
mainfrom
atavism/linux-test

Conversation

@atavism

@atavism atavism commented Feb 24, 2026

Copy link
Copy Markdown
Contributor

Lantern’s Linux app needs to control lanternd without requiring sudo for normal users. This PR secures that path by switching IPC to a least-privilege model (root:lantern, 0660, allow root or users in the lantern group) instead of a world-writable socket.

Copilot AI review requested due to automatic review settings February 24, 2026 20:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Hardens Lantern’s Linux IPC by moving from a world-writable socket to a least-privilege model using a dedicated lantern group and per-peer authorization.

Changes:

  • Adds lantern control group concept and uses root:lantern ownership with 0660 permissions on Linux.
  • Extends peer identity to include inControlGroup and updates access checks accordingly.
  • Adds Linux-specific group membership detection for connecting peers.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
vpn/ipc/usr.go Extends usr with inControlGroup for authorization decisions.
vpn/ipc/socket.go Sets Linux socket ownership/permissions to root:lantern and 0660.
vpn/ipc/middlewares.go Updates authorization to allow admins or members of the control group.
vpn/ipc/conn_nonwindows.go Determines connecting peer’s group membership (Linux) to drive authorization.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread vpn/ipc/middlewares.go Outdated

func peerCanAccess(peer usr) bool {
return peer.isAdmin
return peer.isAdmin || peer.inControlGroup

Copilot AI Feb 24, 2026

Copy link

Choose a reason for hiding this comment

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

This change unintentionally blocks root from accessing the IPC API on Linux: getPeerUser() doesn’t set isAdmin on Linux, and root typically isn’t in the lantern group, so peerCanAccess() will return false even though the socket permissions allow root. Consider explicitly allowing uid 0 (e.g., peer.uid == \"0\") or setting isAdmin for uid 0 in the Linux path.

Suggested change
return peer.isAdmin || peer.inControlGroup
return peer.isAdmin || peer.inControlGroup || peer.uid == "0"

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sounds right!

Comment thread vpn/ipc/socket.go Outdated
Comment thread vpn/ipc/conn_nonwindows.go
@garmr-ulfr

Copy link
Copy Markdown
Collaborator

The socket needs to be world-readable so anyone with sudo permissions can connect to the IPC without requiring them to run using sudo, even if they're not in the lanternd group. We'll be adding a headless CLI (as soon as I clean mine up and add a couple of things that are missing), for which there won't be a lanternd group. The IPC verifies peers have sudo permissions, so it is fine as it.

@atavism

atavism commented Feb 24, 2026

Copy link
Copy Markdown
Contributor Author

We can support headless CLI without reverting to world-accessible sockets by keeping root:lantern 0660 and managing access via lantern group. That keeps no-sudo UX while preserving least-privilege

@garmr-ulfr

Copy link
Copy Markdown
Collaborator

That requires creating and managing the control group. If IPC verifies the peer has sudo permissions, I don't see why we would need to also require a control group.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +94 to +98
func linuxUserInControlGroup(u *user.User) (bool, error) {
controlGroupGID, err := controlGroupGID()
if err != nil {
return false, err
}

Copilot AI Feb 24, 2026

Copy link

Choose a reason for hiding this comment

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

conn_nonwindows.go is built on Android/iOS (//go:build !windows), but the newly added Linux group-check code depends on controlGroupGID() / controlGroupGIDInt() which are only defined in control_group_nonwindows.go (!android && !ios && !windows). This will cause Android/iOS builds of package ipc to fail with an undefined symbol error. Consider moving the Linux-specific helpers (linuxUserInControlGroup and the runtime.GOOS == "linux" branch) into a //go:build linux file, or widening/providing stub implementations for controlGroupGID* on mobile builds.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The above sounds right to me!

Comment thread vpn/ipc/control_group_nonwindows.go Outdated
func controlGroupInfo() (*user.Group, error) {
group, err := user.LookupGroup(controlGroup)
if err != nil {
return nil, fmt.Errorf("lookup %s group: %w", controlGroup, err)

Copilot AI Feb 24, 2026

Copy link

Choose a reason for hiding this comment

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

When user.LookupGroup(controlGroup) fails (e.g., the lantern group doesn’t exist), this propagates as a hard startup failure. The error message would be more actionable if it explicitly explained how to remediate (create the group and add users, or configure a different group if supported), since this is a new deployment prerequisite.

Suggested change
return nil, fmt.Errorf("lookup %s group: %w", controlGroup, err)
return nil, fmt.Errorf("lookup %s group: %w. Ensure the %s group exists on this system and contains the appropriate users, or configure a different control group if supported.", controlGroup, err, controlGroup)

Copilot uses AI. Check for mistakes.
@garmr-ulfr

Copy link
Copy Markdown
Collaborator

Using a control group also means other users with sudo permissions will have to be added to the group manually. Anyone with with sudo permissions needs to be able to run it without having to use sudo.

@atavism

atavism commented Feb 24, 2026

Copy link
Copy Markdown
Contributor Author

Relying on canSudo() alone means exposing a root-control socket to all local users and treating middleware auth as the primary boundary, which is not least-privilege. The standard/safer model is deny-by-default at the OS layer and then authorize approved clients on top

@garmr-ulfr

garmr-ulfr commented Feb 24, 2026

Copy link
Copy Markdown
Collaborator

Relying on canSudo() alone means exposing a root-control socket to all local users and treating middleware auth as the primary boundary, which is not least-privilege.

Normally, I would agree with you on this, but in this case it's perfectly fine because the IPC does the authentication using unix peer credentials. Other programs also do that.

@atavism

atavism commented Feb 24, 2026

Copy link
Copy Markdown
Contributor Author

Peer creds are good for identity, but that still doesn’t make a world-accessible root IPC socket fine. It increases attack surface (any local user can connect/flood/probe) and removes the first security boundary socket ACLs

@garmr-ulfr

garmr-ulfr commented Feb 24, 2026

Copy link
Copy Markdown
Collaborator

Peer creds are good for identity,

Right, and then we verify that they have sudo permissions. Because we're using peer creds to get the identity, we know for certain who the user is.

Even if any user can connect to it, they won't be able to do anything because the IPC will reject the request.

@myleshorton

myleshorton commented Feb 26, 2026

Copy link
Copy Markdown
Contributor

I asked Claude to analyze this debate because I can't think for myself anymore. Here's what it had to say:

Given all this, the 0660 vs 0666 argument is mostly security theater for the actual threat model. Neither setting protects against the realistic attacks (local detection software, network-level censorship). The practical concerns garmr-ulfr raises about operational friction are more impactful than the marginal security benefit of filesystem ACLs on the socket.

@garmr-ulfr

garmr-ulfr commented Feb 26, 2026

Copy link
Copy Markdown
Collaborator

Another concern is that most distros (including Ubuntu and Debian) require you to log out and log back in for the group membership changes to take effect because they're only evaluated at login. Some distros even require a full reboot. This will lead to an increase of issue reports as there will be a decent amount of users that didn't do that.

Also, a much smaller concern, it's a bit inconvenient. 😃

@teamlantern

Copy link
Copy Markdown

This PR has been inactive for 30 days.

Waiting on: @myleshorton

It will be closed automatically in 7 days if no action is taken.

@teamlantern

Copy link
Copy Markdown

This PR has been inactive for 30 days.

Waiting on: @myleshorton

It will be closed automatically in 7 days if no action is taken.

@teamlantern teamlantern removed the stale label May 28, 2026
@atavism

atavism commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

Closing this as stale/no longer aligned with the direction settled in the thread. The PR moves Linux IPC to a root:lantern 0660 control-group model, but the discussion landed on keeping IPC access compatible with users who have sudo permissions and relying on peer credential based authorization, avoiding the extra group-management/login-cycle friction. If we revisit Linux IPC hardening, it should probably start from a fresh design that preserves that UX.

@atavism atavism closed this Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants