From 0fece1af57e74efa5a7248f57495e2bddf72bb38 Mon Sep 17 00:00:00 2001 From: Sebastien Buisson Date: Tue, 17 Nov 2020 17:13:08 +0100 Subject: [PATCH] LU-14095 ssk: default rounds of Miller-Rabin for DH_check OpenSSL 1.1.1c increased the number of rounds used for Miller-Rabin testing of the prime provided as input parameter to DH_check(). This makes the check roughly x10 longer, and can lead to request timeouts when an SSK flavor is being used. Instead, use a dynamic number of rounds based on the speed of the check, evaluated when the lsvcgssd daemon starts. If DH_check() runtime is fine, just use it instead of our own check. Test-Parameters: clientdistro=el8.2 serverdistro=el8.2 testgroup=review-dne-ssk Signed-off-by: Sebastien Buisson Change-Id: Id392cdd76ede196094b146c68d230bc52852aa34 Reviewed-on: https://review.whamcloud.com/40686 Reviewed-by: Andreas Dilger Tested-by: jenkins Reviewed-by: John L. Hammond Tested-by: Maloo Reviewed-by: Oleg Drokin --- lustre/utils/gss/lgss_sk_utils.c | 2 +- lustre/utils/gss/sk_utils.c | 202 +++++++++++++++++++++++++++++++++-- lustre/utils/gss/sk_utils.h | 3 +- lustre/utils/gss/svcgssd.h | 1 + lustre/utils/gss/svcgssd_main_loop.c | 15 +++ lustre/utils/gss/svcgssd_proc.c | 3 +- 6 files changed, 215 insertions(+), 11 deletions(-) mode change 100644 => 100755 lustre/utils/gss/sk_utils.c diff --git a/lustre/utils/gss/lgss_sk_utils.c b/lustre/utils/gss/lgss_sk_utils.c index 3d12921..bbef525 100644 --- a/lustre/utils/gss/lgss_sk_utils.c +++ b/lustre/utils/gss/lgss_sk_utils.c @@ -92,7 +92,7 @@ static int lgss_sk_using_cred(struct lgss_cred *cred) uint32_t flags; int rc; - rc = sk_gen_params(skc); + rc = sk_gen_params(skc, 0); if (rc) return rc; diff --git a/lustre/utils/gss/sk_utils.c b/lustre/utils/gss/sk_utils.c old mode 100644 new mode 100755 index d3cf5ff..2e356a1 --- a/lustre/utils/gss/sk_utils.c +++ b/lustre/utils/gss/sk_utils.c @@ -40,6 +40,7 @@ #include #include #include +#include #include "sk_utils.h" #include "write_bytes.h" @@ -676,6 +677,197 @@ out_err: return NULL; } +#define SK_GENERATOR 2 +#define DH_NUMBER_ITERATIONS_FOR_PRIME 64 + +/* OpenSSL 1.1.1c increased the number of rounds used for Miller-Rabin testing + * of the prime provided as input parameter to DH_check(). This makes the check + * roughly x10 longer, and causes request timeouts when an SSK flavor is being + * used. + * + * Instead, use a dynamic number Miller-Rabin rounds based on the speed of the + * check on the current system, evaluated when the lsvcgssd daemon starts, but + * at least as many as OpenSSL 1.1.1b used for the same key size. If default + * DH_check() duration is OK, use it directly instead of limiting the rounds. + * + * If \a num_rounds == 0, we just call original DH_check() directly. + */ +static bool sk_is_dh_valid(const DH *dh, int num_rounds) +{ + const BIGNUM *p, *g; + BN_ULONG word; + BN_CTX *ctx; + BIGNUM *r; + bool valid = false; + int rc; + + if (num_rounds == 0) { + int codes = 0; + + rc = DH_check(dh, &codes); + if (rc != 1 || codes) { + printerr(0, "DH_check(0) failed: codes=%#x: rc=%d\n", + codes, rc); + return false; + } + return true; + } + + DH_get0_pqg(dh, &p, NULL, &g); + + if (!BN_is_word(g, SK_GENERATOR)) { + printerr(0, "%s: Diffie-Hellman generator is not %u\n", + program_invocation_short_name, SK_GENERATOR); + return false; + } + + word = BN_mod_word(p, 24); + if (word != 11) { + printerr(0, "%s: Diffie-Hellman prime modulo=%lu unsuitable\n", + program_invocation_short_name, word); + return false; + } + + ctx = BN_CTX_new(); + if (ctx == NULL) { + printerr(0, "%s: Diffie-Hellman error allocating context\n", + program_invocation_short_name); + return false; + } + BN_CTX_start(ctx); + r = BN_CTX_get(ctx); /* must be called before "ctx" used elsewhere */ + + rc = BN_is_prime_ex(p, num_rounds, ctx, NULL); + if (rc == 0) + printerr(0, "%s: Diffie-Hellman 'p' not prime in %u rounds\n", + program_invocation_short_name, num_rounds); + if (rc <= 0) + goto out_free; + + if (!BN_rshift1(r, p)) { + printerr(0, "%s: error shifting BigNum 'r' by 'p'\n", + program_invocation_short_name); + goto out_free; + } + rc = BN_is_prime_ex(r, num_rounds, ctx, NULL); + if (rc == 0) + printerr(0, "%s: Diffie-Hellman 'r' not prime in %u rounds\n", + program_invocation_short_name, num_rounds); + if (rc <= 0) + goto out_free; + + valid = true; + +out_free: + BN_CTX_end(ctx); + BN_CTX_free(ctx); + + return valid; +} + +#define VALUE_LENGTH 256 +static unsigned char test_prime[VALUE_LENGTH] = + "\xf7\xfa\x49\xd8\xec\xb1\x3b\xff\x26\x10\x3f\xc5\x3a\xc5\xcc\x40" + "\x4f\xbf\x92\xe1\x8b\x83\xe7\xa2\xba\x0f\x51\x5a\x91\x48\xe0\xa3" + "\xf1\x4d\xbc\xbb\x8a\x28\x14\xac\x02\x23\x76\x42\x17\x4d\x3c\xdc" + "\x5e\x4f\x80\x1f\xd7\x54\x1c\x50\xac\x3b\x28\x68\x8d\x71\x41\x7f" + "\xa7\x1c\x2f\x22\xd3\xa8\x91\xb2\x64\xb6\x84\xa6\xcf\x06\x16\x91" + "\x2f\xb8\xb4\x42\x1d\x3a\x4e\x3a\x0c\x7f\x04\x69\x78\xb5\x8f\x92" + "\x07\x89\xac\x24\x06\x53\x2c\x23\xec\xaa\x5c\xb4\x7b\x49\xbc\xf4" + "\x90\x67\x71\x9c\x24\x2c\x1d\x8d\x76\xc8\x85\x4e\x19\xf1\xf9\x33" + "\x45\xbd\x9f\x7d\x0a\x08\x8c\x22\xcc\x35\xf3\x5b\xab\x3f\x24\x9d" + "\x61\x70\x86\xbb\xbe\xd8\xb0\xf8\x34\xfa\xeb\x5b\x8e\xf2\x62\x23" + "\xd1\xfb\xbb\xb8\x21\x71\x1e\x39\x39\x59\xe0\x82\x98\x41\x84\x40" + "\x1f\xd3\x9b\xa3\x73\xdb\xec\xe0\xc0\xde\x2d\x1c\xea\x43\x40\x93" + "\x98\x38\x03\x36\x1e\xe1\xe7\x39\x7b\x35\x92\x4a\x51\xa5\x91\x63" + "\xd5\x31\x98\x3d\x89\x27\x6f\xcc\x69\xff\xbe\x31\x13\xdc\x2f\x72" + "\x2d\xab\x6a\xb7\x13\xd3\x47\xda\xaa\xf3\x3c\xa0\xfd\xaa\x0f\x02" + "\x96\x81\x1a\x26\xe8\xf7\x25\x65\x33\x78\xd9\x6b\x6d\xb0\xd9\xfb"; + +/** + * Measure time taken by prime testing routine for a 2048 bit long prime, + * depending on the number of check rounds. + * + * \param[in] usec_check_max max time allowed for DH_check completion + * + * \retval max number of rounds to keep prime testing under usec_check_max + * return 0 if we should use the default DH_check rounds + */ +int sk_speedtest_dh_valid(unsigned int usec_check_max) +{ + DH *dh; + BIGNUM *p, *g; + int num_rounds, prev_rounds = 0; + + dh = DH_new(); + if (!dh) + return 0; + + p = BN_bin2bn(test_prime, VALUE_LENGTH, NULL); + if (!p) + goto free_dh; + + g = BN_new(); + if (!g) + goto free_p; + + if (!BN_set_word(g, SK_GENERATOR)) + goto free_g; + + /* "dh" takes over freeing of 'p' and 'g' if this succeeds */ + if (!DH_set0_pqg(dh, p, NULL, g)) { + free_g: + BN_free(g); + free_p: + BN_free(p); + goto free_dh; + } + + for (num_rounds = 0; + num_rounds <= DH_NUMBER_ITERATIONS_FOR_PRIME; + num_rounds += (num_rounds <= 4 ? 4 : 8)) { + unsigned int usec_this; + int j; + + /* get max duration of 4 runs at current number of rounds */ + usec_this = 0; + for (j = 0; j < 4; j++) { + struct timeval now, prev; + unsigned int usec_curr; + + gettimeofday(&prev, NULL); + if (!sk_is_dh_valid(dh, num_rounds)) { + /* if test_prime is found bad, use default */ + prev_rounds = 0; + goto free_dh; + } + gettimeofday(&now, NULL); + usec_curr = (now.tv_sec - prev.tv_sec) * 1000000 + + now.tv_usec - prev.tv_usec; + if (usec_curr > usec_this) + usec_this = usec_curr; + } + printerr(2, "%s: %d rounds: %d usec\n", + program_invocation_short_name, num_rounds, usec_this); + if (num_rounds == 0) { + if (usec_this <= usec_check_max) + /* using original check rounds as implemented in + * DH_check() took less time than the max allowed, + * so just use original DH_check() + */ + break; + } else if (usec_this >= usec_check_max) { + break; + } + prev_rounds = num_rounds; + } + +free_dh: + DH_free(dh); + + return prev_rounds; +} + /** * Populates the DH parameters for the DHKE * @@ -685,12 +877,11 @@ out_err: * \retval GSS_S_COMPLETE success * \retval GSS_S_FAILURE failure */ -uint32_t sk_gen_params(struct sk_cred *skc) +uint32_t sk_gen_params(struct sk_cred *skc, int num_rounds) { uint32_t random; BIGNUM *p, *g; const BIGNUM *pub_key; - int rc; /* Random value used by both the request and response as part of the * key binding material. This also should ensure we have unqiue @@ -738,13 +929,8 @@ uint32_t sk_gen_params(struct sk_cred *skc) } /* Verify that we have a safe prime and valid generator */ - if (DH_check(skc->sc_params, &rc) != 1) { - printerr(0, "DH_check() failed: %d\n", rc); - return GSS_S_FAILURE; - } else if (rc) { - printerr(0, "DH_check() returned error codes: 0x%x\n", rc); + if (!sk_is_dh_valid(skc->sc_params, num_rounds)) return GSS_S_FAILURE; - } if (DH_generate_key(skc->sc_params) != 1) { printerr(0, "Failed to generate public DH key: %s\n", diff --git a/lustre/utils/gss/sk_utils.h b/lustre/utils/gss/sk_utils.h index e20d12d..d942e81 100644 --- a/lustre/utils/gss/sk_utils.h +++ b/lustre/utils/gss/sk_utils.h @@ -327,7 +327,8 @@ uint32_t sk_verify_hash(const char *string, const EVP_MD *hash_alg, const gss_buffer_desc *current_hash); struct sk_cred *sk_create_cred(const char *fsname, const char *cluster, const uint32_t flags); -uint32_t sk_gen_params(struct sk_cred *skc); +int sk_speedtest_dh_valid(unsigned int usec_check_max); +uint32_t sk_gen_params(struct sk_cred *skc, int num_rounds); int sk_sign_bufs(gss_buffer_desc *key, gss_buffer_desc *bufs, const int numbufs, const EVP_MD *hash_alg, gss_buffer_desc *hmac); uint32_t sk_verify_hmac(struct sk_cred *skc, gss_buffer_desc *bufs, diff --git a/lustre/utils/gss/svcgssd.h b/lustre/utils/gss/svcgssd.h index 63e734f..6133589 100644 --- a/lustre/utils/gss/svcgssd.h +++ b/lustre/utils/gss/svcgssd.h @@ -48,6 +48,7 @@ extern char *oss_local_realm; extern int null_enabled; extern int krb_enabled; extern int sk_enabled; +extern int sk_dh_checks; #define GSSD_SERVICE_NAME "lustre" diff --git a/lustre/utils/gss/svcgssd_main_loop.c b/lustre/utils/gss/svcgssd_main_loop.c index bfd0624..30b91c8 100644 --- a/lustre/utils/gss/svcgssd_main_loop.c +++ b/lustre/utils/gss/svcgssd_main_loop.c @@ -47,8 +47,12 @@ #include "svcgssd.h" #include "err_util.h" +#include "sk_utils.h" #define GSS_RPC_FILE "/proc/net/rpc/auth.sptlrpc.init/channel" +/* max allowed time for prime testing: 400 ms */ +#define MAX_ALLOWED_TIME_FOR_PRIME 400000 +int sk_dh_checks; /* * nfs4 in-kernel cache implementation make upcall failed directly @@ -69,6 +73,17 @@ svcgssd_run() struct pollfd pollfd; struct timespec halfsec = { .tv_sec = 0, .tv_nsec = 500000000 }; + if (sk_enabled) { +#if OPENSSL_VERSION_NUMBER >= 0x1010103fL + sk_dh_checks = + sk_speedtest_dh_valid(MAX_ALLOWED_TIME_FOR_PRIME); +#else + sk_dh_checks = 0; +#endif + printerr(1, "will use %d rounds for prime testing\n", + sk_dh_checks); + } + while (1) { int save_err; diff --git a/lustre/utils/gss/svcgssd_proc.c b/lustre/utils/gss/svcgssd_proc.c index d3e1b04..da8dcf8 100644 --- a/lustre/utils/gss/svcgssd_proc.c +++ b/lustre/utils/gss/svcgssd_proc.c @@ -57,6 +57,7 @@ #include "lsupport.h" #include "gss_oids.h" #include "sk_utils.h" +#include #define SVCGSSD_CONTEXT_CHANNEL "/proc/net/rpc/auth.sptlrpc.context/channel" #define SVCGSSD_INIT_CHANNEL "/proc/net/rpc/auth.sptlrpc.init/channel" @@ -460,7 +461,7 @@ int handle_sk(struct svc_nego_data *snd) } redo: - rc = sk_gen_params(skc); + rc = sk_gen_params(skc, sk_dh_checks); if (rc != GSS_S_COMPLETE) { printerr(0, "Failed to generate DH params for responder\n"); goto cleanup_partial; -- 1.8.3.1