Re: [PATCH v1 04/41] SUNRPC: Improve Kerberos confounder generation
From: Chuck Lever III <chuck.lever@oracle.com>
Date: 2023-01-13 17:56:10
On Jan 13, 2023, at 12:45 PM, Simo Sorce [off-list ref] wrote: SIPHash is not a cryptographically secure PRNG, and is not suitable for the confounder, strong NACK on this.
The seed and starting counter are both derived from random sources. This hash doesn't need to be cryptographically secure. But OK; please suggest an alternative.
Simo. On Fri, 2023-01-13 at 10:21 -0500, Chuck Lever wrote:quoted
From: Chuck Lever <chuck.lever@oracle.com> Other common Kerberos implementations use a fully random confounder for encryption. For a Kerberos implementation that is part of an O/S I/O stack, this is impractical. However, using a fast PRG that does not deplete the system entropy pool is possible and desirable. Use an atomic type to ensure that confounder generation deterministically generates a unique and pseudo-random result in the face of concurrent execution, and make the confounder generation materials unique to each Keberos context. The latter has several benefits: - the internal counter will wrap less often - no way to guess confounders based on other Kerberos-encrypted traffic - better scalability Since confounder generation is part of Kerberos itself rather than the GSS-API Kerberos mechanism, the function is renamed and moved. Tested-by: Scott Mayhew <redacted> Signed-off-by: Chuck Lever <chuck.lever@oracle.com> --- include/linux/sunrpc/gss_krb5.h | 7 +++--- net/sunrpc/auth_gss/gss_krb5_crypto.c | 28 ++++++++++++++++++++++- net/sunrpc/auth_gss/gss_krb5_internal.h | 13 +++++++++++ net/sunrpc/auth_gss/gss_krb5_mech.c | 17 +++++++------- net/sunrpc/auth_gss/gss_krb5_wrap.c | 38 ++----------------------------- 5 files changed, 56 insertions(+), 47 deletions(-) create mode 100644 net/sunrpc/auth_gss/gss_krb5_internal.hdiff --git a/include/linux/sunrpc/gss_krb5.h b/include/linux/sunrpc/gss_krb5.h index 51860e3a0216..192f5b37763f 100644 --- a/include/linux/sunrpc/gss_krb5.h +++ b/include/linux/sunrpc/gss_krb5.h@@ -38,6 +38,8 @@#define _LINUX_SUNRPC_GSS_KRB5_H #include <crypto/skcipher.h> +#include <linux/siphash.h> + #include <linux/sunrpc/auth_gss.h> #include <linux/sunrpc/gss_err.h> #include <linux/sunrpc/gss_asn1.h>@@ -106,6 +108,8 @@ struct krb5_ctx {atomic_t seq_send; atomic64_t seq_send64; time64_t endtime; + atomic64_t confounder; + siphash_key_t confkey; struct xdr_netobj mech_used; u8 initiator_sign[GSS_KRB5_MAX_KEYLEN]; u8 acceptor_sign[GSS_KRB5_MAX_KEYLEN];@@ -311,7 +315,4 @@ gss_krb5_aes_decrypt(struct krb5_ctx *kctx, u32 offset, u32 len,struct xdr_buf *buf, u32 *plainoffset, u32 *plainlen); -void -gss_krb5_make_confounder(char *p, u32 conflen); - #endif /* _LINUX_SUNRPC_GSS_KRB5_H */diff --git a/net/sunrpc/auth_gss/gss_krb5_crypto.c b/net/sunrpc/auth_gss/gss_krb5_crypto.c index 8aa5610ef660..6d962079aa95 100644 --- a/net/sunrpc/auth_gss/gss_krb5_crypto.c +++ b/net/sunrpc/auth_gss/gss_krb5_crypto.c@@ -47,10 +47,36 @@#include <linux/sunrpc/gss_krb5.h> #include <linux/sunrpc/xdr.h> +#include "gss_krb5_internal.h" + #if IS_ENABLED(CONFIG_SUNRPC_DEBUG) # define RPCDBG_FACILITY RPCDBG_AUTH #endif +/** + * krb5_make_confounder - Generate a unique pseudorandom string + * @kctx: Kerberos context + * @p: memory location into which to write the string + * @conflen: string length to write, in octets + * + * To avoid draining the system's entropy pool when under heavy + * encrypted I/O loads, the @kctx has a small amount of random seed + * data that is then hashed to generate each pseudorandom confounder + * string. + */ +void +krb5_make_confounder(struct krb5_ctx *kctx, u8 *p, int conflen) +{ + u64 *q = (u64 *)p; + + WARN_ON_ONCE(conflen < sizeof(*q)); + while (conflen > 0) { + *q++ = siphash_1u64(atomic64_inc_return(&kctx->confounder), + &kctx->confkey); + conflen -= sizeof(*q); + } +} + u32 krb5_encrypt( struct crypto_sync_skcipher *tfm,@@ -630,7 +656,7 @@ gss_krb5_aes_encrypt(struct krb5_ctx *kctx, u32 offset,offset += GSS_KRB5_TOK_HDR_LEN; if (xdr_extend_head(buf, offset, conflen)) return GSS_S_FAILURE; - gss_krb5_make_confounder(buf->head[0].iov_base + offset, conflen); + krb5_make_confounder(kctx, buf->head[0].iov_base + offset, conflen); offset -= GSS_KRB5_TOK_HDR_LEN; if (buf->tail[0].iov_base != NULL) {diff --git a/net/sunrpc/auth_gss/gss_krb5_internal.h b/net/sunrpc/auth_gss/gss_krb5_internal.h new file mode 100644 index 000000000000..6249124aba1d --- /dev/null +++ b/net/sunrpc/auth_gss/gss_krb5_internal.h@@ -0,0 +1,13 @@ +/* SPDX-License-Identifier: GPL-2.0 or BSD-3-Clause */ +/* + * SunRPC GSS Kerberos 5 mechanism internal definitions + * + * Copyright (c) 2022 Oracle and/or its affiliates. + */ + +#ifndef _NET_SUNRPC_AUTH_GSS_KRB5_INTERNAL_H +#define _NET_SUNRPC_AUTH_GSS_KRB5_INTERNAL_H + +void krb5_make_confounder(struct krb5_ctx *kctx, u8 *p, int conflen); + +#endif /* _NET_SUNRPC_AUTH_GSS_KRB5_INTERNAL_H */diff --git a/net/sunrpc/auth_gss/gss_krb5_mech.c b/net/sunrpc/auth_gss/gss_krb5_mech.c index 08a86ece665e..6d59794c9b69 100644 --- a/net/sunrpc/auth_gss/gss_krb5_mech.c +++ b/net/sunrpc/auth_gss/gss_krb5_mech.c@@ -550,16 +550,17 @@ gss_import_sec_context_kerberos(const void *p, size_t len,ret = gss_import_v1_context(p, end, ctx); else ret = gss_import_v2_context(p, end, ctx, gfp_mask); - - if (ret == 0) { - ctx_id->internal_ctx_id = ctx; - if (endtime) - *endtime = ctx->endtime; - } else + if (ret) { kfree(ctx); + return ret; + } - dprintk("RPC: %s: returning %d\n", __func__, ret); - return ret; + ctx_id->internal_ctx_id = ctx; + if (endtime) + *endtime = ctx->endtime; + atomic64_set(&ctx->confounder, get_random_u64()); + get_random_bytes(&ctx->confkey, sizeof(ctx->confkey)); + return 0; } static voiddiff --git a/net/sunrpc/auth_gss/gss_krb5_wrap.c b/net/sunrpc/auth_gss/gss_krb5_wrap.c index bd068e936947..374214f3c463 100644 --- a/net/sunrpc/auth_gss/gss_krb5_wrap.c +++ b/net/sunrpc/auth_gss/gss_krb5_wrap.c@@ -32,9 +32,10 @@#include <linux/types.h> #include <linux/jiffies.h> #include <linux/sunrpc/gss_krb5.h> -#include <linux/random.h> #include <linux/pagemap.h> +#include "gss_krb5_internal.h" + #if IS_ENABLED(CONFIG_SUNRPC_DEBUG) # define RPCDBG_FACILITY RPCDBG_AUTH #endif@@ -113,39 +114,6 @@ gss_krb5_remove_padding(struct xdr_buf *buf, int blocksize)return 0; } -void -gss_krb5_make_confounder(char *p, u32 conflen) -{ - static u64 i = 0; - u64 *q = (u64 *)p; - - /* rfc1964 claims this should be "random". But all that's really - * necessary is that it be unique. And not even that is necessary in - * our case since our "gssapi" implementation exists only to support - * rpcsec_gss, so we know that the only buffers we will ever encrypt - * already begin with a unique sequence number. Just to hedge my bets - * I'll make a half-hearted attempt at something unique, but ensuring - * uniqueness would mean worrying about atomicity and rollover, and I - * don't care enough. */ - - /* initialize to random value */ - if (i == 0) { - i = get_random_u32(); - i = (i << 32) | get_random_u32(); - } - - switch (conflen) { - case 16: - *q++ = i++; - fallthrough; - case 8: - *q++ = i++; - break; - default: - BUG(); - } -} - /* Assumptions: the head and tail of inbuf are ours to play with. * The pages, however, may be real pages in the page cache and we replace * them with scratch pages from **pages before writing to them. */@@ -211,7 +179,7 @@ gss_wrap_kerberos_v1(struct krb5_ctx *kctx, int offset,ptr[6] = 0xff; ptr[7] = 0xff; - gss_krb5_make_confounder(msg_start, conflen); + krb5_make_confounder(kctx, msg_start, conflen); if (kctx->gk5e->keyed_cksum) cksumkey = kctx->cksum;-- Simo Sorce RHEL Crypto Team Red Hat, Inc
-- Chuck Lever