[RFC 0/8] Crypto operations by key ID

[RFC 0/8] Crypto operations by key ID

[James Prestwood]

This adds support for several key based crypto opterations but
instead of using a key directly it uses a key ID. This avoids
the need for ELL to ever hold a copy of the key assuming it
already exists in the kernel.

The motivation behind this is to enhance IWDs profile encryption.
Currently this uses a systemd feature but ultimately the secret
key is decrypted and put onto the file system for IWD to use.
This isn't desirable and it would be better if the key never hits
the FS, and even better if IWD never even sees it. This would
allow some external, trusted, entity (e.g. PAM/systemd-logind) to
set a secret key into the kernel upon user login (e.g. the users
password). This key ID could then be used by IWD to encrypt
profiles without ever seeing the actual key.

James Prestwood (8):
  key: add l_key_search
  unit: add key search test
  checksum: commonize checksum creation
  checksum: add l_checksum_new_hmac_from_key_id
  cert-crypto: refactor l_cert_pkcs5_pbkdf2
  cert: add l_cert_pkcs5_pbkdf2_from_key_id
  cert: add explicit length to l_cert_pkcs5_pbkdf2
  unit: update test-pbkdf2 with API change

 ell/cert-crypto.c  |  97 +++++++++++++++++++++++++++-----------
 ell/cert.h         |   7 ++-
 ell/checksum.c     | 114 ++++++++++++++++++++-------------------------
 ell/checksum.h     |   2 +
 ell/ell.sym        |   3 ++
 ell/key.c          |  46 ++++++++++++++++++
 ell/key.h          |   3 ++
 unit/test-key.c    |  26 +++++++++++
 unit/test-pbkdf2.c |   1 +
 9 files changed, 208 insertions(+), 91 deletions(-)

-- 
2.34.3

[RFC 1/8] key: add l_key_search

[James Prestwood]

Search for a key by type, keyring name, and description. Returns the
key ID or an error if not found.
---
 ell/ell.sym |  1 +
 ell/key.c   | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 ell/key.h   |  3 +++
 3 files changed, 50 insertions(+)

diff --git a/ell/ell.sym b/ell/ell.sym
index 6df9024..414b288 100644
--- a/ell/ell.sym
+++ b/ell/ell.sym
@@ -387,6 +387,7 @@ global:
 	l_key_decrypt;
 	l_key_sign;
 	l_key_verify;
+	l_key_search;
 	l_keyring_new;
 	l_keyring_restrict;
 	l_keyring_free;
diff --git a/ell/key.c b/ell/key.c
index 24374a5..5a82531 100644
--- a/ell/key.c
+++ b/ell/key.c
@@ -270,6 +270,26 @@ static long kernel_key_verify(int32_t serial,
 	return result >= 0 ? result : -errno;
 }
 
+static long kernel_key_request(const char *type, const char *description)
+{
+	long result;
+
+	result = syscall(__NR_request_key, type, description, NULL, 0);
+
+	return result >= 0 ? result : -errno;
+}
+
+static long kernel_key_search(int32_t keyring_id, const char *type,
+				const char *description)
+{
+	long result;
+
+	result = syscall(__NR_keyctl, KEYCTL_SEARCH, keyring_id, type,
+				description, 0);
+
+	return result >= 0 ? result : -errno;
+}
+
 static bool setup_internal_keyring(void)
 {
 	internal_keyring = kernel_add_key("keyring", "ell-internal", NULL, 0,
@@ -283,6 +303,32 @@ static bool setup_internal_keyring(void)
 	return true;
 }
 
+LIB_EXPORT int32_t l_key_search(enum l_key_type type, const char *keyring,
+					const char *description)
+{
+	long keyring_id;
+	long key_id;
+
+	if (unlikely((size_t)type >= L_ARRAY_SIZE(key_type_names)))
+		return -EINVAL;
+
+	if (unlikely(!keyring || !description))
+		return -EINVAL;
+
+	/* Find the ID of the keyring */
+	keyring_id = kernel_key_request("keyring", keyring);
+	if (keyring_id < 0)
+		return -ENOENT;
+
+	/* Search for the key by type/description */
+	key_id = kernel_key_search(keyring_id, key_type_names[type],
+					description);
+	if (key_id < 0)
+		return -ENOENT;
+
+	return key_id;
+}
+
 LIB_EXPORT struct l_key *l_key_new(enum l_key_type type, const void *payload,
 					size_t payload_length)
 {
diff --git a/ell/key.h b/ell/key.h
index 6897105..5fe8e00 100644
--- a/ell/key.h
+++ b/ell/key.h
@@ -62,6 +62,9 @@ enum l_key_cipher_type {
 struct l_key *l_key_new(enum l_key_type type, const void *payload,
 			size_t payload_length);
 
+int32_t l_key_search(enum l_key_type type, const char *keyring,
+					const char *description);
+
 void l_key_free(struct l_key *key);
 void l_key_free_norevoke(struct l_key *key);
 
-- 
2.34.3

[RFC 2/8] unit: add key search test

[James Prestwood]

Creates a keyring and key using syscall directly as this is the
intended use case of l_key_search() (keys outside of ELL). Then
verifies that l_key_search() returns the same ID as the created key.
---
 unit/test-key.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/unit/test-key.c b/unit/test-key.c
index 240a02c..0e09cf4 100644
--- a/unit/test-key.c
+++ b/unit/test-key.c
@@ -24,6 +24,9 @@
 #include <config.h>
 #endif
 
+#include <sys/syscall.h>
+#include <linux/keyctl.h>
+#include <unistd.h>
 #include <assert.h>
 
 #include <ell/ell.h>
@@ -661,6 +664,27 @@ static void test_key_crypto(const void *data)
 	l_key_free(pubkey);
 }
 
+static void test_key_search(const void *data)
+{
+	long keyring_id;
+	long key_id;
+
+	/*
+	 * Search is used to find a key out side of ELL's internal keyring. So
+	 * manually create a keyring/key here rather than using l_key APIs
+	 */
+
+	keyring_id = syscall(__NR_add_key, "keyring", "my-keyring", NULL,
+				0, KEY_SPEC_USER_KEYRING);
+	assert(keyring_id >= 0);
+
+	key_id = syscall(__NR_add_key, "user", "my-key",
+				KEY1_STR, KEY1_LEN, keyring_id);
+	assert(key_id >= 0);
+
+	assert(key_id == l_key_search(L_KEY_RAW, "my-keyring", "my-key"));
+}
+
 int main(int argc, char *argv[])
 {
 	l_test_init(&argc, &argv);
@@ -685,5 +709,7 @@ int main(int argc, char *argv[])
 	if (l_key_is_supported(L_KEY_FEATURE_CRYPTO))
 		l_test_add("key crypto", test_key_crypto, NULL);
 
+	l_test_add("key search", test_key_search, NULL);
+
 	return l_test_run();
 }
-- 
2.34.3

[RFC 3/8] checksum: commonize checksum creation

[James Prestwood]

The various checksums were using virutally the same init code. Make
this common which will make initialization by key ID much simpler
to add.
---
 ell/checksum.c | 103 ++++++++++++++++++-------------------------------
 1 file changed, 37 insertions(+), 66 deletions(-)

diff --git a/ell/checksum.c b/ell/checksum.c
index c71205a..e17f070 100644
--- a/ell/checksum.c
+++ b/ell/checksum.c
@@ -146,55 +146,22 @@ static int create_alg(const char *alg)
 	return sk;
 }
 
-/**
- * l_checksum_new:
- * @type: checksum type
- *
- * Creates new #l_checksum, using the checksum algorithm @type.
- *
- * Returns: a newly allocated #l_checksum object.
- **/
-LIB_EXPORT struct l_checksum *l_checksum_new(enum l_checksum_type type)
-{
-	struct l_checksum *checksum;
-	int fd;
-
-	if (!is_valid_index(checksum_algs, type) || !checksum_algs[type].name)
-		return NULL;
-
-	checksum = l_new(struct l_checksum, 1);
-	checksum->alg_info = &checksum_algs[type];
-
-	fd = create_alg(checksum->alg_info->name);
-	if (fd < 0)
-		goto error;
-
-	checksum->sk = accept4(fd, NULL, 0, SOCK_CLOEXEC);
-	close(fd);
-
-	if (checksum->sk < 0)
-		goto error;
-
-	return checksum;
-
-error:
-	l_free(checksum);
-	return NULL;
-}
-
-LIB_EXPORT struct l_checksum *l_checksum_new_cmac_aes(const void *key,
-							size_t key_len)
+static struct l_checksum *checksum_new_common(const char *alg, int sockopt,
+						const void *data, size_t len,
+						struct checksum_info *info)
 {
 	struct l_checksum *checksum;
 	int fd;
 
-	fd = create_alg("cmac(aes)");
+	fd = create_alg(alg);
 	if (fd < 0)
 		return NULL;
 
-	if (setsockopt(fd, SOL_ALG, ALG_SET_KEY, key, key_len) < 0) {
-		close(fd);
-		return NULL;
+	if (data) {
+		if (setsockopt(fd, SOL_ALG, sockopt, data, len) < 0) {
+			close(fd);
+			return NULL;
+		}
 	}
 
 	checksum = l_new(struct l_checksum, 1);
@@ -206,40 +173,44 @@ LIB_EXPORT struct l_checksum *l_checksum_new_cmac_aes(const void *key,
 		return NULL;
 	}
 
-	checksum->alg_info = &checksum_cmac_aes_alg;
+	checksum->alg_info = info;
 	return checksum;
 }
 
-LIB_EXPORT struct l_checksum *l_checksum_new_hmac(enum l_checksum_type type,
-					  const void *key, size_t key_len)
+/**
+ * l_checksum_new:
+ * @type: checksum type
+ *
+ * Creates new #l_checksum, using the checksum algorithm @type.
+ *
+ * Returns: a newly allocated #l_checksum object.
+ **/
+LIB_EXPORT struct l_checksum *l_checksum_new(enum l_checksum_type type)
 {
-	struct l_checksum *checksum;
-	int fd;
-
-	if (!is_valid_index(checksum_hmac_algs, type) ||
-			!checksum_hmac_algs[type].name)
-		return NULL;
-
-	fd = create_alg(checksum_hmac_algs[type].name);
-	if (fd < 0)
+	if (!is_valid_index(checksum_algs, type) || !checksum_algs[type].name)
 		return NULL;
 
-	if (setsockopt(fd, SOL_ALG, ALG_SET_KEY, key, key_len) < 0) {
-		close(fd);
-		return NULL;
-	}
+	return checksum_new_common(checksum_algs[type].name, 0, NULL, 0,
+					&checksum_algs[type]);
+}
 
-	checksum = l_new(struct l_checksum, 1);
-	checksum->sk = accept4(fd, NULL, 0, SOCK_CLOEXEC);
-	close(fd);
+LIB_EXPORT struct l_checksum *l_checksum_new_cmac_aes(const void *key,
+							size_t key_len)
+{
+	return checksum_new_common("cmac(aes)", ALG_SET_KEY, key, key_len,
+					&checksum_cmac_aes_alg);
+}
 
-	if (checksum->sk < 0) {
-		l_free(checksum);
+LIB_EXPORT struct l_checksum *l_checksum_new_hmac(enum l_checksum_type type,
+					  const void *key, size_t key_len)
+{
+	if (!is_valid_index(checksum_hmac_algs, type) ||
+			!checksum_hmac_algs[type].name)
 		return NULL;
-	}
 
-	checksum->alg_info = &checksum_hmac_algs[type];
-	return checksum;
+	return checksum_new_common(checksum_hmac_algs[type].name,
+					ALG_SET_KEY, key, key_len,
+					&checksum_hmac_algs[type]);
 }
 
 /**
-- 
2.34.3

[RFC 4/8] checksum: add l_checksum_new_hmac_from_key_id

[James Prestwood]

Same as l_checksum_new_hmac but uses a key ID rather than a
key directly.
---
 ell/checksum.c | 17 +++++++++++++++++
 ell/checksum.h |  2 ++
 ell/ell.sym    |  1 +
 3 files changed, 20 insertions(+)

diff --git a/ell/checksum.c b/ell/checksum.c
index e17f070..dc5e61a 100644
--- a/ell/checksum.c
+++ b/ell/checksum.c
@@ -68,6 +68,10 @@ struct sockaddr_alg {
 #define SOL_ALG 279
 #endif
 
+#ifndef ALG_SET_KEY_BY_KEY_SERIAL
+#define ALG_SET_KEY_BY_KEY_SERIAL 7
+#endif
+
 struct checksum_info {
 	const char *name;
 	uint8_t digest_len;
@@ -213,6 +217,19 @@ LIB_EXPORT struct l_checksum *l_checksum_new_hmac(enum l_checksum_type type,
 					&checksum_hmac_algs[type]);
 }
 
+struct l_checksum *l_checksum_new_hmac_from_key_id(enum l_checksum_type type,
+							int32_t key_id)
+{
+	if (!is_valid_index(checksum_hmac_algs, type) ||
+			!checksum_hmac_algs[type].name)
+		return NULL;
+
+	return checksum_new_common(checksum_hmac_algs[type].name,
+					ALG_SET_KEY_BY_KEY_SERIAL,
+					&key_id, sizeof(key_id),
+					&checksum_hmac_algs[type]);
+}
+
 /**
  * l_checksum_clone:
  * @checksum: parent checksum object
diff --git a/ell/checksum.h b/ell/checksum.h
index 531fcb0..3dab13f 100644
--- a/ell/checksum.h
+++ b/ell/checksum.h
@@ -48,6 +48,8 @@ struct l_checksum *l_checksum_new(enum l_checksum_type type);
 struct l_checksum *l_checksum_new_cmac_aes(const void *key, size_t key_len);
 struct l_checksum *l_checksum_new_hmac(enum l_checksum_type type,
 					const void *key, size_t key_len);
+struct l_checksum *l_checksum_new_hmac_from_key_id(enum l_checksum_type type,
+							int32_t key_id);
 struct l_checksum *l_checksum_clone(struct l_checksum *checksum);
 
 void l_checksum_free(struct l_checksum *checksum);
diff --git a/ell/ell.sym b/ell/ell.sym
index 414b288..08252b8 100644
--- a/ell/ell.sym
+++ b/ell/ell.sym
@@ -115,6 +115,7 @@ global:
 	l_checksum_new;
 	l_checksum_new_cmac_aes;
 	l_checksum_new_hmac;
+	l_checksum_new_hmac_from_key_id;
 	l_checksum_clone;
 	l_checksum_free;
 	l_checksum_reset;
-- 
2.34.3

[RFC 5/8] cert-crypto: refactor l_cert_pkcs5_pbkdf2

[James Prestwood]

This makes the actual algorithm common to prepare for adding a
new variant which uses a key ID rather than password.
---
 ell/cert-crypto.c | 67 +++++++++++++++++++++++++++++------------------
 1 file changed, 41 insertions(+), 26 deletions(-)

diff --git a/ell/cert-crypto.c b/ell/cert-crypto.c
index e6e8876..bf748b0 100644
--- a/ell/cert-crypto.c
+++ b/ell/cert-crypto.c
@@ -103,44 +103,34 @@ LIB_EXPORT bool l_cert_pkcs5_pbkdf1(enum l_checksum_type type,
 	return !iter_count;
 }
 
-/* RFC8018 section 5.2 */
-LIB_EXPORT bool l_cert_pkcs5_pbkdf2(enum l_checksum_type type,
-					const char *password,
-					const uint8_t *salt, size_t salt_len,
-					unsigned int iter_count,
-					uint8_t *out_dk, size_t dk_len)
+static size_t cert_checksum_to_length(enum l_checksum_type type)
 {
-	size_t h_len;
-	struct l_checksum *checksum;
-	unsigned int i;
-
 	switch (type) {
 	case L_CHECKSUM_SHA1:
-		h_len = 20;
-		break;
+		return 20;
 	case L_CHECKSUM_SHA224:
-		h_len = 28;
-		break;
+		return 28;
 	case L_CHECKSUM_SHA256:
-		h_len = 32;
-		break;
+		return 32;
 	case L_CHECKSUM_SHA384:
-		h_len = 48;
-		break;
+		return 48;
 	case L_CHECKSUM_SHA512:
-		h_len = 64;
-		break;
+		return 64;
 	case L_CHECKSUM_NONE:
 	case L_CHECKSUM_MD4:
 	case L_CHECKSUM_MD5:
-		return false;
+		return 0;
 	default:
-		return false;
+		return 0;
 	}
+}
 
-	checksum = l_checksum_new_hmac(type, password, strlen(password));
-	if (!checksum)
-		return false;
+static bool cert_pkcs5_pbkdf2(struct l_checksum *checksum, const uint8_t *salt,
+				size_t salt_len, size_t h_len,
+				unsigned int iter_count, uint8_t *out_dk,
+				size_t dk_len)
+{
+	unsigned int i;
 
 	for (i = 1; dk_len; i++) {
 		unsigned int j, k;
@@ -180,9 +170,34 @@ LIB_EXPORT bool l_cert_pkcs5_pbkdf2(enum l_checksum_type type,
 		dk_len -= block_len;
 	}
 
+	return !dk_len;
+}
+
+/* RFC8018 section 5.2 */
+LIB_EXPORT bool l_cert_pkcs5_pbkdf2(enum l_checksum_type type,
+					const char *password,
+					const uint8_t *salt, size_t salt_len,
+					unsigned int iter_count,
+					uint8_t *out_dk, size_t dk_len)
+{
+	size_t h_len;
+	struct l_checksum *checksum;
+	bool r;
+
+	h_len = cert_checksum_to_length(type);
+	if (!h_len)
+		return false;
+
+	checksum = l_checksum_new_hmac(type, password, strlen(password));
+	if (!checksum)
+		return false;
+
+	r = cert_pkcs5_pbkdf2(checksum, salt, salt_len, h_len, iter_count,
+				out_dk, dk_len);
+
 	l_checksum_free(checksum);
 
-	return !dk_len;
+	return r;
 }
 
 /* RFC7292 Appendix B */
-- 
2.34.3

[RFC 6/8] cert: add l_cert_pkcs5_pbkdf2_from_key_id

[James Prestwood]

The same pbkdf2 algorithm but uses a key ID as the password.
---
 ell/cert-crypto.c | 27 +++++++++++++++++++++++++++
 ell/cert.h        |  6 +++++-
 ell/ell.sym       |  1 +
 3 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/ell/cert-crypto.c b/ell/cert-crypto.c
index bf748b0..42f602e 100644
--- a/ell/cert-crypto.c
+++ b/ell/cert-crypto.c
@@ -200,6 +200,33 @@ LIB_EXPORT bool l_cert_pkcs5_pbkdf2(enum l_checksum_type type,
 	return r;
 }
 
+LIB_EXPORT bool l_cert_pkcs5_pbkdf2_from_key_id(enum l_checksum_type type,
+						int32_t key_id,
+						const uint8_t *salt,
+						size_t salt_len,
+						unsigned int iter_count,
+						uint8_t *out_dk, size_t dk_len)
+{
+	size_t h_len;
+	struct l_checksum *checksum;
+	bool r;
+
+	h_len = cert_checksum_to_length(type);
+	if (!h_len)
+		return false;
+
+	checksum = l_checksum_new_hmac_from_key_id(type, key_id);
+	if (!checksum)
+		return false;
+
+	r = cert_pkcs5_pbkdf2(checksum, salt, salt_len, h_len, iter_count,
+				out_dk, dk_len);
+
+	l_checksum_free(checksum);
+
+	return r;
+}
+
 /* RFC7292 Appendix B */
 uint8_t *cert_pkcs12_pbkdf(const char *password,
 				const struct cert_pkcs12_hash *hash,
diff --git a/ell/cert.h b/ell/cert.h
index f637588..ce430fa 100644
--- a/ell/cert.h
+++ b/ell/cert.h
@@ -76,7 +76,11 @@ bool l_cert_pkcs5_pbkdf2(enum l_checksum_type type, const char *password,
 				const uint8_t *salt, size_t salt_len,
 				unsigned int iter_count,
 				uint8_t *out_dk, size_t dk_len);
-
+bool l_cert_pkcs5_pbkdf2_from_key_id(enum l_checksum_type type,
+					int32_t key_id, const uint8_t *salt,
+					size_t salt_len,
+					unsigned int iter_count,
+					uint8_t *out_dk, size_t dk_len);
 #ifdef __cplusplus
 }
 #endif
diff --git a/ell/ell.sym b/ell/ell.sym
index 08252b8..2572989 100644
--- a/ell/ell.sym
+++ b/ell/ell.sym
@@ -565,6 +565,7 @@ global:
 	l_cert_load_container_file;
 	l_cert_pkcs5_pbkdf1;
 	l_cert_pkcs5_pbkdf2;
+	l_cert_pkcs5_pbkdf2_from_key_id;
 	/* ecc */
 	l_ecc_supported_ike_groups;
 	l_ecc_supported_tls_groups;
-- 
2.34.3

[RFC 7/8] cert: add explicit length to l_cert_pkcs5_pbkdf2

[James Prestwood]

Rather than assume the password/key is a NULL terminated string pass
the length in too. This is more flexible in case of a binary key.
---
 ell/cert-crypto.c | 7 ++++---
 ell/cert.h        | 1 +
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/ell/cert-crypto.c b/ell/cert-crypto.c
index 42f602e..ec14c04 100644
--- a/ell/cert-crypto.c
+++ b/ell/cert-crypto.c
@@ -175,7 +175,7 @@ static bool cert_pkcs5_pbkdf2(struct l_checksum *checksum, const uint8_t *salt,
 
 /* RFC8018 section 5.2 */
 LIB_EXPORT bool l_cert_pkcs5_pbkdf2(enum l_checksum_type type,
-					const char *password,
+					const char *password, size_t pass_len,
 					const uint8_t *salt, size_t salt_len,
 					unsigned int iter_count,
 					uint8_t *out_dk, size_t dk_len)
@@ -188,7 +188,7 @@ LIB_EXPORT bool l_cert_pkcs5_pbkdf2(enum l_checksum_type type,
 	if (!h_len)
 		return false;
 
-	checksum = l_checksum_new_hmac(type, password, strlen(password));
+	checksum = l_checksum_new_hmac(type, password, pass_len);
 	if (!checksum)
 		return false;
 
@@ -648,7 +648,8 @@ static struct l_cipher *cipher_from_pkcs5_pbes2_params(
 
 	/* RFC8018 section 6.2 */
 
-	if (!l_cert_pkcs5_pbkdf2(prf_alg, password, salt, salt_len, iter_count,
+	if (!l_cert_pkcs5_pbkdf2(prf_alg, password, strlen(password), salt,
+					salt_len, iter_count,
 					derived_key, key_len))
 		return NULL;
 
diff --git a/ell/cert.h b/ell/cert.h
index ce430fa..8ad57b6 100644
--- a/ell/cert.h
+++ b/ell/cert.h
@@ -73,6 +73,7 @@ bool l_cert_pkcs5_pbkdf1(enum l_checksum_type type, const char *password,
 				unsigned int iter_count,
 				uint8_t *out_dk, size_t dk_len);
 bool l_cert_pkcs5_pbkdf2(enum l_checksum_type type, const char *password,
+				size_t pass_len,
 				const uint8_t *salt, size_t salt_len,
 				unsigned int iter_count,
 				uint8_t *out_dk, size_t dk_len);
-- 
2.34.3

[RFC 8/8] unit: update test-pbkdf2 with API change

[James Prestwood]

---
 unit/test-pbkdf2.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/unit/test-pbkdf2.c b/unit/test-pbkdf2.c
index 3f272ce..1053de2 100644
--- a/unit/test-pbkdf2.c
+++ b/unit/test-pbkdf2.c
@@ -52,6 +52,7 @@ static void pbkdf2_test(const void *data)
 	key_len = test->key_len ? : (strlen(test->key) / 2);
 
 	result = l_cert_pkcs5_pbkdf2(L_CHECKSUM_SHA1, test->password,
+					strlen(test->password),
 					(const uint8_t *) test->salt, salt_len,
 					test->count, output, key_len);
 
-- 
2.34.3

Re: [RFC 1/8] key: add l_key_search

[Denis Kenzior]

Hi James,

On 11/18/22 15:16, James Prestwood wrote:
> Search for a key by type, keyring name, and description. Returns the
> key ID or an error if not found.
> ---
>   ell/ell.sym |  1 +
>   ell/key.c   | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>   ell/key.h   |  3 +++
>   3 files changed, 50 insertions(+)
> 

<snip>

> @@ -283,6 +303,32 @@ static bool setup_internal_keyring(void)
>   	return true;
>   }
>   
> +LIB_EXPORT int32_t l_key_search(enum l_key_type type, const char *keyring,

How likely are we to search some custom keyring?  Wouldn't we generally be 
searching a user/default user session keyrings?

> +					const char *description)
> +{
> +	long keyring_id;
> +	long key_id;
> +
> +	if (unlikely((size_t)type >= L_ARRAY_SIZE(key_type_names)))
> +		return -EINVAL;
> +
> +	if (unlikely(!keyring || !description))
> +		return -EINVAL;
> +
> +	/* Find the ID of the keyring */
> +	keyring_id = kernel_key_request("keyring", keyring);
> +	if (keyring_id < 0)
> +		return -ENOENT;
> +
> +	/* Search for the key by type/description */
> +	key_id = kernel_key_search(keyring_id, key_type_names[type],
> +					description);
> +	if (key_id < 0)
> +		return -ENOENT;
> +
> +	return key_id;
> +}
> +
>   LIB_EXPORT struct l_key *l_key_new(enum l_key_type type, const void *payload,
>   					size_t payload_length)
>   {

Regards,
-Denis

Re: [RFC 3/8] checksum: commonize checksum creation

[Denis Kenzior]

Hi James,

On 11/18/22 15:16, James Prestwood wrote:
> The various checksums were using virutally the same init code. Make
> this common which will make initialization by key ID much simpler
> to add.
> ---
>   ell/checksum.c | 103 ++++++++++++++++++-------------------------------
>   1 file changed, 37 insertions(+), 66 deletions(-)
> 

Applied, thanks.

Regards,
-Denis

Re: [RFC 4/8] checksum: add l_checksum_new_hmac_from_key_id

[Denis Kenzior]

Hi James,

On 11/18/22 15:16, James Prestwood wrote:
> Same as l_checksum_new_hmac but uses a key ID rather than a
> key directly.
> ---
>   ell/checksum.c | 17 +++++++++++++++++
>   ell/checksum.h |  2 ++
>   ell/ell.sym    |  1 +
>   3 files changed, 20 insertions(+)
> 

<snip>

> @@ -213,6 +217,19 @@ LIB_EXPORT struct l_checksum *l_checksum_new_hmac(enum l_checksum_type type,
>   					&checksum_hmac_algs[type]);
>   }
>   
> +struct l_checksum *l_checksum_new_hmac_from_key_id(enum l_checksum_type type,
> +							int32_t key_id)

Missing LIB_EXPORT?

> +{
> +	if (!is_valid_index(checksum_hmac_algs, type) ||
> +			!checksum_hmac_algs[type].name)
> +		return NULL;
> +
> +	return checksum_new_common(checksum_hmac_algs[type].name,
> +					ALG_SET_KEY_BY_KEY_SERIAL,
> +					&key_id, sizeof(key_id),
> +					&checksum_hmac_algs[type]);
> +}
> +
>   /**
>    * l_checksum_clone:
>    * @checksum: parent checksum object

Regards,
-Denis

Re: [RFC 5/8] cert-crypto: refactor l_cert_pkcs5_pbkdf2

[Denis Kenzior]

Hi James,

On 11/18/22 15:16, James Prestwood wrote:
> This makes the actual algorithm common to prepare for adding a
> new variant which uses a key ID rather than password.
> ---
>   ell/cert-crypto.c | 67 +++++++++++++++++++++++++++++------------------
>   1 file changed, 41 insertions(+), 26 deletions(-)
> 
> diff --git a/ell/cert-crypto.c b/ell/cert-crypto.c
> index e6e8876..bf748b0 100644
> --- a/ell/cert-crypto.c
> +++ b/ell/cert-crypto.c
> @@ -103,44 +103,34 @@ LIB_EXPORT bool l_cert_pkcs5_pbkdf1(enum l_checksum_type type,
>   	return !iter_count;
>   }
>   
> -/* RFC8018 section 5.2 */
> -LIB_EXPORT bool l_cert_pkcs5_pbkdf2(enum l_checksum_type type,
> -					const char *password,
> -					const uint8_t *salt, size_t salt_len,
> -					unsigned int iter_count,
> -					uint8_t *out_dk, size_t dk_len)
> +static size_t cert_checksum_to_length(enum l_checksum_type type)

We already have l_checksum_digest_length().  Should we use that?

>   {
> -	size_t h_len;
> -	struct l_checksum *checksum;
> -	unsigned int i;
> -
>   	switch (type) {
>   	case L_CHECKSUM_SHA1:
> -		h_len = 20;
> -		break;
> +		return 20;
>   	case L_CHECKSUM_SHA224:
> -		h_len = 28;
> -		break;
> +		return 28;
>   	case L_CHECKSUM_SHA256:
> -		h_len = 32;
> -		break;
> +		return 32;
>   	case L_CHECKSUM_SHA384:
> -		h_len = 48;
> -		break;
> +		return 48;
>   	case L_CHECKSUM_SHA512:
> -		h_len = 64;
> -		break;
> +		return 64;
>   	case L_CHECKSUM_NONE:
>   	case L_CHECKSUM_MD4:
>   	case L_CHECKSUM_MD5:
> -		return false;
> +		return 0;
>   	default:
> -		return false;
> +		return 0;
>   	}
> +}
>   
> -	checksum = l_checksum_new_hmac(type, password, strlen(password));
> -	if (!checksum)
> -		return false;
> +static bool cert_pkcs5_pbkdf2(struct l_checksum *checksum, const uint8_t *salt,
> +				size_t salt_len, size_t h_len,
> +				unsigned int iter_count, uint8_t *out_dk,
> +				size_t dk_len)
> +{
> +	unsigned int i;
>   
>   	for (i = 1; dk_len; i++) {
>   		unsigned int j, k;

Regards,
-Denis

Re: [RFC 6/8] cert: add l_cert_pkcs5_pbkdf2_from_key_id

[Denis Kenzior]

Hi James,

On 11/18/22 15:16, James Prestwood wrote:
> The same pbkdf2 algorithm but uses a key ID as the password.
> ---
>   ell/cert-crypto.c | 27 +++++++++++++++++++++++++++
>   ell/cert.h        |  6 +++++-
>   ell/ell.sym       |  1 +
>   3 files changed, 33 insertions(+), 1 deletion(-)
> 

<snip>

> diff --git a/ell/cert.h b/ell/cert.h
> index f637588..ce430fa 100644
> --- a/ell/cert.h
> +++ b/ell/cert.h
> @@ -76,7 +76,11 @@ bool l_cert_pkcs5_pbkdf2(enum l_checksum_type type, const char *password,
>   				const uint8_t *salt, size_t salt_len,
>   				unsigned int iter_count,
>   				uint8_t *out_dk, size_t dk_len);
> -
> +bool l_cert_pkcs5_pbkdf2_from_key_id(enum l_checksum_type type,

missing LIB_EXPORT?

> +					int32_t key_id, const uint8_t *salt,
> +					size_t salt_len,
> +					unsigned int iter_count,
> +					uint8_t *out_dk, size_t dk_len);

So personally I'd rather have l_cert_pkcs5_pbkdf2 take a struct l_checksum * as 
the first parameter instead of creating two almost identical constructors. 
Especially since we already would have a special l_checksum_hmac_* constructor 
that takes a key id.

Regards,
-Denis

Re: [RFC 1/8] key: add l_key_search

[Denis Kenzior]

Hi James,

>>> +LIB_EXPORT int32_t l_key_search(enum l_key_type type, const char
>>> *keyring,
>>
>> How likely are we to search some custom keyring?  Wouldn't we
>> generally be
>> searching a user/default user session keyrings?
> 
> I was just leaning on the side of flexibility. I don't really care
> either way but figured an extra argument was fine even if we end up
> calling it with "user".

So this function would perform some sort of !strcmp conversion between "user" 
and KEY_SPEC_USER_KEYRING?  That's fine I suppose.

Alternatively you can ignore that for now and if you really need to search 
keyrings that are not "standard", then one can introduce l_keyring_search() or 
something later.

Regards,
-Denis

Re: [RFC 1/8] key: add l_key_search

[James Prestwood]

On Tue, 2022-11-22 at 10:43 -0600, Denis Kenzior wrote:
> Hi James,
> 
> On 11/18/22 15:16, James Prestwood wrote:
> > Search for a key by type, keyring name, and description. Returns
> > the
> > key ID or an error if not found.
> > ---
> >   ell/ell.sym |  1 +
> >   ell/key.c   | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> >   ell/key.h   |  3 +++
> >   3 files changed, 50 insertions(+)
> > 
> 
> <snip>
> 
> > @@ -283,6 +303,32 @@ static bool setup_internal_keyring(void)
> >         return true;
> >   }
> >   
> > +LIB_EXPORT int32_t l_key_search(enum l_key_type type, const char
> > *keyring,
> 
> How likely are we to search some custom keyring?  Wouldn't we
> generally be 
> searching a user/default user session keyrings?

I was just leaning on the side of flexibility. I don't really care
either way but figured an extra argument was fine even if we end up
calling it with "user".

> 
> > +                                       const char *description)
> > +{
> > +       long keyring_id;
> > +       long key_id;
> > +
> > +       if (unlikely((size_t)type >= L_ARRAY_SIZE(key_type_names)))
> > +               return -EINVAL;
> > +
> > +       if (unlikely(!keyring || !description))
> > +               return -EINVAL;
> > +
> > +       /* Find the ID of the keyring */
> > +       keyring_id = kernel_key_request("keyring", keyring);
> > +       if (keyring_id < 0)
> > +               return -ENOENT;
> > +
> > +       /* Search for the key by type/description */
> > +       key_id = kernel_key_search(keyring_id,
> > key_type_names[type],
> > +                                       description);
> > +       if (key_id < 0)
> > +               return -ENOENT;
> > +
> > +       return key_id;
> > +}
> > +
> >   LIB_EXPORT struct l_key *l_key_new(enum l_key_type type, const
> > void *payload,
> >                                         size_t payload_length)
> >   {
> 
> Regards,
> -Denis

Re: [RFC 1/8] key: add l_key_search

[James Prestwood]

On Tue, 2022-11-22 at 11:09 -0600, Denis Kenzior wrote:
> Hi James,
> 
> > > > +LIB_EXPORT int32_t l_key_search(enum l_key_type type, const
> > > > char
> > > > *keyring,
> > > 
> > > How likely are we to search some custom keyring?  Wouldn't we
> > > generally be
> > > searching a user/default user session keyrings?
> > 
> > I was just leaning on the side of flexibility. I don't really care
> > either way but figured an extra argument was fine even if we end up
> > calling it with "user".
> 
> So this function would perform some sort of !strcmp conversion
> between "user" 
> and KEY_SPEC_USER_KEYRING?  That's fine I suppose.

Ah, I kinda overlooked the fact the user keyring is actually named
something like "_uid.foo". And in theory "user" could be an actual
custom keyring... So defaulting to KEYS_SPEC_USER_KEYRING seems like
the most logical option, and if we need custom keyrings we could add
l_keyring_search().

> 
> Regards,
> -Denis