xfrm: allow avoiding buffer overflow for key in xfrmnl_sa_get_*_params()

The previous API of xfrmnl_sa_get_*_params() would always require
a @key buffer, but it was not possible to avoid buffer overflow
because the required size was unknown.

That is not really fixable, because the old API is broken.

Now, allow omitting the @key argument to only request the @key_size.
That allows the caller to ask beforehand how large the @key buffer
must be: ((@key_size + 7) / 8).

Unfortunately, omitting the key against previous versions of libnl
leads to a crash. And passing a key against older versions makes it
impossible to avoid buffer-overflow.

Another option would be to add functions like
xfrmnl_sa_get_crypto_params_keylen() so the user can query the required
buffer size by calling that instead of xfrmnl_sa_get_crypto_params().
However, then the user also requires a backport of the new API
and this will not be possible against older libnl3 versions either.
Thus, if the user already requires the fix, he can just as well
require a backport of this patch and then safely call xfrmnl_sa_get_crypto_params()
without @key argument. This way has the advantage/disadvantage, that
it can detect the presence of the patch at runtime.

The cumbersome way to get it right would be:

    unsiged key_len;
    char *key;
    int r;

    if (!nl_has_capability(17 /*NL_CAPABILITY_XFRM_SA_KEY_SIZE*/)) {
        /* no way to use this API safely. Abort. */
        return -NLE_OPNOTSUPP;
    }

    r = xfrmnl_sa_get_crypto_params(sa, NULL, &key_len, NULL);
    if (r < 0)
         return r;
    key = malloc((key_len + 7) / 8);
    if (!key)
         return -NLE_NOMEM;
    r = xfrmnl_sa_get_crypto_params(sa, NULL, &key_len, &key);
    if (r < 0) {
        free(key);
        return r;
    }
    ...

http://lists.infradead.org/pipermail/libnl/2016-June/002155.html

Signed-off-by: Thomas Haller <thaller@redhat.com>

3 files changed, 98 insertions, 14 deletions

diff --git a/include/netlink/utils.h b/include/netlink/utils.h
index ebf12cf..43c9147 100644
--- a/include/netlink/utils.h
+++ b/include/netlink/utils.h
@@ -197,6 +197,13 @@ enum {
 	NL_CAPABILITY_NL_OBJECT_DIFF64 = 16,
 #define NL_CAPABILITY_NL_OBJECT_DIFF64 NL_CAPABILITY_NL_OBJECT_DIFF64
 
+	/**
+	 * Support omitting @key argument to xfrmnl_sa_get_*_params() to check
+	 * for required buffer size for key.
+	 */
+	NL_CAPABILITY_XFRM_SA_KEY_SIZE = 17,
+#define NL_CAPABILITY_XFRM_SA_KEY_SIZE NL_CAPABILITY_XFRM_SA_KEY_SIZE
+
 	__NL_CAPABILITY_MAX,
 	NL_CAPABILITY_MAX = (__NL_CAPABILITY_MAX - 1),
 #define NL_CAPABILITY_MAX NL_CAPABILITY_MAX
diff --git a/lib/utils.c b/lib/utils.c
index 85beb86..0c91c97 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -1160,6 +1160,15 @@ int nl_has_capability (int capability)
 			NL_CAPABILITY_RTNL_LINK_VLAN_INGRESS_MAP_CLEAR,
 			NL_CAPABILITY_RTNL_LINK_VXLAN_IO_COMPARE,
 			NL_CAPABILITY_NL_OBJECT_DIFF64),
+		_NL_SET (2,
+			NL_CAPABILITY_XFRM_SA_KEY_SIZE,
+			0,
+			0,
+			0,
+			0,
+			0,
+			0,
+			0),
 		/* IMPORTANT: these capability numbers are intended to be universal and stable
 		 * for libnl3. Don't allocate new numbers on your own that differ from upstream
 		 * libnl3.
diff --git a/lib/xfrm/sa.c b/lib/xfrm/sa.c
index a1abfe7..1c0888b 100644
--- a/lib/xfrm/sa.c
+++ b/lib/xfrm/sa.c
@@ -1622,14 +1622,32 @@ int xfrmnl_sa_set_flags (struct xfrmnl_sa* sa, unsigned int flags)
 	return 0;
 }
 
+/**
+ * Get the aead-params
+ * @arg sa              the xfrmnl_sa object
+ * @arg alg_name        an optional output buffer for the algorithm name. Must be at least 64 bytes.
+ * @arg key_len         an optional output value for the key length in bits.
+ * @arg icv_len         an optional output value for the alt-icv-len.
+ * @arg key             an optional buffer large enough for the key. It must contain at least
+ *                      ((@key_len + 7) / 8) bytes.
+ *
+ * Warning: you must ensure that @key is large enough. If you don't know the key_len before-hand,
+ * call xfrmnl_sa_get_aead_params() without @key argument to query only the required buffer size.
+ *
+ * @return 0 on success or a negative error code.
+ */
 int xfrmnl_sa_get_aead_params (struct xfrmnl_sa* sa, char* alg_name, unsigned int* key_len, unsigned int* icv_len, char* key)
 {
 	if (sa->ce_mask & XFRM_SA_ATTR_ALG_AEAD)
 	{
-		strcpy (alg_name, sa->aead->alg_name);
-		*key_len    =   sa->aead->alg_key_len;
-		*icv_len    =   sa->aead->alg_icv_len;
-		memcpy ((void *)key, (void *)sa->aead->alg_key, ((sa->aead->alg_key_len + 7)/8));
+		if (alg_name)
+			strcpy (alg_name, sa->aead->alg_name);
+		if (key_len)
+			*key_len = sa->aead->alg_key_len;
+		if (icv_len)
+			*icv_len = sa->aead->alg_icv_len;
+		if (key)
+			memcpy (key, sa->aead->alg_key, ((sa->aead->alg_key_len + 7)/8));
 	}
 	else
 		return -1;
@@ -1659,14 +1677,32 @@ int xfrmnl_sa_set_aead_params (struct xfrmnl_sa* sa, const char* alg_name, unsig
 	return 0;
 }
 
+/**
+ * Get the auth-params
+ * @arg sa              the xfrmnl_sa object
+ * @arg alg_name        an optional output buffer for the algorithm name. Must be at least 64 bytes.
+ * @arg key_len         an optional output value for the key length in bits.
+ * @arg trunc_len       an optional output value for the alg-trunc-len.
+ * @arg key             an optional buffer large enough for the key. It must contain at least
+ *                      ((@key_len + 7) / 8) bytes.
+ *
+ * Warning: you must ensure that @key is large enough. If you don't know the key_len before-hand,
+ * call xfrmnl_sa_get_auth_params() without @key argument to query only the required buffer size.
+ *
+ * @return 0 on success or a negative error code.
+ */
 int xfrmnl_sa_get_auth_params (struct xfrmnl_sa* sa, char* alg_name, unsigned int* key_len, unsigned int* trunc_len, char* key)
 {
 	if (sa->ce_mask & XFRM_SA_ATTR_ALG_AUTH)
 	{
-		strcpy (alg_name, sa->auth->alg_name);
-		*key_len    =   sa->auth->alg_key_len;
-		*trunc_len  =   sa->auth->alg_trunc_len;
-		memcpy ((void *)key, (void *)sa->auth->alg_key, ((sa->auth->alg_key_len + 7)/8));
+		if (alg_name)
+			strcpy (alg_name, sa->auth->alg_name);
+		if (key_len)
+			*key_len = sa->auth->alg_key_len;
+		if (trunc_len)
+			*trunc_len = sa->auth->alg_trunc_len;
+		if (key)
+			memcpy (key, sa->auth->alg_key, (sa->auth->alg_key_len + 7)/8);
 	}
 	else
 		return -1;
@@ -1696,13 +1732,29 @@ int xfrmnl_sa_set_auth_params (struct xfrmnl_sa* sa, const char* alg_name, unsig
 	return 0;
 }
 
+/**
+ * Get the crypto-params
+ * @arg sa              the xfrmnl_sa object
+ * @arg alg_name        an optional output buffer for the algorithm name. Must be at least 64 bytes.
+ * @arg key_len         an optional output value for the key length in bits.
+ * @arg key             an optional buffer large enough for the key. It must contain at least
+ *                      ((@key_len + 7) / 8) bytes.
+ *
+ * Warning: you must ensure that @key is large enough. If you don't know the key_len before-hand,
+ * call xfrmnl_sa_get_crypto_params() without @key argument to query only the required buffer size.
+ *
+ * @return 0 on success or a negative error code.
+ */
 int xfrmnl_sa_get_crypto_params (struct xfrmnl_sa* sa, char* alg_name, unsigned int* key_len, char* key)
 {
 	if (sa->ce_mask & XFRM_SA_ATTR_ALG_CRYPT)
 	{
-		strcpy (alg_name, sa->crypt->alg_name);
-		*key_len    =   sa->crypt->alg_key_len;
-		memcpy ((void *)key, (void *)sa->crypt->alg_key, ((sa->crypt->alg_key_len + 7)/8));
+		if (alg_name)
+			strcpy (alg_name, sa->crypt->alg_name);
+		if (key_len)
+			*key_len = sa->crypt->alg_key_len;
+		if (key)
+			memcpy (key, sa->crypt->alg_key, ((sa->crypt->alg_key_len + 7)/8));
 	}
 	else
 		return -1;
@@ -1731,13 +1783,29 @@ int xfrmnl_sa_set_crypto_params (struct xfrmnl_sa* sa, const char* alg_name, uns
 	return 0;
 }
 
+/**
+ * Get the comp-params
+ * @arg sa              the xfrmnl_sa object
+ * @arg alg_name        an optional output buffer for the algorithm name. Must be at least 64 bytes.
+ * @arg key_len         an optional output value for the key length in bits.
+ * @arg key             an optional buffer large enough for the key. It must contain at least
+ *                      ((@key_len + 7) / 8) bytes.
+ *
+ * Warning: you must ensure that @key is large enough. If you don't know the key_len before-hand,
+ * call xfrmnl_sa_get_comp_params() without @key argument to query only the required buffer size.
+ *
+ * @return 0 on success or a negative error code.
+ */
 int xfrmnl_sa_get_comp_params (struct xfrmnl_sa* sa, char* alg_name, unsigned int* key_len, char* key)
 {
 	if (sa->ce_mask & XFRM_SA_ATTR_ALG_COMP)
 	{
-		strcpy (alg_name, sa->comp->alg_name);
-		*key_len    =   sa->comp->alg_key_len;
-		memcpy ((void *)key, (void *)sa->comp->alg_key, ((sa->comp->alg_key_len + 7)/8));
+		if (alg_name)
+			strcpy (alg_name, sa->comp->alg_name);
+		if (key_len)
+			*key_len = sa->comp->alg_key_len;
+		if (key)
+			memcpy (key, sa->comp->alg_key, ((sa->comp->alg_key_len + 7)/8));
 	}
 	else
 		return -1;