Allow special characters on pptp password

jimp

It depends on what you mean by "special" characters. Printable (read: on keyboard) symbols are probably safe, so long as you don't lead with a ! (mpd will do fun things with that). Whereas international characters are really not allowed there, and would likely break things.

The input validation may be a tad too strict there.

ggzengel

My password generator gives me passwords for ascii code 32 to 126:
!"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[]^_`abcdefghijklmnopqrstuvwxyz{|}~

With ipsec it works. I think you have to use the same code like ipsec.

jimp

ipsec is handled by racoon, pptp is handled by mpd. Just because one works does not mean the other can/will/does.

I don't have the mpd docs handy (or the time to go hunting them down) but if someone can turn up a list of valid password characters for mpd, I can fix the validation.

ggzengel

http://mpd.sourceforge.net/doc5/mpd14.html

There is no restriction. They allow also carrige return (\r) and new line (\n) in a password.
That means you have to replace:
" to "
\ to \

It's not allowed a "!" as first character.

jimp

ok, I'll try to remember to fix that sometime this week. You might want to open up a ticket on redmine with a link back to this thread. Call it "Relax input validation for PPTP Passwords" or something similar. That way it doesn't get lost.

In addition to the link to the thread, link to the mpd docs as you did here, just for good measure. Someone else may get to it before me if they have all the needed information.

jimp

Should be OK now
https://github.com/bsdperimeter/pfsense/commit/2c466077934c3812aed9d15b77ab515e4b3e116d

ggzengel

Thank you.

A little piece is missing:
In webconfig password

\r"

will be

test "\r\""

in mpd.secret but it should be

test "\\r\""

Please change (in this order):

       foreach ($pptpdcfg['user'] as $user) { 
+          $pass = str_replace('\', '\\', $user['password']); 
+          $pass = str_replace('"', '\"', $pass); 
-           $pass = str_replace('"', '\"', $user['password']);
          $mpdsecret .= "{$user['name']} \"{$pass}\" {$user['ip']}\n"; 
        }

jimp

There is no way you can put a newline/carriage return into the GUI password field. It also wouldn't pass the character test I put in. I do need to escape \ though I see.

ggzengel

The other way around is the right way.
If you use \r as password the mpd would expect an carrige return as password.
If you use aa\ as password there will be "aa" in your mpd.secret and it's missing a closing ".

jimp

Should be fixed by escaping , as I did here:
https://github.com/bsdperimeter/pfsense/commit/4222087e90badb854274109fc32de422c5c25221