[PATCH v6] DH support: add KDF handling support
Hi David,
Changes v6:
* addition of man/keyctl.1 documentation for dh_compute_kdf
and dh_compute_kdf_oi
* change use of .priv instead of .private to compile the code
Thanks
Stephan
---8<---
Add the interface logic to support DH with KDF handling support.
The dh_compute code now allows the following options:
- no KDF support / output of raw DH shared secret:
dh_compute
- KDF support without "other information" string:
dh_compute_kdf
- KDF support with "other information string:
dh_compute_kdf_oi
where the OI string is provided on STDIN.
The test to verify the code is based on a test vector used for the CAVS
testing of SP800-56A.
Signed-off-by: Stephan Mueller
---
Makefile | 1 +
keyctl.c | 133
keyutils.c | 14 +++
keyutils.h | 11 ++
man/keyctl.1 | 26 +
man/keyctl_dh_compute.3 | 57 +++
tests/keyctl/dh_compute/valid/runtest.sh | 168 +++
tests/toolbox.inc.sh | 44
version.lds | 2 +
9 files changed, 456 insertions(+)
diff --git a/Makefile b/Makefile
index 824bbbf..90fc33f 100644
--- a/Makefile
+++ b/Makefile
@@ -195,6 +195,7 @@ endif
$(LNS) keyctl_read.3 $(DESTDIR)$(MAN3)/keyctl_read_alloc.3
$(LNS) recursive_key_scan.3
$(DESTDIR)$(MAN3)/recursive_session_key_scan.3
$(LNS) keyctl_dh_compute.3 $(DESTDIR)$(MAN3)/keyctl_dh_compute_alloc.3
+ $(LNS) keyctl_dh_compute.3 $(DESTDIR)$(MAN3)/keyctl_dh_compute_kdf.3
$(INSTALL) -D -m 0644 keyutils.h $(DESTDIR)$(INCLUDEDIR)/keyutils.h
###
diff --git a/keyctl.c b/keyctl.c
index 801a864..0d1fac8 100644
--- a/keyctl.c
+++ b/keyctl.c
@@ -20,6 +20,7 @@
#include
#include
#include "keyutils.h"
+#include
struct command {
void (*action)(int argc, char *argv[]) __attribute__((noreturn));
@@ -67,6 +68,8 @@ static nr void act_keyctl_purge(int argc, char *argv[]);
static nr void act_keyctl_invalidate(int argc, char *argv[]);
static nr void act_keyctl_get_persistent(int argc, char *argv[]);
static nr void act_keyctl_dh_compute(int argc, char *argv[]);
+static nr void act_keyctl_dh_compute_kdf(int argc, char *argv[]);
+static nr void act_keyctl_dh_compute_kdf_oi(int argc, char *argv[]);
const struct command commands[] = {
{ act_keyctl___version, "--version","" },
@@ -76,6 +79,8 @@ const struct command commands[] = {
{ act_keyctl_clear, "clear","" },
{ act_keyctl_describe, "describe", "" },
{ act_keyctl_dh_compute, "dh_compute", " " },
+ { act_keyctl_dh_compute_kdf, "dh_compute_kdf", "
" },
+ { act_keyctl_dh_compute_kdf_oi, "dh_compute_kdf_oi", "
" },
{ act_keyctl_instantiate, "instantiate"," " },
{ act_keyctl_invalidate,"invalidate", "" },
{ act_keyctl_get_persistent, "get_persistent", " []" },
@@ -1663,6 +1668,7 @@ static void act_keyctl_dh_compute(int argc, char *argv[])
}
printf("%02hhx", *p);
+ *p = 0x00; /* zeroize buffer */
p++;
col++;
@@ -1674,6 +1680,133 @@ static void act_keyctl_dh_compute(int argc, char
*argv[])
} while (--ret > 0);
printf("\n");
+
+ free(buffer);
+
+ exit(0);
+}
+
+static void act_keyctl_dh_compute_kdf(int argc, char *argv[])
+{
+ key_serial_t private, prime, base;
+ char *buffer;
+ char *p;
+ int ret, sep, col;
+ unsigned long buflen = 0;
+
+ if (argc != 6)
+ format();
+
+ private = get_key_id(argv[1]);
+ prime = get_key_id(argv[2]);
+ base = get_key_id(argv[3]);
+
+ buflen = strtoul(argv[4], NULL, 10);
+ if (buflen == ULONG_MAX)
+ error("dh_compute: cannot convert generated length value");
+
+ buffer = malloc(buflen);
+ if (!buffer)
+ error("dh_compute: cannot allocate memory");
+
+ ret = keyctl_dh_compute_kdf(private, prime, base, argv[5], NULL, 0,
+ buffer, buflen);
+ if (ret < 0)
+ error("keyctl_dh_compute_alloc");
+
+ /* hexdump the contents */
+ printf("%u bytes of data in result:\n", ret);
+
+ sep = 0;
+ col = 0;
+ p = buffer;
+
+ do {
+ if (sep) {
+ putchar(sep);
+ sep = 0;
+ }
+
+ printf("%02hhx", *p);
+ *p = 0x00; /* zeroize buffer */
+ p++;
+
+ col++;
+ if (col % 32 == 0)
+ sep = '\n';
+ else if (col % 4 == 0)
+ sep = ' ';
+
+ } while (--
Re: [PATCH v5] DH support: add KDF handling support
Am Dienstag, 4. April 2017, 00:22:28 CEST schrieb David Howells:
Hi David,
> Stephan Mueller wrote:
> > + struct keyctl_dh_params params = { .private = private,
>
> That doesn't compile. I think you meant ".priv".
I think some code has been changed since I prepared the patch. Allow me to
port the patch to the current code tree.
I will also update the keyctl.1 man page.
Ciao
Stephan
Re: [PATCH 7/7] crypto, x86, LLVM: aesni - fix token pasting
El Thu, Mar 16, 2017 at 05:15:20PM -0700 Michael Davidson ha dit: > aes_ctrby8_avx-x86_64.S uses the C preprocessor for token pasting > of character sequences that are not valid preprocessor tokens. > While this is allowed when preprocessing assembler files it exposes > an incompatibilty between the clang and gcc preprocessors where > clang does not strip leading white space from macro parameters, > leading to the CONCAT(%xmm, i) macro expansion on line 96 resulting > in a token with a space character embedded in it. > > While this could be fixed by deleting the offending space character, > the assembler is perfectly capable of handling the token pasting > correctly for itself so it seems preferable to let it do so and to > get rid or the CONCAT(), DDQ() and XMM() preprocessor macros. > > Signed-off-by: Michael Davidson > --- > arch/x86/crypto/aes_ctrby8_avx-x86_64.S | 7 ++- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/crypto/aes_ctrby8_avx-x86_64.S > b/arch/x86/crypto/aes_ctrby8_avx-x86_64.S > index a916c4a61165..5f6a5af9c489 100644 > --- a/arch/x86/crypto/aes_ctrby8_avx-x86_64.S > +++ b/arch/x86/crypto/aes_ctrby8_avx-x86_64.S > @@ -65,7 +65,6 @@ > #include > #include > > -#define CONCAT(a,b) a##b > #define VMOVDQ vmovdqu > > #define xdata0 %xmm0 > @@ -92,8 +91,6 @@ > #define num_bytes%r8 > > #define tmp %r10 > -#define DDQ(i) CONCAT(ddq_add_,i) > -#define XMM(i) CONCAT(%xmm, i) > #define DDQ_DATA0 > #define XDATA 1 > #define KEY_128 1 > @@ -131,12 +128,12 @@ ddq_add_8: > /* generate a unique variable for ddq_add_x */ > > .macro setddq n > - var_ddq_add = DDQ(\n) > + var_ddq_add = ddq_add_\n > .endm > > /* generate a unique variable for xmm register */ > .macro setxdata n > - var_xdata = XMM(\n) > + var_xdata = %xmm\n > .endm > > /* club the numeric 'id' to the symbol 'name' */ Any feedback on this patch? Thanks Matthias
Re: [PATCH v5] KEYS: add SP800-56A KDF support for DH
Pulled.
Re: [PATCH 3/7] x86, LLVM: suppress clang warnings about unaligned accesses
El Fri, Mar 17, 2017 at 04:50:19PM -0700 h...@zytor.com ha dit: > On March 16, 2017 5:15:16 PM PDT, Michael Davidson wrote: > >Suppress clang warnings about potential unaliged accesses > >to members in packed structs. This gets rid of almost 10,000 > >warnings about accesses to the ring 0 stack pointer in the TSS. > > > >Signed-off-by: Michael Davidson > >--- > > arch/x86/Makefile | 5 + > > 1 file changed, 5 insertions(+) > > > >diff --git a/arch/x86/Makefile b/arch/x86/Makefile > >index 894a8d18bf97..7f21703c475d 100644 > >--- a/arch/x86/Makefile > >+++ b/arch/x86/Makefile > >@@ -128,6 +128,11 @@ endif > > KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args) > > endif > > > >+ifeq ($(cc-name),clang) > >+# Suppress clang warnings about potential unaligned accesses. > >+KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member) > >+endif > >+ > > ifdef CONFIG_X86_X32 > > x32_ld_ok := $(call try-run,\ > > /bin/echo -e '1: .quad 1b' | \ > > Why conditional on clang? My understanding is that this warning is clang specific, it is not listed on https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html Cheers Matthias
Re: [PATCH 1/7] Makefile, LLVM: add -no-integrated-as to KBUILD_[AC]FLAGS
El Thu, Mar 16, 2017 at 05:15:14PM -0700 Michael Davidson ha dit: > Add -no-integrated-as to KBUILD_AFLAGS and KBUILD_CFLAGS > for clang. > > Signed-off-by: Michael Davidson > --- > Makefile | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Makefile b/Makefile > index b841fb36beb2..b21fd0ca2946 100644 > --- a/Makefile > +++ b/Makefile > @@ -704,6 +704,8 @@ KBUILD_CFLAGS += $(call cc-disable-warning, > tautological-compare) > # See modpost pattern 2 > KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,) > KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior) > +KBUILD_CFLAGS += $(call cc-option, -no-integrated-as) > +KBUILD_AFLAGS += $(call cc-option, -no-integrated-as) > else > > # These warnings generated too much noise in a regular build. Ping, any feedback on this patch? Cheers Matthias
Re: [PATCH v5] DH support: add KDF handling support
Stephan Mueller wrote: > this patch changes the documentation, the naming of the variables > and the test case to refer to the variable name of a hashname > instead of kdfname to match the current kernel implementation. It's also needs an update to man1/keyctl.1. David
Re: [PATCH v5] DH support: add KDF handling support
Stephan Mueller wrote:
> + struct keyctl_dh_params params = { .private = private,
That doesn't compile. I think you meant ".priv".
David
Re: [PATCH] crypto: caam - fix JR platform device subsequent (re)creations
On Mon, Apr 3, 2017 at 10:12 AM, Horia Geantă wrote:
> The way Job Ring platform devices are created and released does not
> allow for multiple create-release cycles.
>
> JR0 Platform device creation error
> JR0 Platform device creation error
> caam 210.caam: no queues configured, terminating
> caam: probe of 210.caam failed with error -12
>
> The reason is that platform devices are created for each job ring:
>
> for_each_available_child_of_node(nprop, np)
> if (of_device_is_compatible(np, "fsl,sec-v4.0-job-ring") ||
> of_device_is_compatible(np, "fsl,sec4.0-job-ring")) {
> ctrlpriv->jrpdev[ring] =
> of_platform_device_create(np, NULL, dev);
>
> which sets OF_POPULATED on the device node, but then it cleans these up:
>
> /* Remove platform devices for JobRs */
> for (ring = 0; ring < ctrlpriv->total_jobrs; ring++) {
> if (ctrlpriv->jrpdev[ring])
> of_device_unregister(ctrlpriv->jrpdev[ring]);
> }
>
> which leaves OF_POPULATED set.
>
> Use of_platform_populate / of_platform_depopulate instead.
> This allows for a bit of driver clean-up, jrpdev is no longer needed.
>
> Logic changes a bit too:
> -exit in case of_platform_populate fails, since currently even QI backend
> depends on JR; true, we no longer support the case when "some" of the JR
> DT nodes are incorrect
> -when cleaning up, caam_remove() would also depopulate RTIC in case
> it would have been populated somewhere else - not the case for now
>
> Fixes: 313ea293e9c4d ("crypto: caam - Add Platform driver for Job Ring")
> Reported-by: Russell King
> Suggested-by: Rob Herring
> Signed-off-by: Horia Geantă
Acked-by: Rob Herring
[PATCH] crypto: caam - fix invalid dereference in caam_rsa_init_tfm()
In case caam_jr_alloc() fails, ctx->dev carries the error code,
thus accessing it with dev_err() is incorrect.
Cc: # 4.8+
Fixes: 8c419778ab57e ("crypto: caam - add support for RSA algorithm")
Signed-off-by: Horia Geantă
---
drivers/crypto/caam/caampkc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/crypto/caam/caampkc.c b/drivers/crypto/caam/caampkc.c
index 32100c4851dd..49cbdcba7883 100644
--- a/drivers/crypto/caam/caampkc.c
+++ b/drivers/crypto/caam/caampkc.c
@@ -506,7 +506,7 @@ static int caam_rsa_init_tfm(struct crypto_akcipher *tfm)
ctx->dev = caam_jr_alloc();
if (IS_ERR(ctx->dev)) {
- dev_err(ctx->dev, "Job Ring Device allocation for transform
failed\n");
+ pr_err("Job Ring Device allocation for transform failed\n");
return PTR_ERR(ctx->dev);
}
--
2.12.0.264.gd6db3f216544
[PATCH] crypto: caam - fix JR platform device subsequent (re)creations
The way Job Ring platform devices are created and released does not
allow for multiple create-release cycles.
JR0 Platform device creation error
JR0 Platform device creation error
caam 210.caam: no queues configured, terminating
caam: probe of 210.caam failed with error -12
The reason is that platform devices are created for each job ring:
for_each_available_child_of_node(nprop, np)
if (of_device_is_compatible(np, "fsl,sec-v4.0-job-ring") ||
of_device_is_compatible(np, "fsl,sec4.0-job-ring")) {
ctrlpriv->jrpdev[ring] =
of_platform_device_create(np, NULL, dev);
which sets OF_POPULATED on the device node, but then it cleans these up:
/* Remove platform devices for JobRs */
for (ring = 0; ring < ctrlpriv->total_jobrs; ring++) {
if (ctrlpriv->jrpdev[ring])
of_device_unregister(ctrlpriv->jrpdev[ring]);
}
which leaves OF_POPULATED set.
Use of_platform_populate / of_platform_depopulate instead.
This allows for a bit of driver clean-up, jrpdev is no longer needed.
Logic changes a bit too:
-exit in case of_platform_populate fails, since currently even QI backend
depends on JR; true, we no longer support the case when "some" of the JR
DT nodes are incorrect
-when cleaning up, caam_remove() would also depopulate RTIC in case
it would have been populated somewhere else - not the case for now
Fixes: 313ea293e9c4d ("crypto: caam - Add Platform driver for Job Ring")
Reported-by: Russell King
Suggested-by: Rob Herring
Signed-off-by: Horia Geantă
---
Not sending this directly to -stable, since it does not apply cleanly.
drivers/crypto/caam/ctrl.c | 64 ++--
drivers/crypto/caam/intern.h | 1 -
2 files changed, 20 insertions(+), 45 deletions(-)
diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index b3a94d5eff26..f7792a99469a 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -305,15 +305,13 @@ static int caam_remove(struct platform_device *pdev)
struct device *ctrldev;
struct caam_drv_private *ctrlpriv;
struct caam_ctrl __iomem *ctrl;
- int ring;
ctrldev = &pdev->dev;
ctrlpriv = dev_get_drvdata(ctrldev);
ctrl = (struct caam_ctrl __iomem *)ctrlpriv->ctrl;
- /* Remove platform devices for JobRs */
- for (ring = 0; ring < ctrlpriv->total_jobrs; ring++)
- of_device_unregister(ctrlpriv->jrpdev[ring]);
+ /* Remove platform devices under the crypto node */
+ of_platform_depopulate(ctrldev);
#ifdef CONFIG_CAAM_QI
if (ctrlpriv->qidev)
@@ -410,10 +408,21 @@ int caam_get_era(void)
}
EXPORT_SYMBOL(caam_get_era);
+static const struct of_device_id caam_match[] = {
+ {
+ .compatible = "fsl,sec-v4.0",
+ },
+ {
+ .compatible = "fsl,sec4.0",
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(of, caam_match);
+
/* Probe routine for CAAM top (controller) level */
static int caam_probe(struct platform_device *pdev)
{
- int ret, ring, ridx, rspec, gen_sk, ent_delay = RTSDCTL_ENT_DLY_MIN;
+ int ret, ring, gen_sk, ent_delay = RTSDCTL_ENT_DLY_MIN;
u64 caam_id;
struct device *dev;
struct device_node *nprop, *np;
@@ -589,21 +598,9 @@ static int caam_probe(struct platform_device *pdev)
goto iounmap_ctrl;
}
- /*
-* Detect and enable JobRs
-* First, find out how many ring spec'ed, allocate references
-* for all, then go probe each one.
-*/
- rspec = 0;
- for_each_available_child_of_node(nprop, np)
- if (of_device_is_compatible(np, "fsl,sec-v4.0-job-ring") ||
- of_device_is_compatible(np, "fsl,sec4.0-job-ring"))
- rspec++;
-
- ctrlpriv->jrpdev = devm_kcalloc(&pdev->dev, rspec,
- sizeof(*ctrlpriv->jrpdev), GFP_KERNEL);
- if (ctrlpriv->jrpdev == NULL) {
- ret = -ENOMEM;
+ ret = of_platform_populate(nprop, caam_match, NULL, dev);
+ if (ret) {
+ dev_err(dev, "JR platform devices creation error\n");
goto iounmap_ctrl;
}
@@ -618,29 +615,19 @@ static int caam_probe(struct platform_device *pdev)
ctrlpriv->dfs_root = debugfs_create_dir(dev_name(dev), NULL);
ctrlpriv->ctl = debugfs_create_dir("ctl", ctrlpriv->dfs_root);
#endif
+
ring = 0;
- ridx = 0;
- ctrlpriv->total_jobrs = 0;
for_each_available_child_of_node(nprop, np)
if (of_device_is_compatible(np, "fsl,sec-v4.0-job-ring") ||
of_device_is_compatible(np, "fsl,sec4.0-job-ring")) {
- ctrlpriv->jrpdev[ring] =
- of_platform_device_create(np, NULL, dev);
- if (!ct
Re: algif for compression?
Hi Stephen,
> If you follow the AF_ALG implementations that are currently in the kernel, a
> calling application receives two file descriptors. The first file descriptor
> is the reference to a tfm, the second is the reference to a particular
> request.
>
> Wouldn't those file descriptors be the reference you are looking for?
Those descriptors are sufficient to pass data from userspace
applications to the algif interface.
However the issue is passing those from the interface to the driver
using the current acomp API's.
This is the currently exposed interface to the comp framework. Am I
missing something here?
int (*compress)(struct acomp_req *req);
int (*decompress)(struct acomp_req *req);
struct acomp_req {
struct crypto_async_request base;
struct scatterlist *src;
struct scatterlist *dst;
unsigned int slen;
unsigned int dlen;
u32 flags;
void *__ctx[] CRYPTO_MINALIGN_ATTR;
};
Thanks
Abed
On Mon, Apr 3, 2017 at 12:56 PM, Stephan Müller wrote:
> Am Montag, 3. April 2017, 08:10:19 CEST schrieb abed mohammad kamaluddin:
>
> Hi abed,
>
>> Hi Herbert,
>>
>> We have implemented algif acomp interface for user space applications
>> to utilize the kernel compression framework and the hardware drivers
>> using it and have been testing it using userspace zlib library.
>>
>> However, what we find lacking in the existing acomp implementation is
>> the ability to pass context data between the applications and the
>> drivers using the acomp interface. Currently the interface only allows
>> src/dest data and a flag argument with each request. There are two
>> context pointers, one in acomp_req and another in crypto_tfm but they
>> are for internal use and not available to applications through the
>> api's. Would it be acceptable to add fields that need to be
>> communicated b/w the driver and applications like history,
>> csum/adler32, EOF, stream ctx to acomp_req.
>>
>> Or is there any other way, which I may have missed, through which we
>> can pass ctx data between applications and drivers while using the
>> kernel compression framework?
>
> If you follow the AF_ALG implementations that are currently in the kernel, a
> calling application receives two file descriptors. The first file descriptor
> is the reference to a tfm, the second is the reference to a particular
> request.
>
> Wouldn't those file descriptors be the reference you are looking for?
>
> Ciao
> Stephan
Re: algif for compression?
Am Montag, 3. April 2017, 08:10:19 CEST schrieb abed mohammad kamaluddin: Hi abed, > Hi Herbert, > > We have implemented algif acomp interface for user space applications > to utilize the kernel compression framework and the hardware drivers > using it and have been testing it using userspace zlib library. > > However, what we find lacking in the existing acomp implementation is > the ability to pass context data between the applications and the > drivers using the acomp interface. Currently the interface only allows > src/dest data and a flag argument with each request. There are two > context pointers, one in acomp_req and another in crypto_tfm but they > are for internal use and not available to applications through the > api's. Would it be acceptable to add fields that need to be > communicated b/w the driver and applications like history, > csum/adler32, EOF, stream ctx to acomp_req. > > Or is there any other way, which I may have missed, through which we > can pass ctx data between applications and drivers while using the > kernel compression framework? If you follow the AF_ALG implementations that are currently in the kernel, a calling application receives two file descriptors. The first file descriptor is the reference to a tfm, the second is the reference to a particular request. Wouldn't those file descriptors be the reference you are looking for? Ciao Stephan
