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

HEAD: mschap module "Exec" Child returned 0 #259

Closed alanbuxey closed 11 years ago

alanbuxey commented 11 years ago

hi,

we use a number of mschap named modules... eg NEWAD-realm is for talking to our newer AD when the username has a realm in it (usually because eg a realm is on the innerID) anyway, thats to explain the name of the module...it could have been just 'mschap' - HEAD doesnt work when invoking ntlm_auth - used to work fine on 2.x days and on early 3.x code.

(73) NEWAD-realm : Creating challenge hash with username: ccwl@lboro.ac.uk (73) NEWAD-realm : Client is using MS-CHAPv2 for ccwl@lboro.ac.uk, we need NT-Password (73) NEWAD-realm : expand: '%{Stripped-User-Name}' -> 'ccwl' (73) NEWAD-realm : expand: '--username=%{%{Stripped-User-Name}:-%{User-Name:-None}}' -> '--username=ccwl' (73) NEWAD-realm : Creating challenge hash with username: ccwl@realm.ac.uk (73) NEWAD-realm : expand: '--challenge=%{NEWAD:Challenge:-00}' -> '--challenge=5003a64e642e6b6e' (73) NEWAD-realm : expand: '--nt-response=%{NEWAD:NT-Response:-00}' -> '--nt-response=3939f185ab8dc094774f6729d5b72be5ff3ddb053c2401d7' Exec: Program output is NT_KEY: 24D9FCB3EB61DE4D5BAC132146E92C7C (70) ERROR: NEWAD-realm : Exec: child returned 0 (70) NEWAD-realm : adding MS-CHAPv2 MPPE keys (70) [NEWAD-realm] = ok (70) - group returns ok (70) - if ("%{User-Name}" =~ /@/) returns reject rlm_eap_mschapv2: No MS-CHAPv2-Success or MS-CHAP-Error was found. (70) ERROR: eap : Failed continuing EAP MSCHAPv2 (26) session. EAP sub-module failed (70) eap : Failed in EAP select (70) [eap] = invalid (70) Failed to authenticate the user. (70) Login incorrect (NEWAD-realm: Exec: child returned 0): [ccwl@realm.ac.uk](from client test-client port 0 cli 70-6F-6C-69-73-68 via TLS tunnel)

the ntlm_auth works fine on command line - looks like something in the code isnt picking up the exec child return value.

alan

arr2036 commented 11 years ago

Yeah the debugging statement accidentally got changed to RDEBUGE.

alandekok commented 11 years ago

Alan Buxey wrote:

(70) ERROR: NEWAD-realm : Exec: child returned 0

That should be just a DEBUG message. "git pull"

(70) NEWAD-realm : adding MS-CHAPv2 MPPE keys (70) [NEWAD-realm] = ok (70) - group returns ok (70) - if ("%{User-Name}" =~ /@/) returns reject

OK... what else is that section doing? The NEWAD-realm module is returning OK, so it's not doing the reject.

the ntlm_auth works fine on command line - looks like something in the code isnt picking up the exec child return value.

My guess is that your config is looking for Module-Failure-Message, and returning "reject" if so. Changing the RDEBUGE to RDEBUG should fix that.

alanbuxey commented 11 years ago

error message now shows:

(8) NEWAD : Program output is NT_KEY: 24D9FCB3EB62DE5D5CBD232146F92C7C (8) NEWAD : Child executed successfully (8) NEWAD : adding MS-CHAPv2 MPPE keys (8) [NEWAD] = ok (8) - else else returns ok (8) ERROR: No MS-CHAP-Success or MS-CHAP-Error was found. (8) ERROR: eap : Failed continuing EAP MSCHAPv2 (26) session. EAP sub-module failed (8) eap : Failed in EAP select (8) [eap] = invalid (8) Failed to authenticate the user. (8) Login incorrect (: No MS-CHAP-Success or MS-CHAP-Error was found.): [ccwl](from client test port 0 cli 02-00-00-00-00-01 via TLS tunnel)

logic is:

    Auth-Type MS-CHAP {

            if("%{User-Name}" =~ /@/) {
            group {
                    NEWAD-realm {
                    ok = return
                    }
                  }
            }
            else {
                    NEWAD {
                    ok = return
                    }
                 }

    }

...and if the inner-id has a realm delimiter, output looks like:

(8) NEWAD-realm : Program output is NT_KEY: 24D9FCB3EB62DE5D5CBD232146F92C7C (8) NEWAD-realm : Child executed successfully (8) NEWAD-realm : adding MS-CHAPv2 MPPE keys (8) [NEWAD-realm] = ok (8) - group returns ok (8) - if ("%{User-Name}" =~ /@/) returns reject (8) ERROR: No MS-CHAP-Success or MS-CHAP-Error was found. (8) ERROR: eap : Failed continuing EAP MSCHAPv2 (26) session. EAP sub-module failed (8) eap : Failed in EAP select (8) [eap] = invalid (8) Failed to authenticate the user. (8) Login incorrect (: No MS-CHAP-Success or MS-CHAP-Error was found.): [ccwl@lboro.ac.uk](from client test port 0 cli 02-00-00-00-00-01 via TLS tunnel)

there is nothing apart from "ok = return" in play

alanbuxey commented 11 years ago

hmmm, took away the "ok = return" et voila. it works. isnt that WRONG?

with realm delimiter:

(8) NEWAD-realm : Program output is NT_KEY: 24D9FCB3EB62DE5D5CBD232146F92C7C (8) NEWAD-realm : Child executed successfully (8) NEWAD-realm : adding MS-CHAPv2 MPPE keys (8) [NEWAD-realm] = ok (8) - group returns ok (8) - if ("%{User-Name}" =~ /@/) returns ok (8) ... skipping else for request 8: Preceding "if" was taken MSCHAP Success (8) eap : New EAP session, adding 'State' attribute to reply 0x86a8566687a14c78 (8) [eap] = handled

without:

(8) NEWAD : Program output is NT_KEY: 24D9FCB3EB62DE5D5CBD232146F92C7C (8) NEWAD : Child executed successfully (8) NEWAD : adding MS-CHAPv2 MPPE keys (8) [NEWAD] = ok (8) - else else returns ok MSCHAP Success (8) eap : New EAP session, adding 'State' attribute to reply 0x2055453c215c5fd5 (8) [eap] = handled

alandekok commented 11 years ago

Your "Auth-Type MSCHAP" section is completely wrong. The Auth-Type processing is done for one Auth-Type section, and only one Auth-Type section. There's no need to add "ok=return" to the various pieces.

The working debug output shows "MSCHAP Success", which is generated by the EAP-MSCHAPv2 module. Looking at it, the problem becomes clear.

It calls the "authenticate" processing, and expects an OK return code as success. Anything else is interpreted as failure. The "ok=return" means that the Auth-Type section is not returning OK. Instead, it's stopping on OK, and returning the previous return code, which is REJECT.

So... don't do that. Allow the server to work the way it's intended.

alanbuxey commented 11 years ago

this code worked fine on 2.x (and still does). the reason why we have the group stuff is because we used to have 2 options there - OLDAD and NEWAD as we migrated. from what you say..and from current behaviour, we would never be able to do such side-migrations again. in the case of the 'with realm' being the first option, there wouldnt be a previous failure (REJECT) but it still rejected (the second part of my first comment)

alandekok commented 11 years ago

Alan Buxey wrote:

this code worked fine on 2.x (and still does).

It might break in 2.2.1. I'll go look.

Arran ran into an issue where the module return codes didn't obey priority. The fix for that may have changed this behavior.

I'll take a look...

the reason why we have the group stuff is because we used to have 2 options there - OLDAD and NEWAD as we migrated. from what you say..and from current behaviour, we would never be able to do such side-migrations again. in the case of the 'with realm' being the first option, there wouldnt be a previous failure (REJECT) but it still rejected (the second part of my first comment)

Well, if the config you posted was all that there was in Auth-Type MSCHAP, then it does nothing, and always did nothing.

arr2036 commented 11 years ago

I don't see OLDAD in the config? Is this the entirety of the config?

alanbuxey commented 11 years ago

we used to have OLDAD, the framework in which OLDAD was present is still there for future-proofing another dual-AD requirement.. but yes, thats the current entirety of the current config, OLDAD is no longer in there. on our 2.2.x boxes we still have:

#  MSCHAP authentication.
Auth-Type MS-CHAP {

            if("%{User-Name}" =~ /@/) {
                mschap-realm {
                  ok = return
                }
            }
            else {
              mschap {
                ok = return
             }
            }
}

..and they work.

alandekok commented 11 years ago

Alan Buxey wrote:

we used to have OLDAD, the framework in which OLDAD was present is still there for future-proofing another dual-AD requirement.. but yes, thats the current entirety of the current config, OLDAD is no longer in there. on our 2.2.x boxes we still have:

Well, it's a horrible configuration, but I've pushed a fix.