[PATCH] KEYS: Add optional key derivation parameters for DH

[PATCH] KEYS: Add optional key derivation parameters for DH

[Mat Martineau]

The values computed during Diffie-Hellman key exchange are often used
in combination with key derivation functions to create cryptographic
keys. Add an interface to configure a key derivation function that
will transform the Diffie-Hellman result returned by the
KEYCTL_DH_COMPUTE command.

Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
---
 Documentation/security/keys.txt | 9 ++++++++-
 include/uapi/linux/keyctl.h     | 5 +++++
 security/keys/compat.c          | 2 +-
 security/keys/dh.c              | 8 +++++++-
 security/keys/internal.h        | 5 +++--
 security/keys/keyctl.c          | 4 ++--
 6 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/Documentation/security/keys.txt b/Documentation/security/keys.txt
index 20d0571..47470a6 100644
--- a/Documentation/security/keys.txt
+++ b/Documentation/security/keys.txt
@@ -826,7 +826,8 @@ The keyctl syscall functions are:
  (*) Compute a Diffie-Hellman shared secret or public key
 
        long keyctl(KEYCTL_DH_COMPUTE, struct keyctl_dh_params *params,
-		   char *buffer, size_t buflen);
+		   char *buffer, size_t buflen,
+		   struct keyctl_kdf_params *kdf);
 
      The params struct contains serial numbers for three keys:
 
@@ -843,6 +844,12 @@ The keyctl syscall functions are:
      public key.  If the base is the remote public key, the result is
      the shared secret.
 
+     When kdf is NULL, the raw computed value is written to the
+     buffer. If valid kdf parameters are provided, the result of the
+     DH computation is transformed according to the requested key
+     derivation function and the transformed value is written to the
+     buffer.
+
      The buffer length must be at least the length of the prime, or zero.
 
      If the buffer length is nonzero, the length of the result is
diff --git a/include/uapi/linux/keyctl.h b/include/uapi/linux/keyctl.h
index 86eddd6..d55cf53 100644
--- a/include/uapi/linux/keyctl.h
+++ b/include/uapi/linux/keyctl.h
@@ -68,4 +68,9 @@ struct keyctl_dh_params {
 	__s32 base;
 };
 
+struct keyctl_kdf_params {
+	char *name;
+	__u8 reserved[32]; /* Reserved for future use, must be 0 */
+};
+
 #endif /*  _LINUX_KEYCTL_H */
diff --git a/security/keys/compat.c b/security/keys/compat.c
index c8783b3..36c80bf 100644
--- a/security/keys/compat.c
+++ b/security/keys/compat.c
@@ -134,7 +134,7 @@ COMPAT_SYSCALL_DEFINE5(keyctl, u32, option,
 
 	case KEYCTL_DH_COMPUTE:
 		return keyctl_dh_compute(compat_ptr(arg2), compat_ptr(arg3),
-					 arg4);
+					 arg4, compat_ptr(arg5));
 
 	default:
 		return -EOPNOTSUPP;
diff --git a/security/keys/dh.c b/security/keys/dh.c
index 880505a..e415d4c 100644
--- a/security/keys/dh.c
+++ b/security/keys/dh.c
@@ -78,7 +78,8 @@ error:
 }
 
 long keyctl_dh_compute(struct keyctl_dh_params __user *params,
-		       char __user *buffer, size_t buflen)
+		       char __user *buffer, size_t buflen,
+		       struct keyctl_kdf_params __user *kdf)
 {
 	long ret;
 	MPI base, private, prime, result;
@@ -97,6 +98,11 @@ long keyctl_dh_compute(struct keyctl_dh_params __user *params,
 		goto out;
 	}
 
+	if (kdf) {
+		ret = -EINVAL;
+		goto out;
+	}
+
 	keylen = mpi_from_key(pcopy.prime, buflen, &prime);
 	if (keylen < 0 || !prime) {
 		/* buflen == 0 may be used to query the required buffer size,
diff --git a/security/keys/internal.h b/security/keys/internal.h
index 8ec7a52..1b7aaaa22 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -260,10 +260,11 @@ static inline long keyctl_get_persistent(uid_t uid, key_serial_t destring)
 
 #ifdef CONFIG_KEY_DH_OPERATIONS
 extern long keyctl_dh_compute(struct keyctl_dh_params __user *, char __user *,
-			      size_t);
+			      size_t, struct keyctl_kdf_params __user *);
 #else
 static inline long keyctl_dh_compute(struct keyctl_dh_params __user *params,
-				     char __user *buffer, size_t buflen)
+				     char __user *buffer, size_t buflen,
+				     struct keyctl_kdf_params __user *kdf)
 {
 	return -EOPNOTSUPP;
 }
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 3b135a0..b106898 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -1688,8 +1688,8 @@ SYSCALL_DEFINE5(keyctl, int, option, unsigned long, arg2, unsigned long, arg3,
 
 	case KEYCTL_DH_COMPUTE:
 		return keyctl_dh_compute((struct keyctl_dh_params __user *) arg2,
-					 (char __user *) arg3,
-					 (size_t) arg4);
+					 (char __user *) arg3, (size_t) arg4,
+					 (struct keyctl_kdf_params __user *) arg5);
 
 	default:
 		return -EOPNOTSUPP;
-- 
2.8.3

Re: [PATCH] KEYS: Add optional key derivation parameters for DH

[David Howells]

Mat Martineau <mathew.j.martineau@linux.intel.com> wrote:

> +struct keyctl_kdf_params {
> +	char *name;
> +	__u8 reserved[32]; /* Reserved for future use, must be 0 */
> +};
> +
>  #endif /*  _LINUX_KEYCTL_H */
> diff --git a/security/keys/compat.c b/security/keys/compat.c
> index c8783b3..36c80bf 100644
> --- a/security/keys/compat.c
> +++ b/security/keys/compat.c
> @@ -134,7 +134,7 @@ COMPAT_SYSCALL_DEFINE5(keyctl, u32, option,
>  
>  	case KEYCTL_DH_COMPUTE:
>  		return keyctl_dh_compute(compat_ptr(arg2), compat_ptr(arg3),
> -					 arg4);
> +					 arg4, compat_ptr(arg5));

Given the new structure above, this won't work.  The problem is that on a
64-bit system the kernel expects 'name' to be a 64-bit pointer, but if we're
in the compat handler, we have a 32-bit userspace's idea of the struct - in
which 'name' is a 31-bit (s390x) or a 32-bit pointer without any padding.

So in compat code you can't just pass the user pointer direct through to
keyctl_dh_compute().  You need to supply a compat_keyctl_kdf_params struct and
translator code.

What I would recommend you do at the moment is to mark the syscall argument as
"reserved, must be 0" and deal with the implementation in the next merge
window.

David

[PATCH v2] KEYS: Add placeholder KDF usage with DH

[Stephan Mueller]

Am Donnerstag, 26. Mai 2016, 21:10:50 schrieb David Howells:

Hi David,

Mat asked me to step in in case his patch needed an update.

---8<---
The values computed during Diffie-Hellman key exchange are often used
in combination with key derivation functions to create cryptographic
keys. Add a placeholder for a later implementation to configure a
key derivation function that will transform the Diffie-Hellman
result returned by the KEYCTL_DH_COMPUTE command.

Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 Documentation/security/keys.txt | 5 ++++-
 security/keys/compat.c          | 2 +-
 security/keys/dh.c              | 8 +++++++-
 security/keys/internal.h        | 5 +++--
 security/keys/keyctl.c          | 4 ++--
 5 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/Documentation/security/keys.txt b/Documentation/security/keys.txt
index 20d0571..3849814 100644
--- a/Documentation/security/keys.txt
+++ b/Documentation/security/keys.txt
@@ -826,7 +826,8 @@ The keyctl syscall functions are:
  (*) Compute a Diffie-Hellman shared secret or public key
 
        long keyctl(KEYCTL_DH_COMPUTE, struct keyctl_dh_params *params,
-		   char *buffer, size_t buflen);
+		   char *buffer, size_t buflen,
+		   void *reserved);
 
      The params struct contains serial numbers for three keys:
 
@@ -843,6 +844,8 @@ The keyctl syscall functions are:
      public key.  If the base is the remote public key, the result is
      the shared secret.
 
+     The reserved argument must be set to NULL.
+
      The buffer length must be at least the length of the prime, or zero.
 
      If the buffer length is nonzero, the length of the result is
diff --git a/security/keys/compat.c b/security/keys/compat.c
index c8783b3..36c80bf 100644
--- a/security/keys/compat.c
+++ b/security/keys/compat.c
@@ -134,7 +134,7 @@ COMPAT_SYSCALL_DEFINE5(keyctl, u32, option,
 
 	case KEYCTL_DH_COMPUTE:
 		return keyctl_dh_compute(compat_ptr(arg2), compat_ptr(arg3),
-					 arg4);
+					 arg4, compat_ptr(arg5));
 
 	default:
 		return -EOPNOTSUPP;
diff --git a/security/keys/dh.c b/security/keys/dh.c
index 880505a..531ed2e 100644
--- a/security/keys/dh.c
+++ b/security/keys/dh.c
@@ -78,7 +78,8 @@ error:
 }
 
 long keyctl_dh_compute(struct keyctl_dh_params __user *params,
-		       char __user *buffer, size_t buflen)
+		       char __user *buffer, size_t buflen,
+		       void __user *reserved)
 {
 	long ret;
 	MPI base, private, prime, result;
@@ -97,6 +98,11 @@ long keyctl_dh_compute(struct keyctl_dh_params __user *params,
 		goto out;
 	}
 
+	if (reserved) {
+		ret = -EINVAL;
+		goto out;
+	}
+
 	keylen = mpi_from_key(pcopy.prime, buflen, &prime);
 	if (keylen < 0 || !prime) {
 		/* buflen == 0 may be used to query the required buffer size,
diff --git a/security/keys/internal.h b/security/keys/internal.h
index 8ec7a52..a705a7d 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -260,10 +260,11 @@ static inline long keyctl_get_persistent(uid_t uid, key_serial_t destring)
 
 #ifdef CONFIG_KEY_DH_OPERATIONS
 extern long keyctl_dh_compute(struct keyctl_dh_params __user *, char __user *,
-			      size_t);
+			      size_t, void __user *);
 #else
 static inline long keyctl_dh_compute(struct keyctl_dh_params __user *params,
-				     char __user *buffer, size_t buflen)
+				     char __user *buffer, size_t buflen,
+				     void __user *reserved)
 {
 	return -EOPNOTSUPP;
 }
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 3b135a0..d580ad0 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -1688,8 +1688,8 @@ SYSCALL_DEFINE5(keyctl, int, option, unsigned long, arg2, unsigned long, arg3,
 
 	case KEYCTL_DH_COMPUTE:
 		return keyctl_dh_compute((struct keyctl_dh_params __user *) arg2,
-					 (char __user *) arg3,
-					 (size_t) arg4);
+					 (char __user *) arg3, (size_t) arg4,
+					 (void __user *) arg5);
 
 	default:
 		return -EOPNOTSUPP;
-- 
2.5.5

Re: [PATCH] KEYS: Add optional key derivation parameters for DH

[Mat Martineau]

On Thu, 26 May 2016, David Howells wrote:

> Mat Martineau <mathew.j.martineau@linux.intel.com> wrote:
>
>> +struct keyctl_kdf_params {
>> +	char *name;
>> +	__u8 reserved[32]; /* Reserved for future use, must be 0 */
>> +};
>> +
>>  #endif /*  _LINUX_KEYCTL_H */
>> diff --git a/security/keys/compat.c b/security/keys/compat.c
>> index c8783b3..36c80bf 100644
>> --- a/security/keys/compat.c
>> +++ b/security/keys/compat.c
>> @@ -134,7 +134,7 @@ COMPAT_SYSCALL_DEFINE5(keyctl, u32, option,
>>
>>  	case KEYCTL_DH_COMPUTE:
>>  		return keyctl_dh_compute(compat_ptr(arg2), compat_ptr(arg3),
>> -					 arg4);
>> +					 arg4, compat_ptr(arg5));
>
> Given the new structure above, this won't work.  The problem is that on a
> 64-bit system the kernel expects 'name' to be a 64-bit pointer, but if we're
> in the compat handler, we have a 32-bit userspace's idea of the struct - in
> which 'name' is a 31-bit (s390x) or a 32-bit pointer without any padding.
>
> So in compat code you can't just pass the user pointer direct through to
> keyctl_dh_compute().  You need to supply a compat_keyctl_kdf_params struct and
> translator code.

Since none of the members of the structure were accessed, I thought the 
simple conversion was adequate for the null check and was deferring the 
real compat handling until the rest of the structure was known. I should 
have explained that in a comment.

> What I would recommend you do at the moment is to mark the syscall argument as
> "reserved, must be 0" and deal with the implementation in the next merge
> window.

Yeah, there's not much value in defining the keyctl_kdf_params struct and 
then not using it. Should have kept it simple.

Thanks to you and Stephan for updating the patch and moving things along.


Regards,

--
Mat Martineau
Intel OTC