diff mbox series

[01/21] crypto: remove label from prf_plus, instead use va_args

Message ID 20231012200150.338401-2-prestwoj@gmail.com (mailing list archive)
State Accepted, archived
Headers show
Series DPP PKEX Changes | expand

Checks

Context Check Description
tedd_an/pre-ci_am fail error: patch failed: src/crypto.c:624 error: src/crypto.c: patch does not apply error: patch failed: src/crypto.h:122 error: src/crypto.h: patch does not apply error: patch failed: src/erp.c:281 error: src/erp.c: patch does not apply hint: Use 'git am --show-current-patch' to see the failed patch

Commit Message

James Prestwood Oct. 12, 2023, 8:01 p.m. UTC 
The prf_plus API was a bit restrictive because it only took a
string label which isn't compatible with some specs (e.g. DPP
inputs to HKDF-Expand). In addition it took additional label
aruments which were appended to the HMAC call (and the
non-intuitive '\0' if there were extra arguments).

Instead the label argument has been removed and callers can pass
it in through va_args. This also lets the caller decided the length
and can include the '\0' or not, dependent on the spec the caller
is following.
---
 src/crypto.c | 24 +++++++++---------------
 src/crypto.h |  2 +-
 src/erp.c    | 19 +++++++++++--------
 3 files changed, 21 insertions(+), 24 deletions(-)

Comments

Denis Kenzior Oct. 17, 2023, 3:18 p.m. UTC | #1 
Hi James,

On 10/12/23 15:01, James Prestwood wrote:
> The prf_plus API was a bit restrictive because it only took a
> string label which isn't compatible with some specs (e.g. DPP
> inputs to HKDF-Expand). In addition it took additional label
> aruments which were appended to the HMAC call (and the
> non-intuitive '\0' if there were extra arguments).
> 
> Instead the label argument has been removed and callers can pass
> it in through va_args. This also lets the caller decided the length
> and can include the '\0' or not, dependent on the spec the caller
> is following.
> ---
>   src/crypto.c | 24 +++++++++---------------
>   src/crypto.h |  2 +-
>   src/erp.c    | 19 +++++++++++--------
>   3 files changed, 21 insertions(+), 24 deletions(-)
> 

Applied, thanks.

Regards,
-Denis
diff mbox series

Patch

diff --git a/src/crypto.c b/src/crypto.c
index 710641ed..3128b2a5 100644
--- a/src/crypto.c
+++ b/src/crypto.c
@@ -624,10 +624,10 @@  bool prf_sha1(const void *key, size_t key_len,
 
 /* PRF+ from RFC 5295 Section 3.1.2 (also RFC 4306 Section 2.13) */
 bool prf_plus(enum l_checksum_type type, const void *key, size_t key_len,
-		const char *label, void *out, size_t out_len,
+		void *out, size_t out_len,
 		size_t n_extra, ...)
 {
-	struct iovec iov[n_extra + 3];
+	struct iovec iov[n_extra + 2];
 	uint8_t *t = out;
 	size_t t_len = 0;
 	uint8_t count = 1;
@@ -637,24 +637,17 @@  bool prf_plus(enum l_checksum_type type, const void *key, size_t key_len,
 	ssize_t ret;
 	size_t i;
 
-	iov[1].iov_base = (void *) label;
-	iov[1].iov_len = strlen(label);
-
-	/* Include the '\0' from the label in S if extra arguments provided */
-	if (n_extra)
-		iov[1].iov_len += 1;
-
 	va_start(va, n_extra);
 
 	for (i = 0; i < n_extra; i++) {
-		iov[i + 2].iov_base = va_arg(va, void *);
-		iov[i + 2].iov_len = va_arg(va, size_t);
+		iov[i + 1].iov_base = va_arg(va, void *);
+		iov[i + 1].iov_len = va_arg(va, size_t);
 	}
 
 	va_end(va);
 
-	iov[n_extra + 2].iov_base = &count;
-	iov[n_extra + 2].iov_len = 1;
+	iov[n_extra + 1].iov_base = &count;
+	iov[n_extra + 1].iov_len = 1;
 
 	hmac = l_checksum_new_hmac(type, key, key_len);
 	if (!hmac)
@@ -664,7 +657,7 @@  bool prf_plus(enum l_checksum_type type, const void *key, size_t key_len,
 		iov[0].iov_base = t;
 		iov[0].iov_len = t_len;
 
-		if (!l_checksum_updatev(hmac, iov, n_extra + 3)) {
+		if (!l_checksum_updatev(hmac, iov, n_extra + 2)) {
 			l_checksum_free(hmac);
 			return false;
 		}
@@ -874,7 +867,8 @@  bool hkdf_extract(enum l_checksum_type type, const void *key,
 bool hkdf_expand(enum l_checksum_type type, const void *key, size_t key_len,
 			const char *info, void *out, size_t out_len)
 {
-	return prf_plus(type, key, key_len, info, out, out_len, 0);
+	return prf_plus(type, key, key_len, out, out_len, 1,
+			info, strlen(info));
 }
 
 /*
diff --git a/src/crypto.h b/src/crypto.h
index d2a96655..1f48a52b 100644
--- a/src/crypto.h
+++ b/src/crypto.h
@@ -122,7 +122,7 @@  bool prf_plus_sha1(const void *key, size_t key_len,
 		const void *data, size_t data_len, void *output, size_t size);
 
 bool prf_plus(enum l_checksum_type type, const void *key, size_t key_len,
-		const char *label, void *out, size_t out_len,
+		void *out, size_t out_len,
 		size_t n_extra, ...);
 
 bool hkdf_extract(enum l_checksum_type type, const void *key, size_t key_len,
diff --git a/src/erp.c b/src/erp.c
index 5af18fda..2729cfc8 100644
--- a/src/erp.c
+++ b/src/erp.c
@@ -281,8 +281,9 @@  static bool erp_derive_emsk_name(const uint8_t *session_id, size_t session_len,
 	uint16_t eight = L_CPU_TO_BE16(8);
 	char *ascii;
 
-	if (!prf_plus(L_CHECKSUM_SHA256, session_id, session_len, "EMSK",
-				hex, 8, 1, &eight, sizeof(eight)))
+	if (!prf_plus(L_CHECKSUM_SHA256, session_id, session_len,
+				hex, 8, 2, "EMSK", strlen("EMSK") + 1,
+				&eight, sizeof(eight)))
 		return false;
 
 	ascii = l_util_hexstring(hex, 8);
@@ -309,13 +310,15 @@  static bool erp_derive_reauth_keys(const uint8_t *emsk, size_t emsk_len,
 	uint16_t len = L_CPU_TO_BE16(emsk_len);
 	uint8_t cryptosuite = ERP_CRYPTOSUITE_SHA256_128;
 
-	if (!prf_plus(L_CHECKSUM_SHA256, emsk, emsk_len, ERP_RRK_LABEL,
-				r_rk, emsk_len, 1,
+	if (!prf_plus(L_CHECKSUM_SHA256, emsk, emsk_len,
+				r_rk, emsk_len, 2, ERP_RRK_LABEL,
+				strlen(ERP_RRK_LABEL) + 1,
 				&len, sizeof(len)))
 		return false;
 
-	if (!prf_plus(L_CHECKSUM_SHA256, r_rk, emsk_len, ERP_RIK_LABEL,
-				r_ik, emsk_len, 2,
+	if (!prf_plus(L_CHECKSUM_SHA256, r_rk, emsk_len,
+				r_ik, emsk_len, 3, ERP_RIK_LABEL,
+				strlen(ERP_RIK_LABEL) + 1,
 				&cryptosuite, 1, &len, sizeof(len)))
 		return false;
 
@@ -496,8 +499,8 @@  int erp_rx_packet(struct erp_state *erp, const uint8_t *pkt, size_t len)
 	length = L_CPU_TO_BE16(64);
 
 	if (!prf_plus(L_CHECKSUM_SHA256, erp->r_rk, erp->cache->emsk_len,
-				ERP_RMSK_LABEL,
-				erp->rmsk, erp->cache->emsk_len, 2,
+				erp->rmsk, erp->cache->emsk_len, 3,
+				ERP_RMSK_LABEL, strlen(ERP_RMSK_LABEL) + 1,
 				&seq, sizeof(seq),
 				&length, sizeof(length)))
 		goto eap_failed;