FreeRADIUS / freeradius-server

FreeRADIUS - A multi-protocol policy server.
http://freeradius.org
GNU General Public License v2.0
2.11k stars 1.08k forks source link

double quote gets escaped in User-Password #2018

Closed armitasp closed 7 years ago

armitasp commented 7 years ago

Issue type

Defect/Feature description

double quotes in User-Passwords become escaped e.g. test"1234 becomes test\"1234

Found in version 3.0.14

How to reproduce issue

perform a pap authentication with a password containing "

Output of [radiusd|freeradius] -X showing issue occurring

(1) Received Access-Request Id 116 from 131.231.13.21:1025 to 131.231.9.2:1812 length 190
(1)   User-Name = "test-user"
(1)   User-Password = "test\"1234"
(1)   NAS-Port = 1130496
(1)   Called-Station-Id = "193.62.24.107"
(1)   Calling-Station-Id = "131.231.153.30"
(1)   NAS-Port-Type = Virtual
(1)   Tunnel-Client-Endpoint:0 = "131.231.153.30"
(1)   NAS-IP-Address = 131.231.13.21
(1)   Cisco-AVPair = "ip:source-ip=131.231.153.30"
(1)   ASA-TunnelGroupName = "DefaultWEBVPNGroup"
(1)   ASA-ClientType = Clientless-SSL-VPN
(1) # Executing section authorize from file /etc/raddb/sites-enabled/vpn
(1)   authorize {
(1)     policy filter_username {
(1)       if (!&User-Name) {
(1)       if (!&User-Name)  -> FALSE
(1)       if (&User-Name =~ / /) {
(1)       if (&User-Name =~ / /)  -> FALSE
(1)       if (&User-Name =~ /@.*@/ ) {
(1)       if (&User-Name =~ /@.*@/ )  -> FALSE
(1)       if (&User-Name =~ /\.\./ ) {
(1)       if (&User-Name =~ /\.\./ )  -> FALSE
(1)       if ((&User-Name =~ /@/) && (&User-Name !~ /@(.+)\.(.+)$/))  {
(1)       if ((&User-Name =~ /@/) && (&User-Name !~ /@(.+)\.(.+)$/))   -> FALSE
(1)       if (&User-Name =~ /\.$/)  {
(1)       if (&User-Name =~ /\.$/)   -> FALSE
(1)       if (&User-Name =~ /@\./)  {
(1)       if (&User-Name =~ /@\./)   -> FALSE
(1)     } # policy filter_username = notfound
(1)     [preprocess] = ok
(1) suffix: Checking for suffix after "@"
(1) suffix: No '@' in User-Name = "test-user", skipping NULL due to config.
(1)     [suffix] = noop
(1) ntdomain: Checking for prefix before "\"
(1) ntdomain: No '\' in User-Name = "test-user", looking up realm NULL
(1) ntdomain: No such realm "NULL"
(1)     [ntdomain] = noop
(1)     if (!control:Auth-Type && User-Password) {
(1)     if (!control:Auth-Type && User-Password)  -> TRUE
(1)     if (!control:Auth-Type && User-Password)  {
(1)       update control {
(1)         &Auth-Type := ntlm_auth
(1)       } # update control = noop
(1)     } # if (!control:Auth-Type && User-Password)  = noop
(1)   } # authorize = ok
(1) Found Auth-Type = ntlm_auth
(1) # Executing group from file /etc/raddb/sites-enabled/vpn
(1)   Auth-Type ntlm_auth {
(1) ntlm_auth: Executing: /bin/ntlm_auth --request-nt-key --username=%{%{Stripped-User-Name}:-%{%{User-Name}:-None}} --domain=LUNET --password=%{User-Password}:
(1) ntlm_auth: EXPAND --username=%{%{Stripped-User-Name}:-%{%{User-Name}:-None}}
(1) ntlm_auth:    --> --username=test-user
(1) ntlm_auth: EXPAND --password=%{User-Password}
(1) ntlm_auth:    --> --password=test\"1234
(1) ntlm_auth: ERROR: Program returned code (1) and output 'NT_STATUS_WRONG_PASSWORD: Wrong Password (0xc000006a)'
(1)     [ntlm_auth] = reject
(1)   } # Auth-Type ntlm_auth = reject
alanbuxey commented 7 years ago

Hmmm. Quoted and back tick quoted are dynamically expanded. What about ' or &User-Password in that ntlm_auth clause?

alan

armitasp commented 7 years ago

On 10 Jul 2017, at 22:03, Alan Buxey notifications@github.com wrote:

Hmmm. Quoted and back tick quoted are dynamically expanded. What about ‘ or

Single quotes seem fine:

(3) ntlm_auth: Executing: /bin/ntlm_auth --request-nt-key --username=%{%{Stripped-User-Name}:-%{%{User-Name}:-None}} --domain=LUNET --password=%{User-Password}: (3) ntlm_auth: EXPAND --username=%{%{Stripped-User-Name}:-%{%{User-Name}:-None}} (3) ntlm_auth: --> —username=test-user (3) ntlm_auth: EXPAND --password=%{User-Password} (3) ntlm_auth: --> --password=test’1234

&User-Password in that ntlm_auth clause?

&User-Password doesn’t seem to work

Auth-Type ntlm_auth { (0) ntlm_auth: Executing: /bin/ntlm_auth --request-nt-key --username=%{%{Stripped-User-Name}:-%{%{User-Name}:-None}} --domain=LUNET --password=&User-Password: (0) ntlm_auth: EXPAND --username=%{%{Stripped-User-Name}:-%{%{User-Name}:-None}} (0) ntlm_auth: --> —username=test-user (0) ntlm_auth: ERROR: Program returned code (1) and output 'NT_STATUS_WRONG_PASSWORD: Wrong Password (0xc000006a)' (0) [ntlm_auth] = reject (0) } # Auth-Type ntlm_auth = reject

Scott

alanbuxey commented 7 years ago

err, no, I meant putting the %{User-Password} into single quotes...not whether single quotes werent affected ;-)

eg use --password='%{User-Password}' , or --password="%{User-Password}" ?

alan

On 10 July 2017 at 22:15, Scott Armitage notifications@github.com wrote:

On 10 Jul 2017, at 22:03, Alan Buxey notifications@github.com wrote:

Hmmm. Quoted and back tick quoted are dynamically expanded. What about ‘ or

Single quotes seem fine:

(3) ntlm_auth: Executing: /bin/ntlm_auth --request-nt-key --username=%{%{Stripped-User-Name}:-%{%{User-Name}:-None}} --domain=LUNET --password=%{User-Password}: (3) ntlm_auth: EXPAND --username=%{%{Stripped-User- Name}:-%{%{User-Name}:-None}} (3) ntlm_auth: --> —username=test-user (3) ntlm_auth: EXPAND --password=%{User-Password} (3) ntlm_auth: --> --password=test’1234

&User-Password in that ntlm_auth clause?

&User-Password doesn’t seem to work

Auth-Type ntlm_auth { (0) ntlm_auth: Executing: /bin/ntlm_auth --request-nt-key --username=%{%{Stripped-User-Name}:-%{%{User-Name}:-None}} --domain=LUNET --password=&User-Password: (0) ntlm_auth: EXPAND --username=%{%{Stripped-User- Name}:-%{%{User-Name}:-None}} (0) ntlm_auth: --> —username=test-user (0) ntlm_auth: ERROR: Program returned code (1) and output 'NT_STATUS_WRONG_PASSWORD: Wrong Password (0xc000006a)' (0) [ntlm_auth] = reject (0) } # Auth-Type ntlm_auth = reject

Scott

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/FreeRADIUS/freeradius-server/issues/2018#issuecomment-314247101, or mute the thread https://github.com/notifications/unsubscribe-auth/ACE-VYG6JUBZPQrK72it5CrnONswA472ks5sMpRxgaJpZM4OTYvX .

armitasp commented 7 years ago

On 10 Jul 2017, at 22:33, Alan Buxey notifications@github.com wrote:

err, no, I meant putting the %{User-Password} into single quotes...not whether single quotes werent affected ;-)

eg use --password='%{User-Password}' , or --password="%{User-Password}” ?

Ah, OK.

Tried it didn’t make any difference.

alandekok commented 7 years ago

The double quote gets escaped when printing. Especially when running another program / shell script.

I'm not sure there's a simple fix which works here. We could just stop escaping... but that would potentially allow for malicious people to mess with the scripts you execute.

The better alternative is to use winbind instead of ntlm_auth. That avoids the issue entirely.

armitasp commented 7 years ago

On 11 Jul 2017, at 13:30, Alan DeKok notifications@github.com wrote:

The double quote gets escaped when printing. Especially when running another program / shell script.

I'm not sure there's a simple fix which works here. We could just stop escaping... but that would potentially allow for malicious people to mess with the scripts you execute.

The better alternative is to use winbind instead of ntlm_auth. That avoids the issue entirely.

Indeed. I’ll give it a try and see if that works.

Scott