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

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')