Re: TLS PRF + HKDF

[Nikos Mavrogiannopoulos]

On Wed, 2017-09-06 at 22:47 +0200, Niels Möller wrote:
> ni...@lysator.liu.se (Niels Möller) writes (back in May):
> 
> > Nikos Mavrogiannopoulos  writes:
> > 
> > > --- /dev/null
> > > +++ b/hkdf.c
> > > @@ -0,0 +1,85 @@
> > > +/*
> > > + * Copyright (C) 2017 Red Hat, Inc.
> > > + *
> > > + * Author: Nikos Mavrogiannopoulos
> > > + *
> > > + * This file is part of GnuTLS.
> > This file carries a GnuTLS copyright notice rather than the Nettle
> > copyright notice used in most other files. I guess that's
> > unintentional?
> > Indentation in the rest of this file is also using a very different
> > style than the rest of Nettle.
> 
> Forgot about this. I'll just change to the (slightly stricter, GPLv2+
> or
> LGPLv3+) Nettle licensing notice, OK?

Fine with me.

regards,
Nikos

___
nettle-bugs mailing list
nettle-bugs@lists.lysator.liu.se
http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs

Re: TLS PRF + HKDF

[Niels Möller]

Nikos Mavrogiannopoulos  writes:

> On Wed, 2017-08-30 at 19:05 +0200, Niels Möller wrote:
>> I've pushed what I believe was the latest version of the hkdf patch +
>> this documentation patch to the branch hkdf-support. Can you double
>> check that I got it right?
>
> Thanks, it matches the version I've included in gnutls.

Good! Pushed now, with some minor additional cleanups.

Regards,
/Niels

-- 
Niels Möller. PGP-encrypted email is preferred. Keyid 368C6677.
Internet email is subject to wholesale government surveillance.
___
nettle-bugs mailing list
nettle-bugs@lists.lysator.liu.se
http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs

Re: TLS PRF + HKDF

[Nikos Mavrogiannopoulos]

On Wed, 2017-08-30 at 19:05 +0200, Niels Möller wrote:
> Nikos Mavrogiannopoulos  writes:
> 
> > I have modified the text to be more self-contained and clarify the
> > role of the variables, which may address terminology as well. Let
> > me
> > know if that's ok.
> 
> Thanks!
> 
> I've pushed what I believe was the latest version of the hkdf patch +
> this documentation patch to the branch hkdf-support. Can you double
> check that I got it right?

Thanks, it matches the version I've included in gnutls.

regards,
Nikos

___
nettle-bugs mailing list
nettle-bugs@lists.lysator.liu.se
http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs

Re: TLS PRF + HKDF

[Niels Möller]

Nikos Mavrogiannopoulos  writes:

> On Mon, 2017-05-22 at 19:09 +0200, Niels Möller wrote:
>
>> Is it required that hkdf_extract is used in some way to produce the
>> key
>> for hkdf_expand? Then I think the relation between _extract and
>> _expand
>> needs to be clarified. Would you always have the same number of calls
>> to
>> _extract and _expand, or could do _extract once and _expand multiple
>> times (with different info string)?
>
> I'm not sure what you mean. The relation is defined in HKDF document,
> though upper protocols like tls 1.3 may utilize these in arbitrary
> ways. Nettle provides the implementation of the primitives.

Sorry this got a bit stalled. I would like the Nettle docs to be
reasonably self-contained, and explain what the primitive does, what
problem it is intended to solve, and the typical way to use it. And in
case terminology in the relevant RFC or other literature differs from
what's used elsewhere in the Nettle manual, point of how they relate. In
particuler, I found the "salt"/"key"/"secret" arguments a bit confusing,
as well as the purpose of the _extract function.

With your current patch to the docs, I'll have to read the HKDF spec
carefully myself to be able to review the code and the docs, and I
haven't yet gotten the time to do that. Clear and more self-contained
documentation would make this easier.

Best regards,
/Niels

PS. I'm currently on vacation, i.e., no work duties in the way of Nettle
hacking, but I'm a bit offline and not reading email daily.

-- 
Niels Möller. PGP-encrypted email is preferred. Keyid 368C6677.
Internet email is subject to wholesale government surveillance.
___
nettle-bugs mailing list
nettle-bugs@lists.lysator.liu.se
http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs

Re: TLS PRF + HKDF

[Niels Möller]

Nikos Mavrogiannopoulos  writes:

> A backwards compatible approach is to leave the nettle_hashes
> accessible and provide the function as well. That will not be any more
> broken than 3.3 is, and would allow 3.3 programs to be linked with 3.4
> without recompilation.

Could work.

> You can warn on use of nettle_hashes with the attached patch, most
> likely you can introduce a custom message as well with the replacement
> function.

I think I'd prefer to use

  #define nettle_hashes (get_nettle_hashes())

so that a recompile fixes the issue without any source code changes.

Regards,
/Niels

-- 
Niels Möller. PGP-encrypted email is preferred. Keyid 368C6677.
Internet email is subject to wholesale government surveillance.
___
nettle-bugs mailing list
nettle-bugs@lists.lysator.liu.se
http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs

Re: TLS PRF + HKDF

[Niels Möller]

Nikos Mavrogiannopoulos  writes:

>>   update(mac_ctx, 1, );
>>   if (left <= digest_size) 
>> {
>>   digest(mac_ctx, left, dst);
>>   return;
>> }
>>   digest(max_ctx, digest_size, dst);
>>   update(mac_ctx, digest_size, dst);
>> }
>
> I'm fine with that. Do you want me to post an updated patch or would
> you modify it?

If you can test it and post an updated patch, that's helpful.

Regards,
/Niels

-- 
Niels Möller. PGP-encrypted email is preferred. Keyid 368C6677.
Internet email is subject to wholesale government surveillance.
___
nettle-bugs mailing list
nettle-bugs@lists.lysator.liu.se
http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs

Re: TLS PRF + HKDF

[Nikos Mavrogiannopoulos]

On Mon, May 22, 2017 at 12:36 AM, Niels Möller  wrote:

> And regarding nettle-3.3, I guess it's time to try to formulate what
> the relase objectives should be.
>
> 1. Fix the ABI problem (which unfortunately implies an abi break). Some
>progress, but I don't think I've published my wip branch.

I'd recommend against breaking the ABI. It would mean that any new
additions would take significant time to be propagated into
distributions. Is there a way to fix the issue by introducing new safe
APIs?

> 4. Get skein256 in (see skein branch), and possible skein512 too.

While interesting to have, is there any practical use of it?

regards,
Nikos
___
nettle-bugs mailing list
nettle-bugs@lists.lysator.liu.se
http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs

Re: TLS PRF + HKDF

[Niels Möller]

Nikos Mavrogiannopoulos  writes:

> On Tue, 2017-05-16 at 22:47 +0200, Niels Möller wrote:
>

>> > +  while(left > 0) {
>> > +  /* T(i) */
>> > +  set_key(mac_ctx, prk_size, prk);
>> > +  if (started != 0) {
>> > +  update(mac_ctx, digest_size, Ttmp);
>> > +  } else {
>> > +  started = 1;
>> > +  }
>> > +  if (info_size)
>> > +  update(mac_ctx, info_size, info);
>> > +  update(mac_ctx, 1, );
>> > +
>> > +  if (left < digest_size)
>> > +  digest_size = left;
>> > +
>> > +  digest(mac_ctx, digest_size, dst);
>> > +  Ttmp = dst;
>> > +
>> > +  left -= digest_size;
>> > +  dst += digest_size;
>> > +  i++;
>> > +  }
>> 
>> I think this loop would clearer if Ttmp was replaced by (dst -
>> digest_size), and maybe it would make sense to take out the first
>> and/or final iterations.
>
> Patch 0005 unrolls the first loop and does that change. I find that
> longer and not as easy to follow, but I may have not caught what you
> meant.

I was thinking of something like

  if (!length)
return;

  for (;; dst += digest_size, length -= digest_size, i++)
{
  update(mac_ctx, info_size, info); /* info_size == 0 should work fine */
  update(mac_ctx, 1, );
  if (left <= digest_size) 
{
  digest(mac_ctx, left, dst);
  return;
}
  digest(max_ctx, digest_size, dst);
  update(mac_ctx, digest_size, dst);
}

Regards,
/Niels

-- 
Niels Möller. PGP-encrypted email is preferred. Keyid 368C6677.
Internet email is subject to wholesale government surveillance.
___
nettle-bugs mailing list
nettle-bugs@lists.lysator.liu.se
http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs

Re: TLS PRF + HKDF

[Nikos Mavrogiannopoulos]

On Tue, 2017-05-16 at 22:47 +0200, Niels Möller wrote:

> >  and a comment in .bootstrap for the
> > testsuite/.test-rules.make which I always seem to forget and spend
> > an
> > hour figuring out what is going on. Ideally .test-rules.make should
> > depend on Makefile.in.
> 
> Have you tested if it works to just add that dependency? 

It seems to do. The first patch attached does just that.


+void
> > +hkdf_extract(void *mac_ctx,
> > +    nettle_hmac_set_key_func *set_key,
> > +    nettle_hash_update_func *update,
> > +    nettle_hash_digest_func *digest,
> > +    size_t digest_size,
> > +    size_t salt_size, const uint8_t *salt,
> > +    size_t secret_size, const uint8_t *secret,
> > +    uint8_t *dst)
> > +{
> > +   set_key(mac_ctx, salt_size, salt);
> > +
> > +   update(mac_ctx, secret_size, secret);
> > +   digest(mac_ctx, digest_size, dst);
> > +}
> 
> This looks like a plain application of a mac, digest = MAC(salt,
> secret), is this really useful?

I've kept it because it makes the mapping from RFC to hkdf.h simpler.
Let me know if you would prefer a macro or inline function.

> Not sure about the typedef nettle_hmac_set_key_func, there's nothing
> hmac specific, besides key size being variable? And it's identical to
> nettle_hash_update_func, right?

No longer there.

> > +   while(left > 0) {
> > +   /* T(i) */
> > +   set_key(mac_ctx, prk_size, prk);
> > +   if (started != 0) {
> > +   update(mac_ctx, digest_size, Ttmp);
> > +   } else {
> > +   started = 1;
> > +   }
> > +   if (info_size)
> > +   update(mac_ctx, info_size, info);
> > +   update(mac_ctx, 1, );
> > +
> > +   if (left < digest_size)
> > +   digest_size = left;
> > +
> > +   digest(mac_ctx, digest_size, dst);
> > +   Ttmp = dst;
> > +
> > +   left -= digest_size;
> > +   dst += digest_size;
> > +   i++;
> > +   }
> 
> I think this loop would clearer if Ttmp was replaced by (dst -
> digest_size), and maybe it would make sense to take out the first
> and/or final iterations.

Patch 0005 unrolls the first loop and does that change. I find that
longer and not as easy to follow, but I may have not caught what you
meant.

The last patch adds documentation for the added functions.

regards,
Nikos
From 9ce6041c3fb03c6b0afc650fdec3a86c87216a93 Mon Sep 17 00:00:00 2001
From: Nikos Mavrogiannopoulos 
Date: Wed, 17 May 2017 15:45:40 +0200
Subject: [PATCH 1/6] testsuite/Makefile.in: ensure .test-rules.make is
 regenerated

That is, regenerate when Makefile.in is modified.

Signed-off-by: Nikos Mavrogiannopoulos 
---
 testsuite/Makefile.in | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/testsuite/Makefile.in b/testsuite/Makefile.in
index 590691c..8f18f84 100644
--- a/testsuite/Makefile.in
+++ b/testsuite/Makefile.in
@@ -96,7 +96,9 @@ dlopen-test$(EXEEXT): dlopen-test.$(OBJEXT) testutils.$(OBJEXT)
 	$(LINK) dlopen-test.$(OBJEXT) -ldl -o dlopen-test$(EXEEXT)
 
 .PHONY: test-rules
-test-rules:
+test-rules: .test-rules.make
+
+.test-rules.make: Makefile.in
 	(for f in $(TS_NETTLE) $(TS_HOGWEED) $(EXTRA_TARGETS) ; do \
 	  echo $$f'$$(EXEEXT): '$$f'.$$(OBJEXT)' ; \
 	  echo '	$$(LINK) '$$f'.$$(OBJEXT) $$(TEST_OBJS) -o '$$f'$$(EXEEXT)' ; \
@@ -107,6 +109,10 @@ test-rules:
 	  echo '	$$(LINK_CXX) '$$f'.$$(OBJEXT) $$(TEST_OBJS) -o '$$f'$$(EXEEXT)' ; \
 	  echo ; \
 	done) > $(srcdir)/.test-rules.make
+	@echo "**"
+	@echo "testsuite Makefile rules have been regenerated; please re-run make"
+	@echo "**"
+	false
 
 include $(srcdir)/.test-rules.make
 
-- 
2.9.3

From 9150be6bd21639cd0b2a2e86ecd670cd91b1a2c9 Mon Sep 17 00:00:00 2001
From: Nikos Mavrogiannopoulos 
Date: Tue, 16 May 2017 12:57:12 +0200
Subject: [PATCH 2/6] Added the TLS 1.0 PRF and test vectors

Signed-off-by: Nikos Mavrogiannopoulos 
---
 Makefile.in|   4 +-
 testsuite/.test-rules.make |   3 +
 testsuite/Makefile.in  |   2 +-
 testsuite/tls10-prf-test.c |  42 ++
 tls1-prf.c | 133 +
 tls1-prf.h |  57 +++
 6 files changed, 238 insertions(+), 3 deletions(-)
 create mode 100644 testsuite/tls10-prf-test.c
 create mode 100644 tls1-prf.c
 create mode 100644 tls1-prf.h

diff --git a/Makefile.in b/Makefile.in
index 7e8f29c..5f07bee 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -106,7 +106,7 @@ nettle_SOURCES = aes-decrypt-internal.c aes-decrypt.c \
 		 gosthash94.c gosthash94-meta.c \
 		 hmac.c hmac-md5.c hmac-ripemd160.c hmac-sha1.c \
 		 hmac-sha224.c hmac-sha256.c hmac-sha384.c hmac-sha512.c \
-		 knuth-lfib.c \
+		 knuth-lfib.c tls1-prf.c \
 		 md2.c 

Re: TLS PRF + HKDF

[Niels Möller]

Nikos Mavrogiannopoulos  writes:

>  The attached patch set introduces new APIs to access the TLS 1.0 and
> 1.2 PRFs as well as the HKDF (rfc5869) function used in TLS 1.3.

Thanks, a few initial comments.

Are the TLS-1.0 and TLS-1.2 PRFs in any use outside of TLS?

> There are few other updates to .gitlab-ci.yml (to use a more recent
> asan and ubsan versions),

Applied.

>  and a comment in .bootstrap for the
> testsuite/.test-rules.make which I always seem to forget and spend an
> hour figuring out what is going on. Ideally .test-rules.make should
> depend on Makefile.in.

Have you tested if it works to just add that dependency? 

An alternative might be to drop support for non-GNU make programs, then
one could probably write these as %-pattern rules directly in
Makefile.in.

> I'd also suggest to rename the .bootstrap to bootstrap.sh as the latter
> is quite a convention to find in projects (.bootstrap is not even
> listed in ls).

Makes sense. There are references to the name .bootstrap in README and
in .gitlab-ci.yml, any other place that would need updating?

> +void
> +hkdf_extract(void *mac_ctx,
> +  nettle_hmac_set_key_func *set_key,
> +  nettle_hash_update_func *update,
> +  nettle_hash_digest_func *digest,
> +  size_t digest_size,
> +  size_t salt_size, const uint8_t *salt,
> +  size_t secret_size, const uint8_t *secret,
> +  uint8_t *dst)
> +{
> + set_key(mac_ctx, salt_size, salt);
> +
> + update(mac_ctx, secret_size, secret);
> + digest(mac_ctx, digest_size, dst);
> +}

This looks like a plain application of a mac, digest = MAC(salt,
secret), is this really useful?

Not sure about the typedef nettle_hmac_set_key_func, there's nothing
hmac specific, besides key size being variable? And it's identical to
nettle_hash_update_func, right?

Also, since it seems the key is fixed, both in this function and below, I
think it would be better to leave setting the mac key to the caller, and
pass in a mac ctx with key already set. That would also be more consistent
with the pbkdf2 code, and reduce the number of argument.

Note that the Nettle mac convention is that the sequence

  set_key
  update
  digest
  update
  digest
  ...
  update
  digest

works fine.

> +void
> +hkdf_expand(void *mac_ctx,
> +  nettle_hmac_set_key_func *set_key,
> +  nettle_hash_update_func *update,
> +  nettle_hash_digest_func *digest,
> +  size_t digest_size,
> +  size_t prk_size, const uint8_t *prk,
> +  size_t info_size, const uint8_t *info,
> +  size_t length, uint8_t *dst)
> +{
> + uint8_t *Ttmp;
> + ssize_t left;
> + uint8_t i = 1;
> + unsigned started = 0;
> +
> + left = length;
> +
> + while(left > 0) {
> + /* T(i) */
> + set_key(mac_ctx, prk_size, prk);
> + if (started != 0) {
> + update(mac_ctx, digest_size, Ttmp);
> + } else {
> + started = 1;
> + }
> + if (info_size)
> + update(mac_ctx, info_size, info);
> + update(mac_ctx, 1, );
> +
> + if (left < digest_size)
> + digest_size = left;
> +
> + digest(mac_ctx, digest_size, dst);
> + Ttmp = dst;
> +
> + left -= digest_size;
> + dst += digest_size;
> + i++;
> + }

I think this loop would clearer if Ttmp was replaced by (dst -
digest_size), and maybe it would make sense to take out the first and/or
final iterations.

Regards,
/Niels

-- 
Niels Möller. PGP-encrypted email is preferred. Keyid 368C6677.
Internet email is subject to wholesale government surveillance.
___
nettle-bugs mailing list
nettle-bugs@lists.lysator.liu.se
http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs