Project

General

Profile

Actions

Todo #16128

open

Sanitize pppoe configuration parameters

Added by Kristof Provost 5 months ago. Updated 15 days ago.

Status:
Feedback
Priority:
Normal
Assignee:
Category:
PPP Interfaces
Target version:
Start date:
Due date:
% Done:

100%

Estimated time:
Plus Target Version:
25.11
Release Notes:
Default

Description

A user reports (https://forum.netgate.com/topic/197026/25-03-b-20250306-0140-if_pppoe-kernel-module-chap-failure/10 ) that a leading space in a password used to be ignored as it was written to the mpd5 configuration file without quotes around it.
That's not the case with if_pppoe, so it's considered part of the password.

The if_pppoe behaviour is probably what we want, but we ought to at least document this for TAC.

We should also check if we're correctly handling passwords with symbols such as ", ', \, ... in them here: https://gitlab.netgate.com/pfSense/pfSense/-/blob/master/src/etc/inc/interfaces.inc?ref_type=heads#L2311

Actions #1

Updated by Bill Meeks 5 months ago

Just a thought -- but it would potentially be helpful if password validation logic would check for leading or trailing spaces in entered passwords and warn the user when saving by displaying the standard pfSense warning message at the top of the page. Make it a warning and not a fail, so the user can proceed with the space(s) if desired. Unintended spaces in passwords have been the source of several mysterious problems <grin>.

Actions #2

Updated by Kris Phillips 4 months ago

  • Status changed from New to Confirmed

Marking as Confirmed for now, since this is a known difference in behavior.

Actions #3

Updated by Scott Ashcroft 3 months ago

Passwords which begin with exclamation mark (!) are broken see:

https://forum.netgate.com/post/1216202

The proper fix would be to base64 encode the password before passing it to the command line and so avoid all the escaping issues.
The command would then do the decode before passing it to the kernel module.
In theory PPP passwords could contain all sorts of mad characters as all bytes values including NUL are valid. Passing them directly as a command line argument will always be dangerous.
Having the connection not work is probably the least worst thing that could happen.

Actions #4

Updated by Steve Wheeler about 1 month ago

  • Target version set to 2.9.0
  • Plus Target Version set to 25.11
  • Affected Version set to 2.8.0
  • Affected Architecture All added

Additional examples have been found. Still an issue in 25.07.

Actions #5

Updated by Marcos M 17 days ago

  • Status changed from Confirmed to In Progress
  • Assignee set to Marcos M
Actions #6

Updated by Marcos M 15 days ago

  • Tracker changed from Bug to Todo
  • Subject changed from if_pppoe: PHP password handling to Sanitize pppoe configuration parameters
  • Affected Version deleted (2.8.0)
  • Affected Architecture deleted (All)
Actions #7

Updated by Marcos M 15 days ago

  • Status changed from In Progress to Feedback
  • % Done changed from 0 to 100
Actions

Also available in: Atom PDF