Skip to content

pppd: Remove dead check for existence of route to remote IP#585

Merged
paulusmack merged 1 commit into
masterfrom
dev
May 14, 2026
Merged

pppd: Remove dead check for existence of route to remote IP#585
paulusmack merged 1 commit into
masterfrom
dev

Conversation

@paulusmack
Copy link
Copy Markdown
Collaborator

The have_route_to() call in auth_ip_addr() is dead code, so remove it. It can never be reached because, to reach it, we would have to have !auth_required && !allow_any_ip && !privileged, but auth_check_options() sets auth_required to 1 in that case.

In other words, the logic established by previous commits is this:

If pppd is run by non-root and the noauth option isn't in effect, or if the auth option is given, the peer has to authenticate, and the IP addresses are controlled by the secrets file, with no reachability check.

If pppd is run by root and authentication isn't required (i.e. the auth option is not given), the peer can use any IP address, with no reachability check.

In neither case is the reachability check done by have_route_to() used.

The have_route_to() call in auth_ip_addr() is dead code, so remove it.
It can never be reached because, to reach it, we would have to have
!auth_required && !allow_any_ip && !privileged, but auth_check_options()
sets auth_required to 1 in that case.

In other words, the logic established by previous commits is this:

If pppd is run by non-root and the noauth option isn't in effect, or
if the auth option is given, the peer has to authenticate, and the IP
addresses are controlled by the secrets file, with no reachability
check.

If pppd is run by root and authentication isn't required (i.e. the
auth option is not given), the peer can use any IP address, with no
reachability check.

In neither case is the reachability check done by have_route_to() used.

Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
@Neustradamus
Copy link
Copy Markdown
Member

@jkroonza: Have you seen this @paulusmack PR?

@jkroonza
Copy link
Copy Markdown
Contributor

I have not, but the logic sounds OK. It does make me think we should require an explicit auth or noauth to be provided though, or auth should be default.

@paulusmack paulusmack merged commit d5c3917 into master May 14, 2026
61 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants