krb5 commit: Correctly handle fallback in KDC OTP callback

Greg Hudson <[email protected]>
Newsgroups gmane.comp.encryption.kerberos.cvs
Message-ID <[email protected]>
https://github.com/krb5/krb5/commit/09c9b7d6f64767429e90ad11a529e6ffa9538043
commit 09c9b7d6f64767429e90ad11a529e6ffa9538043
Author: Robbie Harwood <[email protected]>
Date:   Tue Oct 3 14:28:47 2017 -0400

    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.
    
    [[email protected]: added test case; edited commit message]
    
    ticket: 8708
    tags: pullup
    target_version: 1.16-next
    target_version: 1.15-next

 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 5ba3d91..93ba6c0 100644
--- a/src/plugins/preauth/otp/otp_state.c
+++ b/src/plugins/preauth/otp/otp_state.c
@@ -649,6 +649,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 1142fc7..2bbc9a1 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"
@@ -244,4 +250,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')
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.