From bd6a449f6591f75d0db6dbf3fb702268b92d7eb8 Mon Sep 17 00:00:00 2001 From: Greg Hudson Date: Mon, 3 Aug 2015 11:44:58 -0400 Subject: [PATCH] Support OTP auth indicators in string attribute To better support integration with FreeIPA, allow authentication indicators to be specified in the "otp" string attribute, overriding any indicators in the token type. ticket: 8157 --- doc/admin/auth_indicator.rst | 3 +- doc/admin/otp.rst | 13 ++++- src/plugins/preauth/otp/otp_state.c | 73 +++++++++++++++++++++++++++-- src/tests/t_otp.py | 26 ++++++++-- 4 files changed, 103 insertions(+), 12 deletions(-) diff --git a/doc/admin/auth_indicator.rst b/doc/admin/auth_indicator.rst index e971aa90eb..b70a8dfc53 100644 --- a/doc/admin/auth_indicator.rst +++ b/doc/admin/auth_indicator.rst @@ -16,7 +16,8 @@ To use authentication indicators with PKINIT or OTP, first configure the KDC to include an indicator when that preauthentication mechanism is used. For PKINIT, use the **pkinit_indicator** variable in :ref:`kdc.conf(5)`. For OTP, use the **indicator** variable in the -token type definition. +token type definition, or specify the indicators in the **otp** user +string as described in :ref:`otp_preauth`. To require an indicator to be present in order to authenticate to a service principal, set the **require_auth** string attribute on the diff --git a/doc/admin/otp.rst b/doc/admin/otp.rst index 9baf7a7bcf..29dc520f32 100644 --- a/doc/admin/otp.rst +++ b/doc/admin/otp.rst @@ -30,6 +30,7 @@ Token types are defined in either :ref:`krb5.conf(5)` or timeout = (default: 5 [seconds]) retries = (default: 3) strip_realm = (default: true) + indicator = (default: none) } If the server field begins with '/', it will be interpreted as a UNIX @@ -43,6 +44,11 @@ used in the User-Name attribute of the RADIUS packet. The strip_realm parameter controls whether the principal is forwarded with or without the realm portion. +If an indicator field is present, tickets issued using this token type +will be annotated with the specified authentication indicator (see +:ref:`auth_indicator`). This key may be specified multiple times to +add multiple indicators. + The default token type ---------------------- @@ -71,7 +77,8 @@ format: [{ "type": , - "username": + "username": , + "indicators": [, ...] }, ...] This is an array of token objects. Both fields of token objects are @@ -79,7 +86,9 @@ optional. The **type** field names the token type of this token; if not specified, it defaults to ``DEFAULT``. The **username** field specifies the value to be sent in the User-Name RADIUS attribute. If not specified, the principal name is sent, with or without realm as -defined in the token type. +defined in the token type. The **indicators** field specifies a list +of authentication indicators to annotate tickets with, overriding any +indicators specified in the token type. For ease of configuration, an empty array (``[]``) is treated as equivalent to one DEFAULT token (``[{}]``). diff --git a/src/plugins/preauth/otp/otp_state.c b/src/plugins/preauth/otp/otp_state.c index 79fbc4db1a..5ba3d917af 100644 --- a/src/plugins/preauth/otp/otp_state.c +++ b/src/plugins/preauth/otp/otp_state.c @@ -58,6 +58,7 @@ typedef struct token_type_st { typedef struct token_st { const token_type *type; krb5_data username; + char **indicators; } token; typedef struct request_st { @@ -339,12 +340,61 @@ cleanup: return retval; } +/* Free a null-terminated array of strings. */ +static void +free_strings(char **list) +{ + char **p; + + for (p = list; p != NULL && *p != NULL; p++) + free(*p); + free(list); +} + /* Free the contents of a single token. */ static void token_free_contents(token *t) { - if (t != NULL) - free(t->username.data); + if (t == NULL) + return; + free(t->username.data); + free_strings(t->indicators); +} + +/* Decode a JSON array of strings into a null-terminated list of C strings. */ +static krb5_error_code +indicators_decode(krb5_context ctx, k5_json_value val, char ***indicators_out) +{ + k5_json_array arr; + k5_json_value obj; + char **indicators; + size_t len, i; + + *indicators_out = NULL; + + if (k5_json_get_tid(val) != K5_JSON_TID_ARRAY) + return EINVAL; + arr = val; + len = k5_json_array_length(arr); + indicators = calloc(len + 1, sizeof(*indicators)); + if (indicators == NULL) + return ENOMEM; + + for (i = 0; i < len; i++) { + obj = k5_json_array_get(arr, i); + if (k5_json_get_tid(obj) != K5_JSON_TID_STRING) { + free_strings(indicators); + return EINVAL; + } + indicators[i] = strdup(k5_json_string_utf8(obj)); + if (indicators[i] == NULL) { + free_strings(indicators); + return ENOMEM; + } + } + + *indicators_out = indicators; + return 0; } /* Decode a single token from a JSON token object. */ @@ -354,7 +404,7 @@ token_decode(krb5_context ctx, krb5_const_principal princ, { const char *typename = DEFAULT_TYPE_NAME; const token_type *type = NULL; - char *username = NULL; + char *username = NULL, **indicators = NULL; krb5_error_code retval; k5_json_value val; size_t i; @@ -386,8 +436,19 @@ token_decode(krb5_context ctx, krb5_const_principal princ, return retval; } + /* Get the authentication indicators if specified. */ + val = k5_json_object_get(obj, "indicators"); + if (val != NULL) { + retval = indicators_decode(ctx, val, &indicators); + if (retval != 0) { + free(username); + return retval; + } + } + out->type = type; out->username = string2data(username); + out->indicators = indicators; return 0; } @@ -563,7 +624,8 @@ callback(krb5_error_code retval, const krad_packet *rqst, const krad_packet *resp, void *data) { request *req = data; - char *const *indicators = req->tokens[req->index].type->indicators; + token *tok = &req->tokens[req->index]; + char *const *indicators; req->index++; @@ -573,6 +635,9 @@ callback(krb5_error_code retval, const krad_packet *rqst, /* If we received an accept packet, success! */ if (krad_packet_get_code(resp) == krad_code_name2num("Access-Accept")) { + indicators = tok->indicators; + if (indicators == NULL) + indicators = tok->type->indicators; req->cb(req->data, retval, otp_response_success, indicators); request_free(req); return; diff --git a/src/tests/t_otp.py b/src/tests/t_otp.py index bfc1eefe93..f098374f9e 100755 --- a/src/tests/t_otp.py +++ b/src/tests/t_otp.py @@ -149,12 +149,15 @@ def verify(daemon, queue, reply, usernm, passwd): assert data['pass'] == [passwd] daemon.join() -def otpconfig(toktype, username=None): +def otpconfig(toktype, username=None, indicators=None): val = '[{"type": "%s"' % toktype - if username is None: - val += '}]' - else: - val += ', "username": "%s"}]' % username + 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 += '}]' return val prefix = "/tmp/%d" % os.getpid() @@ -200,6 +203,19 @@ out = realm.run(['./adata', realm.krbtgt_princ]) if '+97: [indotp1, indotp2]' not in out: fail('auth indicators not seen in OTP ticket') +# Repeat with an indicators override in the string attribute. +daemon = UDPRadiusDaemon(args=(server_addr, secret_file, 'accept', queue)) +daemon.start() +queue.get() +oconf = otpconfig('udp', indicators=['indtok1', 'indtok2']) +realm.run([kadminl, 'setstr', realm.user_princ, 'otp', oconf]) +realm.kinit(realm.user_princ, 'accept', flags=flags) +verify(daemon, queue, True, realm.user_princ.split('@')[0], 'accept') +realm.extract_keytab(realm.krbtgt_princ, realm.keytab) +out = realm.run(['./adata', realm.krbtgt_princ]) +if '+97: [indtok1, indtok2]' not in out: + fail('auth indicators not seen in OTP ticket') + # Detect upstream pyrad bug # https://github.com/wichert/pyrad/pull/18 try: -- 2.47.2