From cf00f0538b6b6e45171739a49424281a1802bb26 Mon Sep 17 00:00:00 2001 From: Robbie Harwood Date: Tue, 3 Oct 2017 14:28:47 -0400 Subject: Correctly handle fallback in KDC OTP callback In otp_state.c:callback(), avoid invoking the failure callback when we fall back to the next token. Since request_send() consumes the request, don't try to free it. [ghudson@mit.edu: added test case; edited commit message] (cherry picked from commit 09c9b7d6f64767429e90ad11a529e6ffa9538043) ticket: 8708 version_fixed: 1.15.4 --- src/plugins/preauth/otp/otp_state.c | 1 + src/tests/t_otp.py | 28 +++++++++++++++++++++++++--- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/src/plugins/preauth/otp/otp_state.c b/src/plugins/preauth/otp/otp_state.c index 7c76bd0..acdbca9 100644 --- a/src/plugins/preauth/otp/otp_state.c +++ b/src/plugins/preauth/otp/otp_state.c @@ -652,6 +652,7 @@ callback(krb5_error_code retval, const krad_packet *rqst, /* Try the next token. */ request_send(req); + return; error: req->cb(req->data, retval, otp_response_fail, NULL); diff --git a/src/tests/t_otp.py b/src/tests/t_otp.py index f098374..945853b 100755 --- a/src/tests/t_otp.py +++ b/src/tests/t_otp.py @@ -149,17 +149,23 @@ def verify(daemon, queue, reply, usernm, passwd): assert data['pass'] == [passwd] daemon.join() -def otpconfig(toktype, username=None, indicators=None): - val = '[{"type": "%s"' % toktype +# Compose a single token configuration. +def otpconfig_1(toktype, username=None, indicators=None): + val = '{"type": "%s"' % toktype if username is not None: val += ', "username": "%s"' % username if indicators is not None: qind = ['"%s"' % s for s in indicators] jsonlist = '[' + ', '.join(qind) + ']' val += ', "indicators":' + jsonlist - val += '}]' + val += '}' return val +# Compose a token configuration list suitable for the "otp" string +# attribute. +def otpconfig(toktype, username=None, indicators=None): + return '[' + otpconfig_1(toktype, username, indicators) + ']' + prefix = "/tmp/%d" % os.getpid() secret_file = prefix + ".secret" socket_file = prefix + ".socket" @@ -241,4 +247,20 @@ realm.run([kadminl, 'setstr', realm.user_princ, 'otp', otpconfig('unix')]) realm.kinit(realm.user_princ, 'accept', flags=flags) verify(daemon, queue, True, realm.user_princ, 'accept') +## Regression test for #8708: test with the standard username and two +## tokens configured, with the first rejecting and the second +## accepting. With the bug, the KDC incorrectly rejects the request +## and then performs invalid memory accesses, most likely crashing. +daemon1 = UDPRadiusDaemon(args=(server_addr, secret_file, 'accept1', queue)) +daemon2 = UnixRadiusDaemon(args=(socket_file, '', 'accept2', queue)) +daemon1.start() +queue.get() +daemon2.start() +queue.get() +oconf = '[' + otpconfig_1('udp') + ', ' + otpconfig_1('unix') + ']' +realm.run([kadminl, 'setstr', realm.user_princ, 'otp', oconf]) +realm.kinit(realm.user_princ, 'accept2', flags=flags) +verify(daemon1, queue, False, realm.user_princ.split('@')[0], 'accept2') +verify(daemon2, queue, True, realm.user_princ, 'accept2') + success('OTP tests') -- cgit v1.1