krb5 commit: PKINIT ECDH support
ghudson at mit.edu
ghudson at mit.edu
Mon Jul 17 01:46:32 EDT 2023
https://github.com/krb5/krb5/commit/0f870b1bcad960fd5319a3f97aafd7f4a289e2fb
commit 0f870b1bcad960fd5319a3f97aafd7f4a289e2fb
Author: Greg Hudson <ghudson at mit.edu>
Date: Fri May 12 15:38:46 2023 -0400
PKINIT ECDH support
Add support for elliptic curve key exchange to PKINIT (RFC 5349
section 4). Extend pkinit_dh_min_bits to allow the string values
"P-256", "P-384", and "P-521", using rough finite-field strength
equivalents to rank them relative to the Oakley Diffie-Hellman groups.
When processing TD-DH-PARAMETERS on the client, only accept the three
Oakley groups or the three supported elliptic curve groups.
Previously we accepted any Diffie-Hellman parameters that passed
EVP_PKEY_param_check()/DH_check() and had equal or better bit strength
to the original proposal.
ticket: 9095 (new)
doc/admin/conf_files/kdc_conf.rst | 7 +-
doc/admin/conf_files/krb5_conf.rst | 7 +-
src/plugins/preauth/pkinit/pkinit.h | 6 +-
src/plugins/preauth/pkinit/pkinit_clnt.c | 17 +-
src/plugins/preauth/pkinit/pkinit_constants.c | 27 ++
src/plugins/preauth/pkinit/pkinit_crypto.h | 7 +
src/plugins/preauth/pkinit/pkinit_crypto_openssl.c | 470 ++++++++++++++-------
src/plugins/preauth/pkinit/pkinit_crypto_openssl.h | 4 +-
src/plugins/preauth/pkinit/pkinit_lib.c | 3 -
src/plugins/preauth/pkinit/pkinit_srv.c | 17 +-
src/plugins/preauth/pkinit/pkinit_trace.h | 11 +
src/tests/t_pkinit.py | 12 +
12 files changed, 405 insertions(+), 183 deletions(-)
diff --git a/doc/admin/conf_files/kdc_conf.rst b/doc/admin/conf_files/kdc_conf.rst
index 74a0a2ace..d1de933a5 100644
--- a/doc/admin/conf_files/kdc_conf.rst
+++ b/doc/admin/conf_files/kdc_conf.rst
@@ -768,8 +768,11 @@ For information about the syntax of some of these options, see
be specified multiple times.
**pkinit_dh_min_bits**
- Specifies the minimum number of bits the KDC is willing to accept
- for a client's Diffie-Hellman key. The default is 2048.
+ Specifies the minimum strength of Diffie-Hellman group the KDC is
+ willing to accept for key exchange. Valid values in order of
+ increasing strength are 1024, 2048, P-256, 4096, P-384, and P-521.
+ The default is 2048. (P-256, P-384, and P-521 are new in release
+ 1.22.)
**pkinit_allow_upn**
Specifies that the KDC is willing to accept client certificates
diff --git a/doc/admin/conf_files/krb5_conf.rst b/doc/admin/conf_files/krb5_conf.rst
index ecdf91750..651e0e78d 100644
--- a/doc/admin/conf_files/krb5_conf.rst
+++ b/doc/admin/conf_files/krb5_conf.rst
@@ -1128,9 +1128,10 @@ PKINIT krb5.conf options
option is not recommended.
**pkinit_dh_min_bits**
- Specifies the size of the Diffie-Hellman key the client will
- attempt to use. The acceptable values are 1024, 2048, and 4096.
- The default is 2048.
+ Specifies the group of the Diffie-Hellman key the client will
+ attempt to use. The acceptable values are 1024, 2048, P-256,
+ 4096, P-384, and P-521. The default is 2048. (P-256, P-384, and
+ P-521 are new in release 1.22.)
**pkinit_identities**
Specifies the location(s) to be used to find the user's X.509
diff --git a/src/plugins/preauth/pkinit/pkinit.h b/src/plugins/preauth/pkinit/pkinit.h
index 66f92d8f0..d06edfb87 100644
--- a/src/plugins/preauth/pkinit/pkinit.h
+++ b/src/plugins/preauth/pkinit/pkinit.h
@@ -59,6 +59,10 @@
#define PKINIT_DEFAULT_DH_MIN_BITS 2048
#define PKINIT_DH_MIN_CONFIG_BITS 1024
+/* Rough finite-field bit strength equivalents for the elliptic curve groups */
+#define PKINIT_DH_P256_BITS 3072
+#define PKINIT_DH_P384_BITS 7680
+#define PKINIT_DH_P521_BITS 15360
#define KRB5_CONF_KDCDEFAULTS "kdcdefaults"
#define KRB5_CONF_LIBDEFAULTS "libdefaults"
@@ -101,8 +105,6 @@ static inline void pkiDebug (const char *fmt, ...) { }
#define OCTETDATA_TO_KRB5DATA(octd, k5d) \
(k5d)->length = (octd)->length; (k5d)->data = (char *)(octd)->data;
-extern const krb5_data dh_oid;
-
/*
* notes about crypto contexts:
*
diff --git a/src/plugins/preauth/pkinit/pkinit_clnt.c b/src/plugins/preauth/pkinit/pkinit_clnt.c
index ea9ba454d..cfccd7b79 100644
--- a/src/plugins/preauth/pkinit/pkinit_clnt.c
+++ b/src/plugins/preauth/pkinit/pkinit_clnt.c
@@ -774,7 +774,7 @@ pkinit_client_profile(krb5_context context,
const krb5_data *realm)
{
const char *configured_identity;
- char *eku_string = NULL;
+ char *eku_string = NULL, *minbits = NULL;
pkiDebug("pkinit_client_profile %p %p %p %p\n",
context, plgctx, reqctx, realm);
@@ -783,17 +783,10 @@ pkinit_client_profile(krb5_context context,
KRB5_CONF_PKINIT_REQUIRE_CRL_CHECKING,
reqctx->opts->require_crl_checking,
&reqctx->opts->require_crl_checking);
- pkinit_libdefault_integer(context, realm,
- KRB5_CONF_PKINIT_DH_MIN_BITS,
- reqctx->opts->dh_size,
- &reqctx->opts->dh_size);
- if (reqctx->opts->dh_size != 1024 && reqctx->opts->dh_size != 2048
- && reqctx->opts->dh_size != 4096) {
- pkiDebug("%s: invalid value (%d) for pkinit_dh_min_bits, "
- "using default value (%d) instead\n", __FUNCTION__,
- reqctx->opts->dh_size, PKINIT_DEFAULT_DH_MIN_BITS);
- reqctx->opts->dh_size = PKINIT_DEFAULT_DH_MIN_BITS;
- }
+ pkinit_libdefault_string(context, realm, KRB5_CONF_PKINIT_DH_MIN_BITS,
+ &minbits);
+ reqctx->opts->dh_size = parse_dh_min_bits(context, minbits);
+ free(minbits);
pkinit_libdefault_string(context, realm,
KRB5_CONF_PKINIT_EKU_CHECKING,
&eku_string);
diff --git a/src/plugins/preauth/pkinit/pkinit_constants.c b/src/plugins/preauth/pkinit/pkinit_constants.c
index 1da482e0b..10f8688ec 100644
--- a/src/plugins/preauth/pkinit/pkinit_constants.c
+++ b/src/plugins/preauth/pkinit/pkinit_constants.c
@@ -320,6 +320,33 @@ static const uint8_t o4096[] = {
0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF
};
+/* Named curve prime256v1 (1.2.840.10045.3.1.7) as parameters for RFC 3279
+ * section 2.3.5 id-ecPublicKey */
+static const uint8_t p256[] = {
+ 0x06, 0x08, 0x2A, 0x86, 0x48, 0xCE, 0x3D, 0x03, 0x01, 0x07
+};
+
+/* Named curve secp384r1 (1.3.132.0.34, from RFC 5480 section 2.1.1.1) as
+ * parameters for RFC 3279 section 2.3.5 id-ecPublicKey */
+static const uint8_t p384[] = {
+ 0x06, 0x05, 0x2B, 0x81, 0x04, 0x00, 0x22
+};
+
+/* Named curve secp521r1 (1.3.132.0.35, from RFC 5480 section 2.1.1.1) as
+ * parameters for RFC 3279 section 2.3.5 id-ecPublicKey */
+static const uint8_t p521[] = {
+ 0x06, 0x05, 0x2B, 0x81, 0x04, 0x00, 0x23
+};
+
const krb5_data oakley_1024 = { KV5M_DATA, sizeof(o1024), (char *)o1024 };
const krb5_data oakley_2048 = { KV5M_DATA, sizeof(o2048), (char *)o2048 };
const krb5_data oakley_4096 = { KV5M_DATA, sizeof(o4096), (char *)o4096 };
+const krb5_data ec_p256 = { KV5M_DATA, sizeof(p256), (char *)p256 };
+const krb5_data ec_p384 = { KV5M_DATA, sizeof(p384), (char *)p384 };
+const krb5_data ec_p521 = { KV5M_DATA, sizeof(p521), (char *)p521 };
+
+/* RFC 3279 section 2.3.3 dhpublicnumber (1.2.840.10046.2.1) */
+const krb5_data dh_oid = { 0, 7, "\x2A\x86\x48\xce\x3e\x02\x01" };
+
+/* RFC 3279 section 2.3.5 id-ecPublicKey (1.2.840.10045.2.1) */
+const krb5_data ec_oid = { 0, 7, "\x2A\x86\x48\xCE\x3D\x02\x01" };
diff --git a/src/plugins/preauth/pkinit/pkinit_crypto.h b/src/plugins/preauth/pkinit/pkinit_crypto.h
index 4e9a4d40f..53f1d0f13 100644
--- a/src/plugins/preauth/pkinit/pkinit_crypto.h
+++ b/src/plugins/preauth/pkinit/pkinit_crypto.h
@@ -607,6 +607,11 @@ extern const krb5_data sha512_id;
extern const krb5_data oakley_1024;
extern const krb5_data oakley_2048;
extern const krb5_data oakley_4096;
+extern const krb5_data ec_p256;
+extern const krb5_data ec_p384;
+extern const krb5_data ec_p521;
+extern const krb5_data dh_oid;
+extern const krb5_data ec_oid;
/**
* An ordered set of OIDs, stored as krb5_data, of KDF algorithms
@@ -629,4 +634,6 @@ crypto_req_cert_matching_data(krb5_context context,
pkinit_req_crypto_context reqctx,
pkinit_cert_matching_data **md_out);
+int parse_dh_min_bits(krb5_context context, const char *str);
+
#endif /* _PKINIT_CRYPTO_H */
diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
index d65867a5e..a04db9db1 100644
--- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
+++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
@@ -205,6 +205,15 @@ compat_get0_DH(const EVP_PKEY *pkey)
}
+#define EVP_PKEY_get0_EC_KEY compat_get0_EC
+static EC_KEY *
+compat_get0_EC(const EVP_PKEY *pkey)
+{
+ if (pkey->type != EVP_PKEY_EC)
+ return NULL;
+ return pkey->pkey.ec;
+}
+
/* Return true if the cert c includes a key usage which doesn't include u.
* Define using direct member access for pre-1.1. */
#define ku_reject(c, u) \
@@ -284,37 +293,11 @@ decode_bn_der(const uint8_t *der, size_t len)
return bn;
}
-#if OPENSSL_VERSION_NUMBER >= 0x10100000L
-static int
-params_valid(EVP_PKEY *params)
-{
- EVP_PKEY_CTX *ctx;
- int result;
-
- ctx = EVP_PKEY_CTX_new(params, NULL);
- if (ctx == NULL)
- return 0;
- result = EVP_PKEY_param_check(ctx);
- EVP_PKEY_CTX_free(ctx);
- return result == 1;
-}
-#else
-static int
-params_valid(EVP_PKEY *params)
-{
- DH *dh;
- int codes;
-
- dh = EVP_PKEY_get0_DH(params);
- return (dh == NULL) ? 0 : (DH_check(dh, &codes) && codes == 0);
-}
-#endif
-
#if OPENSSL_VERSION_NUMBER >= 0x10100000L
#if OPENSSL_VERSION_NUMBER >= 0x30000000L
static EVP_PKEY *
-decode_dh_params(const krb5_data *params_der)
+decode_params(const krb5_data *params_der, const char *type)
{
EVP_PKEY *pkey = NULL;
const uint8_t *inptr = (uint8_t *)params_der->data;
@@ -322,7 +305,7 @@ decode_dh_params(const krb5_data *params_der)
OSSL_DECODER_CTX *dctx;
int ok;
- dctx = OSSL_DECODER_CTX_new_for_pkey(&pkey, "DER", "type-specific", "DHX",
+ dctx = OSSL_DECODER_CTX_new_for_pkey(&pkey, "DER", "type-specific", type,
EVP_PKEY_KEY_PARAMETERS, NULL, NULL);
if (dctx == NULL)
return NULL;
@@ -331,7 +314,15 @@ decode_dh_params(const krb5_data *params_der)
OSSL_DECODER_CTX_free(dctx);
return ok ? pkey : NULL;
}
+
+static EVP_PKEY *
+decode_dh_params(const krb5_data *params_der)
+{
+ return decode_params(params_der, "DHX");
+}
+
#else
+
static EVP_PKEY *
decode_dh_params(const krb5_data *params_der)
{
@@ -344,6 +335,7 @@ decode_dh_params(const krb5_data *params_der)
DH_free(dh);
return pkey;
}
+
#endif
static krb5_error_code
@@ -544,6 +536,39 @@ cleanup:
#endif /* OPENSSL_VERSION_NUMBER < 0x10100000L */
+#if OPENSSL_VERSION_NUMBER >= 0x30000000L
+
+static EVP_PKEY *
+decode_ec_params(const krb5_data *params_der)
+{
+ return decode_params(params_der, "EC");
+}
+
+#else /* OPENSSL_VERSION_NUMBER < 0x30000000L */
+
+static EVP_PKEY *
+decode_ec_params(const krb5_data *params_der)
+{
+ const uint8_t *p = (uint8_t *)params_der->data;
+ EC_KEY *eckey;
+ EVP_PKEY *pkey;
+
+ eckey = d2i_ECParameters(NULL, &p, params_der->length);
+ if (eckey == NULL)
+ return NULL;
+ pkey = EVP_PKEY_new();
+ if (pkey != NULL) {
+ if (!EVP_PKEY_set1_EC_KEY(pkey, eckey)) {
+ EVP_PKEY_free(pkey);
+ pkey = NULL;
+ }
+ }
+ EC_KEY_free(eckey);
+ return pkey;
+}
+
+#endif /* OPENSSL_VERSION_NUMBER < 0x30000000L */
+
/* Attempt to specify padded Diffie-Hellman result derivation. Don't error out
* if this fails since we also detect short results and adjust them. */
#if OPENSSL_VERSION_NUMBER >= 0x30000000L
@@ -575,7 +600,8 @@ dh_result(EVP_PKEY *pkey, EVP_PKEY *peer,
EVP_PKEY_CTX *derive_ctx = NULL;
int ok = 0;
uint8_t *buf = NULL;
- size_t len, dh_size = EVP_PKEY_get_size(pkey);
+ size_t len, result_size;
+ krb5_boolean ecc = (EVP_PKEY_id(pkey) == EVP_PKEY_EC);
*result_out = NULL;
*len_out = 0;
@@ -585,24 +611,39 @@ dh_result(EVP_PKEY *pkey, EVP_PKEY *peer,
goto cleanup;
if (EVP_PKEY_derive_init(derive_ctx) <= 0)
goto cleanup;
- set_padded_derivation(derive_ctx);
+ if (!ecc)
+ set_padded_derivation(derive_ctx);
if (EVP_PKEY_derive_set_peer(derive_ctx, peer) <= 0)
goto cleanup;
- buf = malloc(dh_size);
+ if (ecc) {
+ if (EVP_PKEY_derive(derive_ctx, NULL, &result_size) <= 0)
+ goto cleanup;
+ } else {
+ /*
+ * For finite-field Diffie-Hellman we must ensure that the result
+ * matches the key size (normally through padded derivation, but that
+ * isn't supported by OpenSSL 1.0 so we must check).
+ */
+ result_size = EVP_PKEY_get_size(pkey);
+ }
+ buf = malloc(result_size);
if (buf == NULL)
goto cleanup;
- len = dh_size;
+ len = result_size;
if (EVP_PKEY_derive(derive_ctx, buf, &len) <= 0)
goto cleanup;
- if (len < dh_size) { /* only possible without padded derivation */
- memmove(buf + (dh_size - len), buf, len);
- memset(buf, 0, dh_size - len);
+
+ /* If we couldn't specify padded derivation for finite-field DH we may need
+ * to fix up the result by right-shifting it within the buffer. */
+ if (len < result_size) {
+ memmove(buf + (result_size - len), buf, len);
+ memset(buf, 0, result_size - len);
}
ok = 1;
*result_out = buf;
- *len_out = dh_size;
+ *len_out = result_size;
buf = NULL;
cleanup:
@@ -616,13 +657,21 @@ static int
dh_pubkey_der(EVP_PKEY *pkey, uint8_t **pubkey_out, unsigned int *len_out)
{
BIGNUM *pubkey_bn = NULL;
- int len, ok;
- uint8_t *buf;
-
- if (!EVP_PKEY_get_bn_param(pkey, OSSL_PKEY_PARAM_PUB_KEY, &pubkey_bn))
- return 0;
- ok = encode_bn_der(pubkey_bn, &buf, &len);
- BN_free(pubkey_bn);
+ int len, ok = 0;
+ uint8_t *buf, *outptr;
+
+ if (EVP_PKEY_id(pkey) == EVP_PKEY_EC) {
+ len = i2d_PublicKey(pkey, NULL);
+ if (len > 0 && (outptr = buf = malloc(len)) != NULL) {
+ (void)i2d_PublicKey(pkey, &outptr);
+ ok = 1;
+ }
+ } else {
+ if (!EVP_PKEY_get_bn_param(pkey, OSSL_PKEY_PARAM_PUB_KEY, &pubkey_bn))
+ return 0;
+ ok = encode_bn_der(pubkey_bn, &buf, &len);
+ BN_free(pubkey_bn);
+ }
if (ok) {
*pubkey_out = buf;
*len_out = len;
@@ -634,19 +683,33 @@ static int
dh_pubkey_der(EVP_PKEY *pkey, uint8_t **pubkey_out, unsigned int *len_out)
{
const DH *dh;
+ EC_KEY *eckey; /* can be const when OpenSSL 1.0 dropped */
const BIGNUM *pubkey_bn;
- uint8_t *buf;
+ uint8_t *buf, *outptr;
int len;
dh = EVP_PKEY_get0_DH(pkey);
- if (dh == NULL)
- return 0;
- DH_get0_key(dh, &pubkey_bn, NULL);
- if (!encode_bn_der(pubkey_bn, &buf, &len))
- return 0;
- *pubkey_out = buf;
- *len_out = len;
- return 1;
+ if (dh != NULL) {
+ DH_get0_key(dh, &pubkey_bn, NULL);
+ if (!encode_bn_der(pubkey_bn, &buf, &len))
+ return 0;
+ *pubkey_out = buf;
+ *len_out = len;
+ return 1;
+ }
+
+ eckey = EVP_PKEY_get0_EC_KEY(pkey);
+ if (eckey != NULL) {
+ len = i2o_ECPublicKey(eckey, NULL);
+ if (len > 0 && (outptr = buf = malloc(len)) != NULL) {
+ (void)i2o_ECPublicKey(eckey, &outptr);
+ *pubkey_out = buf;
+ *len_out = len;
+ return 1;
+ }
+ }
+
+ return 0;
}
#endif
@@ -710,17 +773,23 @@ compose_dh_pkey(EVP_PKEY *params, const uint8_t *pubkey_der, size_t der_len)
if (pkey == NULL)
goto cleanup;
- pubkey_bn = decode_bn_der(pubkey_der, der_len);
- if (pubkey_bn == NULL)
- goto cleanup;
- binlen = EVP_PKEY_get_size(pkey);
- pubkey_bin = malloc(binlen);
- if (pubkey_bin == NULL)
- goto cleanup;
- if (BN_bn2binpad(pubkey_bn, pubkey_bin, binlen) != binlen)
- goto cleanup;
- if (EVP_PKEY_set1_encoded_public_key(pkey, pubkey_bin, binlen) != 1)
- goto cleanup;
+ if (EVP_PKEY_id(params) == EVP_PKEY_EC) {
+ if (d2i_PublicKey(EVP_PKEY_id(params), &pkey, &pubkey_der,
+ der_len) == NULL)
+ goto cleanup;
+ } else {
+ pubkey_bn = decode_bn_der(pubkey_der, der_len);
+ if (pubkey_bn == NULL)
+ goto cleanup;
+ binlen = EVP_PKEY_get_size(pkey);
+ pubkey_bin = malloc(binlen);
+ if (pubkey_bin == NULL)
+ goto cleanup;
+ if (BN_bn2binpad(pubkey_bn, pubkey_bin, binlen) != binlen)
+ goto cleanup;
+ if (EVP_PKEY_set1_encoded_public_key(pkey, pubkey_bin, binlen) != 1)
+ goto cleanup;
+ }
pkey_ret = pkey;
pkey = NULL;
@@ -765,29 +834,60 @@ static EVP_PKEY *
compose_dh_pkey(EVP_PKEY *params, const uint8_t *pubkey_der, size_t der_len)
{
DH *dhparams, *dh = NULL;
- EVP_PKEY *pkey = NULL;
+ EVP_PKEY *pkey = NULL, *pkey_ret = NULL;
BIGNUM *pubkey_bn = NULL;
+ EC_KEY *params_eckey, *eckey = NULL;
+ const EC_GROUP *group;
+
+ if (EVP_PKEY_id(params) == EVP_PKEY_EC) {
+ /* We would like to use EVP_PKEY_copy_parameters() and d2i_PublicKey(),
+ * but the latter is broken in OpenSSL 1.1.0-1.1.1a for EC keys. */
+ params_eckey = EVP_PKEY_get0_EC_KEY(params);
+ if (params_eckey == NULL)
+ goto cleanup;
+ group = EC_KEY_get0_group(params_eckey);
+ eckey = EC_KEY_new();
+ if (eckey == NULL)
+ goto cleanup;
+ if (!EC_KEY_set_group(eckey, group))
+ goto cleanup;
+ if (o2i_ECPublicKey(&eckey, &pubkey_der, der_len) == NULL)
+ goto cleanup;
+ pkey = EVP_PKEY_new();
+ if (pkey == NULL)
+ return NULL;
+ if (!EVP_PKEY_assign(pkey, EVP_PKEY_EC, eckey)) {
+ EVP_PKEY_free(pkey);
+ return NULL;
+ }
+ eckey = NULL;
+ } else {
+ pubkey_bn = decode_bn_der(pubkey_der, der_len);
+ if (pubkey_bn == NULL)
+ goto cleanup;
- pubkey_bn = decode_bn_der(pubkey_der, der_len);
- if (pubkey_bn == NULL)
- goto cleanup;
+ dhparams = EVP_PKEY_get0_DH(params);
+ if (dhparams == NULL)
+ goto cleanup;
+ dh = dup_dh_params(dhparams);
+ if (dh == NULL)
+ goto cleanup;
+ if (!DH_set0_key(dh, pubkey_bn, NULL))
+ goto cleanup;
+ pubkey_bn = NULL;
- dhparams = EVP_PKEY_get0_DH(params);
- if (dhparams == NULL)
- goto cleanup;
- dh = dup_dh_params(dhparams);
- if (dh == NULL)
- goto cleanup;
- if (!DH_set0_key(dh, pubkey_bn, NULL))
- goto cleanup;
- pubkey_bn = NULL;
+ pkey = dh_to_pkey(&dh);
+ }
- pkey = dh_to_pkey(&dh);
+ pkey_ret = pkey;
+ pkey = NULL;
cleanup:
BN_free(pubkey_bn);
DH_free(dh);
- return pkey;
+ EC_KEY_free(eckey);
+ EVP_PKEY_free(pkey);
+ return pkey_ret;
}
#endif /* OPENSSL_VERSION_NUMBER < 0x30000000L */
@@ -1056,7 +1156,6 @@ pkinit_init_req_crypto(pkinit_req_crypto_context *cryptoctx)
memset(ctx, 0, sizeof(*ctx));
ctx->client_pkey = NULL;
- ctx->received_params = NULL;
ctx->received_cert = NULL;
*cryptoctx = ctx;
@@ -1078,7 +1177,6 @@ pkinit_fini_req_crypto(pkinit_req_crypto_context req_cryptoctx)
pkiDebug("%s: freeing ctx at %p\n", __FUNCTION__, req_cryptoctx);
EVP_PKEY_free(req_cryptoctx->client_pkey);
- EVP_PKEY_free(req_cryptoctx->received_params);
X509_free(req_cryptoctx->received_cert);
free(req_cryptoctx);
@@ -1282,9 +1380,9 @@ pkinit_fini_pkinit_oids(pkinit_plg_crypto_context ctx)
static int
try_import_group(krb5_context context, const krb5_data *params,
- const char *name, EVP_PKEY **pkey_out)
+ const char *name, krb5_boolean ec, EVP_PKEY **pkey_out)
{
- *pkey_out = decode_dh_params(params);
+ *pkey_out = ec ? decode_ec_params(params) : decode_dh_params(params);
if (*pkey_out == NULL)
TRACE_PKINIT_DH_GROUP_UNAVAILABLE(context, name);
return (*pkey_out != NULL) ? 1 : 0;
@@ -1295,12 +1393,15 @@ pkinit_init_dh_params(krb5_context context, pkinit_plg_crypto_context plgctx)
{
int n = 0;
- n += try_import_group(context, &oakley_1024, "MODP 2 (1024-bit)",
+ n += try_import_group(context, &oakley_1024, "MODP 2 (1024-bit)", FALSE,
&plgctx->dh_1024);
- n += try_import_group(context, &oakley_2048, "MODP 14 (2048-bit)",
+ n += try_import_group(context, &oakley_2048, "MODP 14 (2048-bit)", FALSE,
&plgctx->dh_2048);
- n += try_import_group(context, &oakley_4096, "MODP 16 (4096-bit)",
+ n += try_import_group(context, &oakley_4096, "MODP 16 (4096-bit)", FALSE,
&plgctx->dh_4096);
+ n += try_import_group(context, &ec_p256, "P-256", TRUE, &plgctx->ec_p256);
+ n += try_import_group(context, &ec_p384, "P-384", TRUE, &plgctx->ec_p384);
+ n += try_import_group(context, &ec_p521, "P-521", TRUE, &plgctx->ec_p521);
if (n == 0) {
pkinit_fini_dh_params(plgctx);
@@ -1318,7 +1419,11 @@ pkinit_fini_dh_params(pkinit_plg_crypto_context plgctx)
EVP_PKEY_free(plgctx->dh_1024);
EVP_PKEY_free(plgctx->dh_2048);
EVP_PKEY_free(plgctx->dh_4096);
+ EVP_PKEY_free(plgctx->ec_p256);
+ EVP_PKEY_free(plgctx->ec_p384);
+ EVP_PKEY_free(plgctx->ec_p521);
plgctx->dh_1024 = plgctx->dh_2048 = plgctx->dh_4096 = NULL;
+ plgctx->ec_p256 = plgctx->ec_p384 = plgctx->ec_p521 = NULL;
}
static krb5_error_code
@@ -2895,6 +3000,62 @@ cleanup:
return ret;
}
+/* Return the equivalent finite-field bit strength of pkey if it matches a
+ * well-known group, or -1 if it doesn't. */
+static int
+check_dh_wellknown(pkinit_plg_crypto_context cryptoctx, EVP_PKEY *pkey)
+{
+ int nbits = EVP_PKEY_get_bits(pkey);
+
+ if (nbits == 1024 && EVP_PKEY_parameters_eq(cryptoctx->dh_1024, pkey) == 1)
+ return nbits;
+ if (nbits == 2048 && EVP_PKEY_parameters_eq(cryptoctx->dh_2048, pkey) == 1)
+ return nbits;
+ if (nbits == 4096 && EVP_PKEY_parameters_eq(cryptoctx->dh_4096, pkey) == 1)
+ return nbits;
+ if (nbits == 256 && EVP_PKEY_parameters_eq(cryptoctx->ec_p256, pkey) == 1)
+ return PKINIT_DH_P256_BITS;
+ if (nbits == 384 && EVP_PKEY_parameters_eq(cryptoctx->ec_p384, pkey) == 1)
+ return PKINIT_DH_P384_BITS;
+ if (nbits == 521 && EVP_PKEY_parameters_eq(cryptoctx->ec_p521, pkey) == 1)
+ return PKINIT_DH_P521_BITS;
+ return -1;
+}
+
+/* Return a short description of the Diffie-Hellman group with the given
+ * finite-field group size equivalent. */
+static const char *
+group_desc(int dh_bits)
+{
+ switch (dh_bits) {
+ case PKINIT_DH_P256_BITS: return "P-256";
+ case PKINIT_DH_P384_BITS: return "P-384";
+ case PKINIT_DH_P521_BITS: return "P-521";
+ case 1024: return "1024-bit DH";
+ case 2048: return "2048-bit DH";
+ case 4096: return "4096-bit DH";
+ }
+ return "(unknown)";
+}
+
+static EVP_PKEY *
+choose_dh_group(pkinit_plg_crypto_context plg_cryptoctx, int dh_size)
+{
+ if (dh_size == 1024)
+ return plg_cryptoctx->dh_1024;
+ if (dh_size == 2048)
+ return plg_cryptoctx->dh_2048;
+ if (dh_size == 4096)
+ return plg_cryptoctx->dh_4096;
+ if (dh_size == PKINIT_DH_P256_BITS)
+ return plg_cryptoctx->ec_p256;
+ if (dh_size == PKINIT_DH_P384_BITS)
+ return plg_cryptoctx->ec_p384;
+ if (dh_size == PKINIT_DH_P521_BITS)
+ return plg_cryptoctx->ec_p521;
+ return NULL;
+}
+
krb5_error_code
client_create_dh(krb5_context context,
pkinit_plg_crypto_context plg_cryptoctx,
@@ -2907,16 +3068,10 @@ client_create_dh(krb5_context context,
*spki_out = empty_data();
- if (cryptoctx->received_params != NULL)
- params = cryptoctx->received_params;
- else if (plg_cryptoctx->dh_1024 != NULL && dh_size == 1024)
- params = plg_cryptoctx->dh_1024;
- else if (plg_cryptoctx->dh_2048 != NULL && dh_size == 2048)
- params = plg_cryptoctx->dh_2048;
- else if (plg_cryptoctx->dh_4096 != NULL && dh_size == 4096)
- params = plg_cryptoctx->dh_4096;
- else
+ params = choose_dh_group(plg_cryptoctx, dh_size);
+ if (params == NULL)
goto cleanup;
+ TRACE_PKINIT_DH_PROPOSING_GROUP(context, group_desc(dh_size));
pkey = generate_dh_pkey(params);
if (pkey == NULL)
@@ -2956,8 +3111,11 @@ client_process_dh(krb5_context context,
server_pkey = compose_dh_pkey(cryptoctx->client_pkey,
subjectPublicKey_data,
subjectPublicKey_length);
- if (server_pkey == NULL)
+ if (server_pkey == NULL) {
+ retval = KRB5_PREAUTH_FAILED;
+ k5_setmsg(context, retval, _("Cannot compose PKINIT KDC public key"));
goto cleanup;
+ }
if (!dh_result(cryptoctx->client_pkey, server_pkey,
&client_key, &client_key_len))
@@ -2981,20 +3139,6 @@ cleanup:
return retval;
}
-/* Return 1 if dh is a permitted well-known group, otherwise return 0. */
-static int
-check_dh_wellknown(pkinit_plg_crypto_context cryptoctx, EVP_PKEY *pkey,
- int nbits)
-{
- if (nbits == 1024)
- return EVP_PKEY_parameters_eq(cryptoctx->dh_1024, pkey) == 1;
- else if (nbits == 2048)
- return EVP_PKEY_parameters_eq(cryptoctx->dh_2048, pkey) == 1;
- else if (nbits == 4096)
- return EVP_PKEY_parameters_eq(cryptoctx->dh_4096, pkey) == 1;
- return 0;
-}
-
krb5_error_code
server_check_dh(krb5_context context,
pkinit_plg_crypto_context cryptoctx,
@@ -3004,7 +3148,7 @@ server_check_dh(krb5_context context,
int minbits)
{
EVP_PKEY *client_pkey = NULL;
- int dh_prime_bits;
+ int dh_bits;
krb5_error_code retval = KRB5KDC_ERR_DH_KEY_PARAMETERS_NOT_ACCEPTED;
client_pkey = decode_spki(client_spki);
@@ -3013,16 +3157,15 @@ server_check_dh(krb5_context context,
goto cleanup;
}
- /* KDC SHOULD check to see if the key parameters satisfy its policy */
- dh_prime_bits = EVP_PKEY_get_bits(client_pkey);
- if (minbits && dh_prime_bits < minbits) {
- pkiDebug("client sent dh params with %d bits, we require %d\n",
- dh_prime_bits, minbits);
+ dh_bits = check_dh_wellknown(cryptoctx, client_pkey);
+ if (dh_bits == -1 || dh_bits < minbits) {
+ TRACE_PKINIT_DH_REJECTING_GROUP(context, group_desc(dh_bits),
+ group_desc(minbits));
goto cleanup;
}
+ TRACE_PKINIT_DH_RECEIVED_GROUP(context, group_desc(dh_bits));
- if (check_dh_wellknown(cryptoctx, client_pkey, dh_prime_bits))
- retval = 0;
+ retval = 0;
cleanup:
if (retval == 0)
@@ -3207,9 +3350,20 @@ pkinit_create_td_dh_parameters(krb5_context context,
krb5_algorithm_identifier alg_1024 = { dh_oid, oakley_1024 };
krb5_algorithm_identifier alg_2048 = { dh_oid, oakley_2048 };
krb5_algorithm_identifier alg_4096 = { dh_oid, oakley_4096 };
- krb5_algorithm_identifier *alglist[4];
+ krb5_algorithm_identifier alg_p256 = { ec_oid, ec_p256 };
+ krb5_algorithm_identifier alg_p384 = { ec_oid, ec_p384 };
+ krb5_algorithm_identifier alg_p521 = { ec_oid, ec_p521 };
+ krb5_algorithm_identifier *alglist[7];
i = 0;
+ if (plg_cryptoctx->ec_p256 != NULL &&
+ opts->dh_min_bits <= PKINIT_DH_P256_BITS)
+ alglist[i++] = &alg_p256;
+ if (plg_cryptoctx->ec_p384 != NULL &&
+ opts->dh_min_bits <= PKINIT_DH_P384_BITS)
+ alglist[i++] = &alg_p384;
+ if (plg_cryptoctx->ec_p521 != NULL)
+ alglist[i++] = &alg_p521;
if (plg_cryptoctx->dh_2048 != NULL && opts->dh_min_bits <= 2048)
alglist[i++] = &alg_2048;
if (plg_cryptoctx->dh_4096 != NULL && opts->dh_min_bits <= 4096)
@@ -3294,13 +3448,10 @@ pkinit_process_td_dh_params(krb5_context context,
{
krb5_error_code retval = KRB5KDC_ERR_DH_KEY_PARAMETERS_NOT_ACCEPTED;
EVP_PKEY *params = NULL;
- int i, dh_prime_bits, old_dh_size;
+ int i, dh_bits, old_dh_size;
pkiDebug("dh parameters\n");
- EVP_PKEY_free(req_cryptoctx->received_params);
- req_cryptoctx->received_params = NULL;
-
old_dh_size = *new_dh_size;
for (i = 0; algId[i] != NULL; i++) {
@@ -3308,36 +3459,22 @@ pkinit_process_td_dh_params(krb5_context context,
EVP_PKEY_free(params);
params = NULL;
- /* Skip any parameters for algorithms other than DH. */
- if (algId[i]->algorithm.length != dh_oid.length ||
- memcmp(algId[i]->algorithm.data, dh_oid.data, dh_oid.length))
- continue;
-
- params = decode_dh_params(&algId[i]->parameters);
+ if (data_eq(algId[i]->algorithm, dh_oid))
+ params = decode_dh_params(&algId[i]->parameters);
+ else if (data_eq(algId[i]->algorithm, ec_oid))
+ params = decode_ec_params(&algId[i]->parameters);
if (params == NULL)
continue;
- dh_prime_bits = EVP_PKEY_get_bits(params);
- /* Skip any parameters shorter than the previous size. */
- if (dh_prime_bits < old_dh_size)
- continue;
- pkiDebug("client sent %d DH bits server prefers %d DH bits\n",
- *new_dh_size, dh_prime_bits);
- /* If this is one of our well-known groups, just save the new size; we
- * will use our own copy of the parameters. */
- if (check_dh_wellknown(cryptoctx, params, dh_prime_bits)) {
- *new_dh_size = dh_prime_bits;
- retval = 0;
- goto cleanup;
- }
+ dh_bits = check_dh_wellknown(cryptoctx, params);
+ /* Skip any parameters shorter than the previous size or unknown. */
+ if (dh_bits == -1 || dh_bits < old_dh_size)
+ continue;
+ TRACE_PKINIT_DH_NEGOTIATED_GROUP(context, group_desc(dh_bits));
- /* If the parameters aren't well-known but check out, save them. */
- if (params_valid(params)) {
- req_cryptoctx->received_params = params;
- params = NULL;
- retval = 0;
- goto cleanup;
- }
+ *new_dh_size = dh_bits;
+ retval = 0;
+ goto cleanup;
}
cleanup:
@@ -5822,3 +5959,40 @@ crypto_req_cert_matching_data(krb5_context context,
return get_matching_data(context, plgctx, reqctx, reqctx->received_cert,
md_out);
}
+
+/*
+ * Historically, the strength of PKINIT key exchange has been determined by the
+ * pkinit_dh_min_bits variable, which gives a finite field size. With the
+ * addition of ECDH support, we allow the string values P-256, P-384, and P-521
+ * for this config variable, represented with the rough equivalent bit
+ * strengths for finite fields.
+ */
+int
+parse_dh_min_bits(krb5_context context, const char *str)
+{
+ char *endptr;
+ long n;
+
+ if (str == NULL)
+ return PKINIT_DEFAULT_DH_MIN_BITS;
+
+ n = strtol(str, &endptr, 0);
+ if (endptr == str) {
+ if (strcasecmp(str, "P-256") == 0)
+ return PKINIT_DH_P256_BITS;
+ else if (strcasecmp(str, "P-384") == 0)
+ return PKINIT_DH_P384_BITS;
+ else if (strcasecmp(str, "P-521") == 0)
+ return PKINIT_DH_P521_BITS;
+ } else {
+ if (n == 1024)
+ return 1024;
+ else if (n > 1024 && n <= 2048)
+ return 2048;
+ else if (n > 2048 && n <= 4096)
+ return 4096;
+ }
+
+ TRACE_PKINIT_DH_INVALID_MIN_BITS(context, str);
+ return PKINIT_DEFAULT_DH_MIN_BITS;
+}
diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.h b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.h
index c807f044a..b7a335880 100644
--- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.h
+++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.h
@@ -99,6 +99,9 @@ struct _pkinit_plg_crypto_context {
EVP_PKEY *dh_1024;
EVP_PKEY *dh_2048;
EVP_PKEY *dh_4096;
+ EVP_PKEY *ec_p256;
+ EVP_PKEY *ec_p384;
+ EVP_PKEY *ec_p521;
ASN1_OBJECT *id_pkinit_authData;
ASN1_OBJECT *id_pkinit_DHKeyData;
ASN1_OBJECT *id_pkinit_rkeyData;
@@ -113,7 +116,6 @@ struct _pkinit_plg_crypto_context {
struct _pkinit_req_crypto_context {
X509 *received_cert;
EVP_PKEY *client_pkey;
- EVP_PKEY *received_params;
};
#endif /* _PKINIT_CRYPTO_OPENSSL_H */
diff --git a/src/plugins/preauth/pkinit/pkinit_lib.c b/src/plugins/preauth/pkinit/pkinit_lib.c
index 4c3d46bf5..9212f7a07 100644
--- a/src/plugins/preauth/pkinit/pkinit_lib.c
+++ b/src/plugins/preauth/pkinit/pkinit_lib.c
@@ -33,9 +33,6 @@
#define FAKECERT
-const krb5_data dh_oid = { 0, 7, "\x2A\x86\x48\xce\x3e\x02\x01" };
-
-
krb5_error_code
pkinit_init_req_opts(pkinit_req_opts **reqopts)
{
diff --git a/src/plugins/preauth/pkinit/pkinit_srv.c b/src/plugins/preauth/pkinit/pkinit_srv.c
index 768a4e559..5a8d13627 100644
--- a/src/plugins/preauth/pkinit/pkinit_srv.c
+++ b/src/plugins/preauth/pkinit/pkinit_srv.c
@@ -1070,7 +1070,7 @@ static krb5_error_code
pkinit_init_kdc_profile(krb5_context context, pkinit_kdc_context plgctx)
{
krb5_error_code retval;
- char *eku_string = NULL, *ocsp_check = NULL;
+ char *eku_string = NULL, *ocsp_check = NULL, *minbits = NULL;
pkiDebug("%s: entered for realm %s\n", __FUNCTION__, plgctx->realmname);
retval = pkinit_kdcdefault_string(context, plgctx->realmname,
@@ -1115,17 +1115,10 @@ pkinit_init_kdc_profile(krb5_context context, pkinit_kdc_context plgctx)
goto errout;
}
- pkinit_kdcdefault_integer(context, plgctx->realmname,
- KRB5_CONF_PKINIT_DH_MIN_BITS,
- PKINIT_DEFAULT_DH_MIN_BITS,
- &plgctx->opts->dh_min_bits);
- if (plgctx->opts->dh_min_bits < PKINIT_DH_MIN_CONFIG_BITS) {
- pkiDebug("%s: invalid value (%d < %d) for pkinit_dh_min_bits, "
- "using default value (%d) instead\n", __FUNCTION__,
- plgctx->opts->dh_min_bits, PKINIT_DH_MIN_CONFIG_BITS,
- PKINIT_DEFAULT_DH_MIN_BITS);
- plgctx->opts->dh_min_bits = PKINIT_DEFAULT_DH_MIN_BITS;
- }
+ pkinit_kdcdefault_string(context, plgctx->realmname,
+ KRB5_CONF_PKINIT_DH_MIN_BITS, &minbits);
+ plgctx->opts->dh_min_bits = parse_dh_min_bits(context, minbits);
+ free(minbits);
pkinit_kdcdefault_boolean(context, plgctx->realmname,
KRB5_CONF_PKINIT_ALLOW_UPN,
diff --git a/src/plugins/preauth/pkinit/pkinit_trace.h b/src/plugins/preauth/pkinit/pkinit_trace.h
index 5ee39c085..1a9545dc7 100644
--- a/src/plugins/preauth/pkinit/pkinit_trace.h
+++ b/src/plugins/preauth/pkinit/pkinit_trace.h
@@ -92,6 +92,17 @@
#define TRACE_PKINIT_DH_GROUP_UNAVAILABLE(c, name) \
TRACE(c, "PKINIT key exchange group {str} unsupported", name)
+#define TRACE_PKINIT_DH_INVALID_MIN_BITS(c, str) \
+ TRACE(c, "Invalid pkinit_dh_min_bits value {str}, using default", str)
+#define TRACE_PKINIT_DH_NEGOTIATED_GROUP(c, desc) \
+ TRACE(c, "PKINIT accepting KDC key exchange group preference {str}", desc)
+#define TRACE_PKINIT_DH_PROPOSING_GROUP(c, desc) \
+ TRACE(c, "PKINIT using {str} key exchange group", desc)
+#define TRACE_PKINIT_DH_RECEIVED_GROUP(c, desc) \
+ TRACE(c, "PKINIT received {str} key from client for key exchange", desc)
+#define TRACE_PKINIT_DH_REJECTING_GROUP(c, desc, mindesc) \
+ TRACE(c, "PKINIT client key has group {str}, need at least {str}", \
+ desc, mindesc)
#define TRACE_PKINIT_OPENSSL_ERROR(c, msg) \
TRACE(c, "PKINIT OpenSSL error: {str}", msg)
diff --git a/src/tests/t_pkinit.py b/src/tests/t_pkinit.py
index ec2356ea2..8245ba1c6 100755
--- a/src/tests/t_pkinit.py
+++ b/src/tests/t_pkinit.py
@@ -172,6 +172,15 @@ realm.pkinit(realm.user_princ, expected_trace=msgs)
realm.klist(realm.user_princ)
realm.run([kvno, realm.host_princ])
+# Test each Diffie-Hellman group except 1024-bit (which doesn't work
+# in OpenSSL 3.0) and the default 2048-bit group.
+for g in ('4096', 'P-256', 'P-384', 'P-521'):
+ mark('Diffie-Hellman group ' + g)
+ group_conf = {'realms': {'$realm': {'pkinit_dh_min_bits': g}}}
+ group_env = realm.special_env(g, True, krb5_conf=group_conf)
+ realm.pkinit(realm.user_princ, expected_trace=('PKINIT using ' + g,),
+ env=group_env)
+
# Try using multiple configured pkinit_identities, to make sure we
# fall back to the second one when the first one cannot be read.
id_conf = {'realms': {'$realm': {'pkinit_identities': [file_identity + 'X',
@@ -197,11 +206,14 @@ realm.start_kdc(env=minbits_env)
msgs = ('Sending unauthenticated request',
'/Additional pre-authentication required',
'Preauthenticating using KDC method data',
+ 'PKINIT using 2048-bit DH key exchange group',
'Preauth module pkinit (16) (real) returned: 0/Success',
' preauth for next request: PA-FX-COOKIE (133), PA-PK-AS-REQ (16)',
'/Key parameters not accepted',
'Preauth tryagain input types (16): 109, PA-FX-COOKIE (133)',
+ 'PKINIT accepting KDC key exchange group preference P-384',
'trying again with KDC-provided parameters',
+ 'PKINIT using P-384 key exchange group',
'Preauth module pkinit (16) tryagain returned: 0/Success',
' preauth for next request: PA-PK-AS-REQ (16), PA-FX-COOKIE (133)')
realm.pkinit(realm.user_princ, expected_trace=msgs)
More information about the cvs-krb5
mailing list