MarcJHuber / event-driven-servers

A collection of event-driven servers (currently: tac_plus, tac_plus-ng, ftpd, tcprelay)
https://www.pro-bono-publico.de/projects
Other
100 stars 25 forks source link

tacplus-ng: Missing TACACS reply with status="Authentication Failed" if LDAP response resultCode="InvalidCredentials" #58

Closed beranf closed 1 year ago

beranf commented 1 year ago

Hi Marc,

if external authentication module mavis_tacplus-ng_ldap.pl gets response from LDAP server bindResponse with resultCode="InvalidCredentials" (only wrong password is used), then tacplus-ng does not pass "Authentication Failed" into TACACS reply message. Instead tacplus-ng is waiting until the NAS will send new TACACS authentication session. This is probably a bug. It causes very long delay for some nas devices (Fortigate, F5, ...) if an user does a mistake at writing password before he can write it again. I think tacplus-ng should immediately reply TACACS packet with "Authentication Failed" if gets a response bindResponse with resultCode="InvalidCredentials" from mavis_tacplus-ng_ldap.pl module. What fo you think?

In my configuration I uses multiple external authentication: primary mavis_tacplus-ng_ldap.pl secondary pammavis

I have also tested config with single external authentication (mavis_tacplus-ng_ldap.pl) and the issue persists.

Thank you in advanced. Regards, Filip

MarcJHuber commented 1 year ago

Hi Filip,

my test script doesn't show that behavior ... please configure with --debug, rebuild and retest, that may or may not help finding the root cause.

The configuration I usually use to test module chaining should be quite similar to yours:

id = spawnd { listen { port = 4949 } }
id = tac_plus-ng {
        mavis module ldap = external {
                setenv LDAP_BASE = "dc=example,dc=com"
                setenv LDAP_USER = "cn=admin,dc=example,dc=com"
                setenv LDAP_PASSWD = "mypassword"
                setenv LDAP_HOSTS = "ldaps://localhost:636"
                exec = /usr/local/lib/mavis/mavis_tacplus-ng_ldap.pl
        }
        mavis module pam-external = external {
                exec = /usr/local/sbin/pammavis pammavis -s ssh
        }
        user backend = mavis
        login backend = mavis chpass
        pap backend = mavis
        host any { address = ::0/0 key = demo }
        profile admin {
                script {
                        if (service == shell) {
                            if (cmd == "") set priv-lvl = 15
                            permit
                        }
                }
        }
        ruleset {
                rule {
                        script {
                                profile = admin
                                permit
                        }
                }
        }
}

In my tests, this works just fine for both LDAP and PAM.

Cheers,

Marc

beranf commented 1 year ago

Hi Marc, 1) have you tried/tested to authenticate on LDAP using a wrong password? 2) in attachment is my log when a user sends a wrong password and authenticated on MS-AD (if I trace into pcap, there I can see, that fortigate sends TCP/FIN to the tacacs server after 60s since authenticated request was send).

beranf_-_bad_passwd_MS-AD_missing_tacacs_reply_authentication_failed.txt

Regards, Filip

MarcJHuber commented 1 year ago

Hi Filip,

first, thanks a lot for providing the debug log, I very much appreciate that.

Yes, I did a series of tests (ldap with bad password, ldap with good password, pam with good password), and all were just fine.

Your debug log indeed gives a clue. "COMMENT" is set to "invalid request", and upon closer investigation the "TACTYPE" AV pair isn't part of the module reply, so the module answer is considered as malformed.

When testing this locally, I can easily see

pid:   av_set(TACTYPE) = AUTH
pid:   av_get(TACTYPE) = AUTH

within "external:read_from_child", just before "mavis_recv". That's missing from your debug output. The previous "mavis_send" section even shows that AV pair, and I've currently no idea why it's dropped in the next step.

Is your /root/event-driven-servers/mavis/perl/mavis_tacplus-ng_ldap.pl script current? "make install" will only update unmodified versions.

I'm afraid this issue will take some time to be resolved.

Cheers,

Marc

beranf commented 1 year ago

Hi Marc, yes I have installed the current version of mavis_tacplus-ng_ldap.pl

Here is full debug log from tacplus-ng, if an existing user at MS-AD uses a bad password. In such case my MS-AD replies over LDAP protocol to tacacs server bindResponse with resultCode="InvalidCredentials" and tacacs server does not send any tacacs REPLY message to the NAS (I would expect tacacs REPLY Authentication Failed should be send to NAS).

beranf_-_bad_password_user_test_sd1_MS-AD_missing_tacacas_reply_authentication_failedFULL_DEBUG_LOG.txt

Regards, Filip

MarcJHuber commented 1 year ago

Hi Filip,

ok, I still can't reproduce this.

Next step: Let's see whether the Perl script doesn't return the TACAUTH attribute or whether the daemon skips it for whatever reason.

Please include the "tee" module in your config. It will write the AV pairs seen to the specified paths.

....
        mavis module = tee {
                path in = /tmp/in.txt
                path out = /tmp/out.txt
        }
        mavis module ldap = external {
....

Both files should come with a "49 AUTH" line.

Thanks,

Marc

MarcJHuber commented 1 year ago

Hi Filip,

I think I found the reason. The issue happens only with multi-line AD error messages. I hope to push a fix later this day.

Thanks,

Marc

MarcJHuber commented 1 year ago

Hi Filip,

the latest push might work better, please try.

Cheers,

Marc

beranf commented 1 year ago

Hi Marc, now it is much better 👍 It is working well as I would expect, thank you for identifying route cause and fixing!
I don't know in what debug output you see a multi-line errot event from LDAP/MS-AD server. In this issue case, my MS-AD returns over LDAP protocol the error message (snapshot from wireshark capture): image

So this issue can be closed. Thank you again,

Regards, Filip

MarcJHuber commented 1 year ago

Hi Filip,

thanks for confirming that the issue is resolved!

I usually test that LDAP stuff via OpenLDAP, and, now obviously, that's not the same as AD.

Actually, upon closer inspection, that wasn't even a multi-line issue. I wrongly assumed that the AD LDAP response would include a newline, but the attempt to convert NL to CR didn't improve things.

It's even worse: The AD LDAP server returns an error message containing a NULL byte. Perl can handle that (much to my surprise), but C will obviously just stop processing the character sequence. I'm not sure about the Python backend yet.

The positive side effect of the latest changes is that the current code could greatly improve the password-change experience for users.

Thanks for holding on :-)

Cheers,

Marc