> On Jan 13, 2023, at 12:45 PM, Simo Sorce <simo@xxxxxxxxxx> 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:
>> From: Chuck Lever <chuck.lever@xxxxxxxxxx>
>>
>> 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 <smayhew@xxxxxxxxxx>
>> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
>> ---
>> 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.h
>>
>> diff --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 void
>> diff --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
