diff mbox

[5/8] KEYS: Provide software public key query function [ver #2]

Message ID 146668969255.2977.2158699976750892093.stgit@warthog.procyon.org.uk (mailing list archive)
State New, archived
Headers show

Commit Message

David Howells June 23, 2016, 1:48 p.m. UTC
Provide a query function for the software public key implementation.  This
permits information about such a key to be obtained using
query_asymmetric_key() or KEYCTL_PKEY_QUERY.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 crypto/asymmetric_keys/public_key.c |   96 ++++++++++++++++++++++++++++++-----
 1 file changed, 82 insertions(+), 14 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Mat Martineau June 23, 2016, 3:46 p.m. UTC | #1
David,

On Thu, 23 Jun 2016, David Howells wrote:

> Provide a query function for the software public key implementation.  This
> permits information about such a key to be obtained using
> query_asymmetric_key() or KEYCTL_PKEY_QUERY.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
>
> crypto/asymmetric_keys/public_key.c |   96 ++++++++++++++++++++++++++++++-----
> 1 file changed, 82 insertions(+), 14 deletions(-)
>
> diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
> index fd76b5fc3b3a..a48a47a1dff0 100644
> --- a/crypto/asymmetric_keys/public_key.c
> +++ b/crypto/asymmetric_keys/public_key.c
> @@ -57,6 +57,81 @@ static void public_key_destroy(void *payload0, void *payload3)
> 	public_key_signature_free(payload3);
> }
>
> +/*
> + * Determine the crypto algorithm name.
> + */
> +static
> +int software_key_determine_akcipher(const char *encoding,
> +				    const char *hash_algo,
> +				    const struct public_key *pkey,
> +				    char alg_name[CRYPTO_MAX_ALG_NAME])
> +{
> +	int n;
> +
> +	if (strcmp(encoding, "pkcs1") == 0) {
> +		/* The data wangled by the RSA algorithm is typically padded
> +		 * and encoded in some manner, such as EMSA-PKCS1-1_5 [RFC3447
> +		 * sec 8.2].
> +		 */
> +		if (!hash_algo)
> +			n = snprintf(alg_name, CRYPTO_MAX_ALG_NAME,
> +				     "pkcs1pad(%s)",
> +				     pkey->pkey_algo);

Did you see Herbert's patch that strips out non-hash pkcs1pad capabilities 
(and the ensuing discussion)?

http://www.spinics.net/lists/linux-crypto/index.html#20432

I'm making use of pkcs1pad(rsa) with a TLS implementation, so it's good to 
see it supported here.

> +		else
> +			n = snprintf(alg_name, CRYPTO_MAX_ALG_NAME,
> +				     "pkcs1pad(%s,%s)",
> +				     pkey->pkey_algo, hash_algo);
> +		return n >= CRYPTO_MAX_ALG_NAME ? -EINVAL : 0;
> +	}
> +
> +	if (strcmp(encoding, "raw") == 0) {
> +		strcpy(alg_name, pkey->pkey_algo);
> +		return 0;
> +	}
> +
> +	return -ENOPKG;
> +}


Regards,

--
Mat Martineau
Intel OTC
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Herbert Xu June 24, 2016, 10:02 a.m. UTC | #2
Mat Martineau <mathew.j.martineau@linux.intel.com> wrote:
>
>> +     if (strcmp(encoding, "pkcs1") == 0) {
>> +             /* The data wangled by the RSA algorithm is typically padded
>> +              * and encoded in some manner, such as EMSA-PKCS1-1_5 [RFC3447
>> +              * sec 8.2].
>> +              */
>> +             if (!hash_algo)
>> +                     n = snprintf(alg_name, CRYPTO_MAX_ALG_NAME,
>> +                                  "pkcs1pad(%s)",
>> +                                  pkey->pkey_algo);
> 
> Did you see Herbert's patch that strips out non-hash pkcs1pad capabilities 
> (and the ensuing discussion)?
> 
> http://www.spinics.net/lists/linux-crypto/index.html#20432
> 
> I'm making use of pkcs1pad(rsa) with a TLS implementation, so it's good to 
> see it supported here.

Indeed I'm nacking this patch because it's exporting a purely
software algorithm to user-space for no good reason.  AFAICS
there is nothing in the pkcs1pad code that cannot be done in
user-space, even assuming that your private key is secret and
only accessible from the kernel.

IOW exporting the raw RSA might make sense because the key may
not be visible to user-space, or that the RSA might be implemented
in hardware offload, but there is no sane reason to export pkcs1pad.

Cheers,
David Howells June 24, 2016, 12:06 p.m. UTC | #3
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> IOW exporting the raw RSA might make sense because the key may
> not be visible to user-space, or that the RSA might be implemented
> in hardware offload, but there is no sane reason to export pkcs1pad.

The problem is that if I'm to produce consistency with, say, the TPM
interface, then I have to deal in wrapped/padded data - leastways as far as I
can tell from reading the docs.

David
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Herbert Xu June 25, 2016, 1:37 a.m. UTC | #4
On Fri, Jun 24, 2016 at 01:06:02PM +0100, David Howells wrote:
> 
> The problem is that if I'm to produce consistency with, say, the TPM
> interface, then I have to deal in wrapped/padded data - leastways as far as I
> can tell from reading the docs.

So the TPM device is accessed through the same interface? Where is
the code for it?

Thanks,
David Howells June 27, 2016, 2:27 p.m. UTC | #5
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> > The problem is that if I'm to produce consistency with, say, the TPM
> > interface, then I have to deal in wrapped/padded data - leastways as far
> > as I can tell from reading the docs.
> 
> So the TPM device is accessed through the same interface? Where is
> the code for it?

I have some patches I need to finish revamping.  I had it kind of working
(though with a slightly different user interface) - then TPMv2 support was
added to the TPM driver before I finished and I need to redo the patches.

David
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Herbert Xu June 28, 2016, 4:54 a.m. UTC | #6
On Mon, Jun 27, 2016 at 03:27:13PM +0100, David Howells wrote:
> 
> I have some patches I need to finish revamping.  I had it kind of working
> (though with a slightly different user interface) - then TPMv2 support was
> added to the TPM driver before I finished and I need to redo the patches.

In that case can we wait until this is ready before pushing the
user-space interface? Once you add a user-space interface it's
very difficult to change it again so we should be absolutely sure
what it's supposed to  look like before we add a new interace.

Thanks,
diff mbox

Patch

diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
index fd76b5fc3b3a..a48a47a1dff0 100644
--- a/crypto/asymmetric_keys/public_key.c
+++ b/crypto/asymmetric_keys/public_key.c
@@ -57,6 +57,81 @@  static void public_key_destroy(void *payload0, void *payload3)
 	public_key_signature_free(payload3);
 }
 
+/*
+ * Determine the crypto algorithm name.
+ */
+static
+int software_key_determine_akcipher(const char *encoding,
+				    const char *hash_algo,
+				    const struct public_key *pkey,
+				    char alg_name[CRYPTO_MAX_ALG_NAME])
+{
+	int n;
+
+	if (strcmp(encoding, "pkcs1") == 0) {
+		/* The data wangled by the RSA algorithm is typically padded
+		 * and encoded in some manner, such as EMSA-PKCS1-1_5 [RFC3447
+		 * sec 8.2].
+		 */
+		if (!hash_algo)
+			n = snprintf(alg_name, CRYPTO_MAX_ALG_NAME,
+				     "pkcs1pad(%s)",
+				     pkey->pkey_algo);
+		else
+			n = snprintf(alg_name, CRYPTO_MAX_ALG_NAME,
+				     "pkcs1pad(%s,%s)",
+				     pkey->pkey_algo, hash_algo);
+		return n >= CRYPTO_MAX_ALG_NAME ? -EINVAL : 0;
+	}
+
+	if (strcmp(encoding, "raw") == 0) {
+		strcpy(alg_name, pkey->pkey_algo);
+		return 0;
+	}
+
+	return -ENOPKG;
+}
+
+/*
+ * Query information about a key.
+ */
+static int software_key_query(const struct kernel_pkey_params *params,
+			      struct kernel_pkey_query *info)
+{
+	struct crypto_akcipher *tfm;
+	struct public_key *pkey = params->key->payload.data[asym_crypto];
+	char alg_name[CRYPTO_MAX_ALG_NAME];
+	int ret, len;
+
+	ret = software_key_determine_akcipher(params->encoding,
+					      params->hash_algo,
+					      pkey, alg_name);
+	if (ret < 0)
+		return ret;
+
+	tfm = crypto_alloc_akcipher(alg_name, 0, 0);
+	if (IS_ERR(tfm))
+		return PTR_ERR(tfm);
+
+	ret = crypto_akcipher_set_pub_key(tfm, pkey->key, pkey->keylen);
+	if (ret < 0)
+		goto error_free_tfm;
+
+	len = crypto_akcipher_maxsize(tfm);
+	info->key_size = len * 8;
+	info->max_data_size = len;
+	info->max_sig_size = len;
+	info->max_enc_size = len;
+	info->max_dec_size = len;
+	info->supported_ops = KEYCTL_SUPPORTS_VERIFY;
+	ret = 0;
+
+error_free_tfm:
+	crypto_free_akcipher(tfm);
+	pr_devel("<==%s() = %d\n", __func__, ret);
+	return ret;
+}
+
 struct public_key_completion {
 	struct completion completion;
 	int err;
@@ -83,8 +158,7 @@  int public_key_verify_signature(const struct public_key *pkey,
 	struct crypto_akcipher *tfm;
 	struct akcipher_request *req;
 	struct scatterlist sig_sg, digest_sg;
-	const char *alg_name;
-	char alg_name_buf[CRYPTO_MAX_ALG_NAME];
+	char alg_name[CRYPTO_MAX_ALG_NAME];
 	void *output;
 	unsigned int outlen;
 	int ret = -ENOMEM;
@@ -96,18 +170,11 @@  int public_key_verify_signature(const struct public_key *pkey,
 	BUG_ON(!sig->digest);
 	BUG_ON(!sig->s);
 
-	alg_name = sig->pkey_algo;
-	if (strcmp(sig->pkey_algo, "rsa") == 0) {
-		/* The data wangled by the RSA algorithm is typically padded
-		 * and encoded in some manner, such as EMSA-PKCS1-1_5 [RFC3447
-		 * sec 8.2].
-		 */
-		if (snprintf(alg_name_buf, CRYPTO_MAX_ALG_NAME,
-			     "pkcs1pad(rsa,%s)", sig->hash_algo
-			     ) >= CRYPTO_MAX_ALG_NAME)
-			return -EINVAL;
-		alg_name = alg_name_buf;
-	}
+	ret = software_key_determine_akcipher(sig->encoding,
+					      sig->hash_algo,
+					      pkey, alg_name);
+	if (ret < 0)
+		return ret;
 
 	tfm = crypto_alloc_akcipher(alg_name, 0, 0);
 	if (IS_ERR(tfm))
@@ -179,6 +246,7 @@  struct asymmetric_key_subtype public_key_subtype = {
 	.name_len		= sizeof("public_key") - 1,
 	.describe		= public_key_describe,
 	.destroy		= public_key_destroy,
+	.query			= software_key_query,
 	.verify_signature	= public_key_verify_signature_2,
 };
 EXPORT_SYMBOL_GPL(public_key_subtype);