gh-libtom/libtomcrypt
Watch
1
0
Fork
You've already forked libtomcrypt
 0
Code Issues 42 Pull requests 17 Releases 9 Wiki Activity

RFC 7539 - ChaCha20 and Poly1305 #171

Merged
karel-m merged 18 commits from pr/chacha20poly1305 into develop 2017-04-03 14:21:21 -05:00
Conversation 49 Commits 18 Files changed 38 +1933 -73
karel-m commented 2017-03-21 13:52:00 -05:00 (Migrated from github.com)

Basically the reference implementation adopted to our C macros and our calling conventions.

https://tools.ietf.org/html/rfc7539

Basically the reference implementation adopted to our C macros and our calling conventions. https://tools.ietf.org/html/rfc7539
👍 2
sjaeckel commented 2017-03-22 06:13:17 -05:00 (Migrated from github.com)

  1. as it's a stream cipher, would it make sense to integrate it in the prng environment as all the other stream ciphers?

    or should we think about integrating resp. how to integrate the stream ciphers in the crypt environment?

    or should we provide an alternative environment or re-think the entire approach?

  2. what do you think about this?

In my opinion we shouldn't introduce new algorithms with unsigned char* for in- or output data.

diff --git a/src/ciphers/chacha/chacha_crypt.c b/src/ciphers/chacha/chacha_crypt.c
index 1ac3aa7..359e485 100644
--- a/src/ciphers/chacha/chacha_crypt.c
+++ b/src/ciphers/chacha/chacha_crypt.c
@@ -51,10 +51,11 @@ static void _chacha_block(unsigned char *output, const ulong32 *input, int round
    @param out     [out] The ciphertext (or plaintext), length inlen
    @return CRYPT_OK if successful
 */
-int chacha_crypt(chacha_state *st, const unsigned char *in, unsigned long inlen, unsigned char *out)
+int chacha_crypt(chacha_state *st, const void *pin, unsigned long inlen, unsigned char *out)
 {
    unsigned char buf[64];
    unsigned long i, j;
+   const unsigned char *in = pin;
 
    if (inlen == 0) return CRYPT_OK;
    if (st->ksleft > 0) {
diff --git a/src/ciphers/chacha/chacha_test.c b/src/ciphers/chacha/chacha_test.c
index d678e93..7218425 100644
--- a/src/ciphers/chacha/chacha_test.c
+++ b/src/ciphers/chacha/chacha_test.c
@@ -41,16 +41,16 @@ int chacha_test(void)
    /* crypt piece by piece */
    chacha_setup(&st, k, sizeof(k), 20);
    chacha_ivctr32(&st, n, sizeof(n), 1);
-   chacha_crypt(&st, (unsigned char*)pt,      35,       out);
-   chacha_crypt(&st, (unsigned char*)pt + 35, 35,       out + 35);
-   chacha_crypt(&st, (unsigned char*)pt + 70,  5,       out + 70);
-   chacha_crypt(&st, (unsigned char*)pt + 75,  5,       out + 75);
-   chacha_crypt(&st, (unsigned char*)pt + 80, len - 80, out + 80);
+   chacha_crypt(&st, pt,      35,       out);
+   chacha_crypt(&st, pt + 35, 35,       out + 35);
+   chacha_crypt(&st, pt + 70,  5,       out + 70);
+   chacha_crypt(&st, pt + 75,  5,       out + 75);
+   chacha_crypt(&st, pt + 80, len - 80, out + 80);
    if (compare_testvector(out, len, ct, sizeof(ct), "CHACHA-TV1", 1) != 0) return CRYPT_FAIL_TESTVECTOR;
    /* crypt in one go */
    chacha_setup(&st, k, sizeof(k), 20);
    chacha_ivctr32(&st, n, sizeof(n), 1);
-   chacha_crypt(&st, (unsigned char*)pt, len, out);
+   chacha_crypt(&st, pt, len, out);
    if (compare_testvector(out, len, ct, sizeof(ct), "CHACHA-TV2", 1) != 0) return CRYPT_FAIL_TESTVECTOR;
    /* crypt in one go - using chacha_ivctr64() */
    chacha_setup(&st, k, sizeof(k), 20);
diff --git a/src/headers/tomcrypt_cipher.h b/src/headers/tomcrypt_cipher.h
index f26115d..e38d680 100644
--- a/src/headers/tomcrypt_cipher.h
+++ b/src/headers/tomcrypt_cipher.h
@@ -819,7 +819,7 @@ typedef struct {
 int chacha_setup(chacha_state *st, const unsigned char *key, unsigned long keylen, int rounds);
 int chacha_ivctr32(chacha_state *st, const unsigned char *iv, unsigned long ivlen, ulong32 counter);
 int chacha_ivctr64(chacha_state *st, const unsigned char *iv, unsigned long ivlen, ulong64 counter);
-int chacha_crypt(chacha_state *st, const unsigned char *in, unsigned long inlen, unsigned char *out);
+int chacha_crypt(chacha_state *st, const void *pin, unsigned long inlen, unsigned char *out);
 int chacha_keystream(chacha_state *st, unsigned char *out, unsigned long *outlen);
 int chacha_test(void);
 #endif /* LTC_CHACHA */
1. as it's a stream cipher, would it make sense to integrate it in the prng environment as all the other stream ciphers? or should we think about integrating resp. how to integrate the stream ciphers in the crypt environment? or should we provide an alternative environment or re-think the entire approach? 2. what do you think about this? In my opinion we shouldn't introduce new algorithms with `unsigned char*` for in- or output data. ``` diff --git a/src/ciphers/chacha/chacha_crypt.c b/src/ciphers/chacha/chacha_crypt.c index 1ac3aa7..359e485 100644 --- a/src/ciphers/chacha/chacha_crypt.c +++ b/src/ciphers/chacha/chacha_crypt.c @@ -51,10 +51,11 @@ static void _chacha_block(unsigned char *output, const ulong32 *input, int round @param out [out] The ciphertext (or plaintext), length inlen @return CRYPT_OK if successful */ -int chacha_crypt(chacha_state *st, const unsigned char *in, unsigned long inlen, unsigned char *out) +int chacha_crypt(chacha_state *st, const void *pin, unsigned long inlen, unsigned char *out) { unsigned char buf[64]; unsigned long i, j; + const unsigned char *in = pin; if (inlen == 0) return CRYPT_OK; if (st->ksleft > 0) { diff --git a/src/ciphers/chacha/chacha_test.c b/src/ciphers/chacha/chacha_test.c index d678e93..7218425 100644 --- a/src/ciphers/chacha/chacha_test.c +++ b/src/ciphers/chacha/chacha_test.c @@ -41,16 +41,16 @@ int chacha_test(void) /* crypt piece by piece */ chacha_setup(&st, k, sizeof(k), 20); chacha_ivctr32(&st, n, sizeof(n), 1); - chacha_crypt(&st, (unsigned char*)pt, 35, out); - chacha_crypt(&st, (unsigned char*)pt + 35, 35, out + 35); - chacha_crypt(&st, (unsigned char*)pt + 70, 5, out + 70); - chacha_crypt(&st, (unsigned char*)pt + 75, 5, out + 75); - chacha_crypt(&st, (unsigned char*)pt + 80, len - 80, out + 80); + chacha_crypt(&st, pt, 35, out); + chacha_crypt(&st, pt + 35, 35, out + 35); + chacha_crypt(&st, pt + 70, 5, out + 70); + chacha_crypt(&st, pt + 75, 5, out + 75); + chacha_crypt(&st, pt + 80, len - 80, out + 80); if (compare_testvector(out, len, ct, sizeof(ct), "CHACHA-TV1", 1) != 0) return CRYPT_FAIL_TESTVECTOR; /* crypt in one go */ chacha_setup(&st, k, sizeof(k), 20); chacha_ivctr32(&st, n, sizeof(n), 1); - chacha_crypt(&st, (unsigned char*)pt, len, out); + chacha_crypt(&st, pt, len, out); if (compare_testvector(out, len, ct, sizeof(ct), "CHACHA-TV2", 1) != 0) return CRYPT_FAIL_TESTVECTOR; /* crypt in one go - using chacha_ivctr64() */ chacha_setup(&st, k, sizeof(k), 20); diff --git a/src/headers/tomcrypt_cipher.h b/src/headers/tomcrypt_cipher.h index f26115d..e38d680 100644 --- a/src/headers/tomcrypt_cipher.h +++ b/src/headers/tomcrypt_cipher.h @@ -819,7 +819,7 @@ typedef struct { int chacha_setup(chacha_state *st, const unsigned char *key, unsigned long keylen, int rounds); int chacha_ivctr32(chacha_state *st, const unsigned char *iv, unsigned long ivlen, ulong32 counter); int chacha_ivctr64(chacha_state *st, const unsigned char *iv, unsigned long ivlen, ulong64 counter); -int chacha_crypt(chacha_state *st, const unsigned char *in, unsigned long inlen, unsigned char *out); +int chacha_crypt(chacha_state *st, const void *pin, unsigned long inlen, unsigned char *out); int chacha_keystream(chacha_state *st, unsigned char *out, unsigned long *outlen); int chacha_test(void); #endif /* LTC_CHACHA */ ```
sjaeckel (Migrated from github.com) reviewed 2017-03-22 06:58:33 -05:00
src/headers/tomcrypt_custom.h
sjaeckel (Migrated from github.com) commented 2017-03-22 06:58:33 -05:00

I guess we should add a sanity check in the cleanup section

#if defined(LTC_CHACHA20POLY1305_MODE) && !defined(LTC_POLY1305) ...
I guess we should add a sanity check in the cleanup section ``` #if defined(LTC_CHACHA20POLY1305_MODE) && !defined(LTC_POLY1305) ... ```
sjaeckel (Migrated from github.com) reviewed 2017-03-22 08:27:27 -05:00
src/encauth/chachapoly/chacha20poly1305_done.c
@ -0,0 +1,42 @@
/* LibTomCrypt, modular cryptographic library -- Tom St Denis
sjaeckel (Migrated from github.com) commented 2017-03-22 08:27:27 -05:00

assignment please in a separate line

assignment please in a separate line
karel-m commented 2017-03-22 09:12:11 -05:00 (Migrated from github.com)

Ad unsigned char* vs. void * I am not against that change but I prefer to make a BIG SWITCH and change all such cases in the whole libtomcrypt. Not just in a single function.

Ad `unsigned char*` vs. `void *` I am not against that change but I prefer to make a BIG SWITCH and change all such cases in the whole libtomcrypt. Not just in a single function.
karel-m (Migrated from github.com) reviewed 2017-03-22 09:12:40 -05:00
src/headers/tomcrypt_custom.h
karel-m (Migrated from github.com) commented 2017-03-22 09:12:40 -05:00

I agree

I agree
karel-m (Migrated from github.com) reviewed 2017-03-22 09:12:45 -05:00
src/encauth/chachapoly/chacha20poly1305_done.c
@ -0,0 +1,42 @@
/* LibTomCrypt, modular cryptographic library -- Tom St Denis
karel-m (Migrated from github.com) commented 2017-03-22 09:12:45 -05:00

I agree

I agree
karel-m commented 2017-03-22 19:13:43 -05:00 (Migrated from github.com)

Ad moving chacha implementation under prng:
It would be probably logical to have all stream cipher in one place, however I am not a fan of the idea of having stream ciphers in prng in general. It is IMO very confusing for newcomers and frankly speaking if you want to use RC4 or SOBER128 for encrypting/decrypting your first impression might be that libtomcrypt is not supporting it (users will be looking for a simple rc4_setup(key) > rc4_crypt(in, out) > rc4_crypt(in, out) > ... interface which we do not have).

We can create src/stream and I can move src/cipher/chacha/* to src/stream/chacha/*

What I would like more than current state of RC4 + SOBER128

  • having src/stream/rc4.c and * having src/stream/sober128.c which will provide cipher_setup/cipher_crypt interface
  • and turning src/prngs/rc4.c and src/prngs/sober128.c into thin wrappers around the functions from previous point
Ad moving `chacha` implementation under `prng`: It would be probably logical to have all stream cipher in one place, however I am not a fan of the idea of having stream ciphers in `prng` in general. It is IMO very confusing for newcomers and frankly speaking if you want to use RC4 or SOBER128 for encrypting/decrypting your first impression might be that libtomcrypt is not supporting it (users will be looking for a simple `rc4_setup(key)` > `rc4_crypt(in, out)` > `rc4_crypt(in, out)` > `...` interface which we do not have). We can create `src/stream` and I can move `src/cipher/chacha/*` to `src/stream/chacha/*` What I would like more than current state of RC4 + SOBER128 * having `src/stream/rc4.c` and * having `src/stream/sober128.c` which will provide cipher_setup/cipher_crypt interface * and turning `src/prngs/rc4.c` and `src/prngs/sober128.c` into thin wrappers around the functions from previous point
👍 1
karel-m commented 2017-03-23 02:57:52 -05:00 (Migrated from github.com)

Let's move size_t discussion to #175 and unsigned char* vs. void * to #174 so that they do not block this PR.

Let's move `size_t` discussion to #175 and `unsigned char*` vs. `void *` to #174 so that they do not block this PR.
👍 2
karel-m commented 2017-03-23 08:38:09 -05:00 (Migrated from github.com)

We have the following:

/* by definition of poly1305 maclen is always 16 */
int poly1305_done(poly_state *st, unsigned char *mac, unsigned long *maclen);
int poly1305_memory(const unsigned char *key, unsigned long keylen, const unsigned char *in, unsigned long inlen, unsigned char *mac, unsigned long *maclen);
int poly1305_memory_multi(const unsigned char *key, unsigned long keylen, unsigned char *mac, unsigned long *maclen, const unsigned char *in,  unsigned long inlen, ...);
int poly1305_file(const char *fname, const unsigned char *key, unsigned long keylen, unsigned char *mac, unsigned long *maclen);

What's the best practice in these cases?

  1. int poly1305_done(poly_state *st, unsigned char *mac, unsigned long *maclen) ?
  2. int poly1305_done(poly_state *st, unsigned char *mac, unsigned long maclen) ?
  3. int poly1305_done(poly_state *st, unsigned char *mac) ?
  4. int poly1305_done(poly_state *st, unsigned char mac[16]) ?
We have the following: ```C /* by definition of poly1305 maclen is always 16 */ int poly1305_done(poly_state *st, unsigned char *mac, unsigned long *maclen); int poly1305_memory(const unsigned char *key, unsigned long keylen, const unsigned char *in, unsigned long inlen, unsigned char *mac, unsigned long *maclen); int poly1305_memory_multi(const unsigned char *key, unsigned long keylen, unsigned char *mac, unsigned long *maclen, const unsigned char *in, unsigned long inlen, ...); int poly1305_file(const char *fname, const unsigned char *key, unsigned long keylen, unsigned char *mac, unsigned long *maclen); ``` What's the best practice in these cases? 1. `int poly1305_done(poly_state *st, unsigned char *mac, unsigned long *maclen)` ? 1. `int poly1305_done(poly_state *st, unsigned char *mac, unsigned long maclen)` ? 1. `int poly1305_done(poly_state *st, unsigned char *mac)` ? 1. `int poly1305_done(poly_state *st, unsigned char mac[16])` ?
sjaeckel commented 2017-03-23 08:51:18 -05:00 (Migrated from github.com)

I'd say 1.

int poly1305_done(poly_state *st, unsigned char *mac, unsigned long *maclen)

also I'd prefer

  1. to use the established error codes CRYPT_INVALID_ARG for invalid buffer sizes like the maclen and CRYPT_INVALID_KEYSIZE in e.g. chacha_setup() et. al.

  2. to be less strict on these buffer sizes

so in poly1305_done() it'd become

if (*maclen < 16) return CRYPT_INVALID_ARG;
...
*maclen = 16;
return...

instead of

if (*maclen != 16) return CRYPT_ERROR;

I'd say 1. `int poly1305_done(poly_state *st, unsigned char *mac, unsigned long *maclen)` also I'd prefer 1. to use the established error codes `CRYPT_INVALID_ARG` for invalid buffer sizes like the `maclen` and `CRYPT_INVALID_KEYSIZE` in e.g. `chacha_setup()` et. al. 2. to be less strict on these buffer sizes so in `poly1305_done()` it'd become ``` if (*maclen < 16) return CRYPT_INVALID_ARG; ... *maclen = 16; return... ``` instead of `if (*maclen != 16) return CRYPT_ERROR;`
karel-m commented 2017-03-23 13:58:21 -05:00 (Migrated from github.com)

Ad unsigned long outlen vs. unsigned long *outlen there 3 more places I have found where we are not consistent (I'll create a separate issue)

Ad `unsigned long outlen` vs. `unsigned long *outlen` there 3 more places I have found where we are not consistent (I'll create a separate issue)
buggywhip commented 2017-03-24 01:34:38 -05:00 (Migrated from github.com)

On Mar 22, 2017, at 5:13 PM, karel-m notifications@github.com wrote:
Ad moving chacha implementation under prng:
It would be probably logical to have all stream cipher in one place, however I am not a fan of the idea of having stream ciphers in prng in general.

Agreed. IMO all stream ciphers would be better served under src/stream/, not under prng.

Many algorithms can be used in ways to produce output normally the province of other classes of algorithms, but without a formal ontology, a "typical"/"traditional" organization structure is probably best.

And it can get really messy trying to classify authenticated encryption. ...a unique classification by itself? ...a mode? Arrgghh. :-/

> On Mar 22, 2017, at 5:13 PM, karel-m <notifications@github.com> wrote: > Ad moving chacha implementation under prng: > It would be probably logical to have all stream cipher in one place, however I am not a fan of the idea of having stream ciphers in prng in general. > Agreed. IMO all stream ciphers would be better served under src/stream/, not under prng. Many algorithms can be used in ways to produce output normally the province of other classes of algorithms, but without a formal ontology, a "typical"/"traditional" organization structure is probably best. And it can get really messy trying to classify authenticated encryption. ...a unique classification by itself? ...a mode? Arrgghh. :-/
karel-m commented 2017-03-26 16:49:48 -05:00 (Migrated from github.com)

Rebased & squashed into 1 commit, travis-ci all green. I propose merging to develop.

Rebased & squashed into 1 commit, travis-ci all green. I propose merging to develop.
karel-m commented 2017-03-29 06:49:05 -05:00 (Migrated from github.com)

@sjaeckel do you (dis)agree with merging this one?

@sjaeckel do you (dis)agree with merging this one?
coveralls commented 2017-03-29 13:22:51 -05:00 (Migrated from github.com)

Coverage Status

Coverage decreased (-0.04%) to 87.44% when pulling 6ca293d6893b066f3396526d1a06b328e472ccb6 on pr/chacha20poly1305 into 4bcd48411a on develop.

[![Coverage Status](https://coveralls.io/builds/10833103/badge)](https://coveralls.io/builds/10833103) Coverage decreased (-0.04%) to 87.44% when pulling **6ca293d6893b066f3396526d1a06b328e472ccb6 on pr/chacha20poly1305** into **4bcd48411a050f99876057a96179acbc5a6d0e06 on develop**.
sjaeckel (Migrated from github.com) reviewed 2017-03-30 10:11:55 -05:00
sjaeckel (Migrated from github.com) left a comment

Can you please also add

diff --git a/src/ciphers/chacha/chacha_test.c b/src/ciphers/chacha/chacha_test.c
index 7417117..d678e93 100644
--- a/src/ciphers/chacha/chacha_test.c
+++ b/src/ciphers/chacha/chacha_test.c
@@ -52,6 +52,11 @@ int chacha_test(void)
    chacha_ivctr32(&st, n, sizeof(n), 1);
    chacha_crypt(&st, (unsigned char*)pt, len, out);
    if (compare_testvector(out, len, ct, sizeof(ct), "CHACHA-TV2", 1) != 0) return CRYPT_FAIL_TESTVECTOR;
+   /* crypt in one go - using chacha_ivctr64() */
+   chacha_setup(&st, k, sizeof(k), 20);
+   chacha_ivctr64(&st, n + 4, sizeof(n) - 4, 1);
+   chacha_crypt(&st, (unsigned char*)pt, len, out);
+   if (compare_testvector(out, len, ct, sizeof(ct), "CHACHA-TV3", 1) != 0) return CRYPT_FAIL_TESTVECTOR;
    return CRYPT_OK;
 #endif
 }
Can you please also add ``` diff --git a/src/ciphers/chacha/chacha_test.c b/src/ciphers/chacha/chacha_test.c index 7417117..d678e93 100644 --- a/src/ciphers/chacha/chacha_test.c +++ b/src/ciphers/chacha/chacha_test.c @@ -52,6 +52,11 @@ int chacha_test(void) chacha_ivctr32(&st, n, sizeof(n), 1); chacha_crypt(&st, (unsigned char*)pt, len, out); if (compare_testvector(out, len, ct, sizeof(ct), "CHACHA-TV2", 1) != 0) return CRYPT_FAIL_TESTVECTOR; + /* crypt in one go - using chacha_ivctr64() */ + chacha_setup(&st, k, sizeof(k), 20); + chacha_ivctr64(&st, n + 4, sizeof(n) - 4, 1); + chacha_crypt(&st, (unsigned char*)pt, len, out); + if (compare_testvector(out, len, ct, sizeof(ct), "CHACHA-TV3", 1) != 0) return CRYPT_FAIL_TESTVECTOR; return CRYPT_OK; #endif } ```
src/encauth/chachapoly/chacha20poly1305_decrypt.c
@ -0,0 +1,45 @@
/* LibTomCrypt, modular cryptographic library -- Tom St Denis
sjaeckel (Migrated from github.com) commented 2017-03-30 09:38:45 -05:00

assignment separate please

assignment separate please
src/encauth/chachapoly/chacha20poly1305_setiv_rfc7905.c
@ -0,0 +1,36 @@
/* LibTomCrypt, modular cryptographic library -- Tom St Denis
sjaeckel (Migrated from github.com) commented 2017-03-30 09:40:53 -05:00

can we probably test this as well?

can we probably test this as well?
src/headers/tomcrypt_mac.h
@ -96,6 +96,26 @@ void pmac_shift_xor(pmac_state *pmac);
sjaeckel (Migrated from github.com) commented 2017-03-30 09:42:11 -05:00

I'd prefer poly1305_state

I'd prefer `poly1305_state`
src/headers/tomcrypt_mac.h
@ -477,6 +497,36 @@ int f9_test(void);
sjaeckel (Migrated from github.com) commented 2017-03-30 09:42:42 -05:00

also chacha20poly1305_state

also `chacha20poly1305_state`
src/headers/tomcrypt_prng.h
@ -15,6 +15,15 @@ struct rc4_prng {
};
sjaeckel (Migrated from github.com) commented 2017-03-30 09:43:43 -05:00

chacha20_prng

`chacha20_prng`
src/headers/tomcrypt_prng.h
@ -55,6 +64,9 @@ typedef union Prng_state {
#ifdef LTC_RC4
sjaeckel (Migrated from github.com) commented 2017-03-30 09:44:26 -05:00

how about having a separate define LTC_CHACHA20_PRNG?

how about having a separate define `LTC_CHACHA20_PRNG`?
src/mac/poly1305/poly1305_file.c
@ -0,0 +1,70 @@
/* LibTomCrypt, modular cryptographic library -- Tom St Denis
sjaeckel (Migrated from github.com) commented 2017-03-30 09:51:47 -05:00

should we make this 8196 somehow configurable/better?

by better I mean "can we easily get the pagesize of the running arch?" or similar

should we make this 8196 somehow configurable/better? by better I mean "can we easily get the pagesize of the running arch?" or similar
src/prngs/chacha.c
@ -0,0 +1,219 @@
/* LibTomCrypt, modular cryptographic library -- Tom St Denis
sjaeckel (Migrated from github.com) commented 2017-03-30 10:06:03 -05:00

Did you follow some guide "how to use ChaCha as a prng" or is this all your work?

From my point of view it looks very good, but if you created this I'd like to have someone else with crypto expertise to review this code.

Did you follow some guide "how to use ChaCha as a prng" or is this all your work? From my point of view it looks very good, but if you created this I'd like to have someone else with crypto expertise to review this code.
karel-m commented 2017-03-30 10:25:13 -05:00 (Migrated from github.com)

AFAIK the only unusual magic here is in chacha_prng_add_entropy, either initially (before make_ready) and later (kind of rekey operation).

The implementation (namely the rekey operation) is based on chacha-based arc4random implemented in OpenBSD/FreeBSD (I do not have a link handy but you can easily google it).

AFAIK the only unusual magic here is in `chacha_prng_add_entropy`, either initially (before make_ready) and later (kind of rekey operation). The implementation (namely the rekey operation) is based on chacha-based arc4random implemented in OpenBSD/FreeBSD (I do not have a link handy but you can easily google it).
karel-m commented 2017-03-30 10:30:33 -05:00 (Migrated from github.com)

Seeing:

diff --git a/src/ciphers/chacha/chacha_test.c b/src/ciphers/chacha/chacha_test.c

I am in doubts whether you have pulled (you might need pull --force) the latest version as I have moved src/ciphers/chacha to src/stream/chacha.

Seeing: ``` diff --git a/src/ciphers/chacha/chacha_test.c b/src/ciphers/chacha/chacha_test.c ``` I am in doubts whether you have pulled (you might need `pull --force`) the latest version as I have moved `src/ciphers/chacha` to `src/stream/chacha`.
sjaeckel commented 2017-03-30 10:37:47 -05:00 (Migrated from github.com)

I am in doubts whether you have pulled (you might need pull --force) the latest version as I have moved src/ciphers/chacha to src/stream/chacha.

simply s/ciphers/stream/g in the diff and applying the patch is successful

> I am in doubts whether you have pulled (you might need pull --force) the latest version as I have moved src/ciphers/chacha to src/stream/chacha. simply `s/ciphers/stream/g` in the diff and applying the patch is successful
sjaeckel (Migrated from github.com) reviewed 2017-03-30 10:40:39 -05:00
src/stream/chacha/chacha_test.c
@ -0,0 +21,4 @@
#ifndef LTC_TEST
return CRYPT_NOP;
#else
unsigned long len;
sjaeckel (Migrated from github.com) commented 2017-03-30 10:40:39 -05:00

why should we duplicate the same test-result?

why should we duplicate the same test-result?
karel-m (Migrated from github.com) reviewed 2017-03-30 10:42:24 -05:00
src/encauth/chachapoly/chacha20poly1305_decrypt.c
@ -0,0 +1,45 @@
/* LibTomCrypt, modular cryptographic library -- Tom St Denis
karel-m (Migrated from github.com) commented 2017-03-30 10:42:24 -05:00

done

done
karel-m (Migrated from github.com) reviewed 2017-03-30 10:45:19 -05:00
src/encauth/chachapoly/chacha20poly1305_setiv_rfc7905.c
@ -0,0 +1,36 @@
/* LibTomCrypt, modular cryptographic library -- Tom St Denis
karel-m (Migrated from github.com) commented 2017-03-30 10:45:19 -05:00

and perhaps also chacha20poly1305_memory

and perhaps also `chacha20poly1305_memory`
sjaeckel (Migrated from github.com) reviewed 2017-03-30 10:56:28 -05:00
src/encauth/chachapoly/chacha20poly1305_setiv_rfc7905.c
@ -0,0 +1,36 @@
/* LibTomCrypt, modular cryptographic library -- Tom St Denis
sjaeckel (Migrated from github.com) commented 2017-03-30 10:56:28 -05:00

If you're upto it, sure 👍

If you're upto it, sure :+1:
karel-m (Migrated from github.com) reviewed 2017-03-30 13:23:02 -05:00
src/encauth/chachapoly/chacha20poly1305_setiv_rfc7905.c
@ -0,0 +1,36 @@
/* LibTomCrypt, modular cryptographic library -- Tom St Denis
karel-m (Migrated from github.com) commented 2017-03-30 13:23:02 -05:00

done

done
karel-m (Migrated from github.com) reviewed 2017-03-30 13:32:55 -05:00
src/headers/tomcrypt_mac.h
@ -96,6 +96,26 @@ void pmac_shift_xor(pmac_state *pmac);
karel-m (Migrated from github.com) commented 2017-03-30 13:32:55 -05:00

done

done
karel-m (Migrated from github.com) reviewed 2017-03-30 13:33:00 -05:00
src/headers/tomcrypt_mac.h
@ -477,6 +497,36 @@ int f9_test(void);
karel-m (Migrated from github.com) commented 2017-03-30 13:33:00 -05:00

done

done
karel-m (Migrated from github.com) reviewed 2017-03-30 13:33:06 -05:00
src/headers/tomcrypt_prng.h
@ -15,6 +15,15 @@ struct rc4_prng {
};
karel-m (Migrated from github.com) commented 2017-03-30 13:33:06 -05:00

done

done
karel-m (Migrated from github.com) reviewed 2017-03-30 13:44:54 -05:00
src/headers/tomcrypt_prng.h
@ -55,6 +64,9 @@ typedef union Prng_state {
#ifdef LTC_RC4
karel-m (Migrated from github.com) commented 2017-03-30 13:44:54 -05:00

done

done
coveralls commented 2017-03-30 13:45:07 -05:00 (Migrated from github.com)

Coverage Status

Coverage increased (+0.2%) to 87.666% when pulling e969a10bd92410c34d5ef134b5e4f61963af700d on pr/chacha20poly1305 into 4bcd48411a on develop.

[![Coverage Status](https://coveralls.io/builds/10854773/badge)](https://coveralls.io/builds/10854773) Coverage increased (+0.2%) to 87.666% when pulling **e969a10bd92410c34d5ef134b5e4f61963af700d on pr/chacha20poly1305** into **4bcd48411a050f99876057a96179acbc5a6d0e06 on develop**.
🎉 1
karel-m (Migrated from github.com) reviewed 2017-03-30 13:48:31 -05:00
src/mac/poly1305/poly1305_file.c
@ -0,0 +1,70 @@
/* LibTomCrypt, modular cryptographic library -- Tom St Denis
karel-m (Migrated from github.com) commented 2017-03-30 13:48:31 -05:00

Like

#define FILE_READ_BUFSIZE 8192

somewhere in tomcrypt_custom.h ?

Like ```C #define FILE_READ_BUFSIZE 8192 ``` somewhere in `tomcrypt_custom.h `?
karel-m (Migrated from github.com) reviewed 2017-03-30 13:50:37 -05:00
src/prngs/chacha.c
@ -0,0 +1,219 @@
/* LibTomCrypt, modular cryptographic library -- Tom St Denis
karel-m (Migrated from github.com) commented 2017-03-30 13:50:37 -05:00

A review would be great.

Should I create a separate issue like Call for reviewers - PRNG/ChaCha20 ?

A review would be great. Should I create a separate issue like `Call for reviewers - PRNG/ChaCha20` ?
karel-m (Migrated from github.com) reviewed 2017-03-30 13:51:21 -05:00
src/headers/tomcrypt_custom.h
karel-m (Migrated from github.com) commented 2017-03-30 13:51:21 -05:00

done

done
karel-m (Migrated from github.com) reviewed 2017-03-30 14:23:39 -05:00
src/mac/poly1305/poly1305_file.c
@ -0,0 +1,70 @@
/* LibTomCrypt, modular cryptographic library -- Tom St Denis
karel-m (Migrated from github.com) commented 2017-03-30 14:23:39 -05:00

done cd5e4e8

done cd5e4e8
sjaeckel (Migrated from github.com) reviewed 2017-03-30 15:06:07 -05:00
src/headers/tomcrypt_custom.h
@ -535,7 +550,12 @@
sjaeckel (Migrated from github.com) commented 2017-03-30 15:06:07 -05:00 
#ifndef LTC_FILE_READ_BUFSIZE
#define LTC_FILE_READ_BUFSIZE 8192
#endif
``` #ifndef LTC_FILE_READ_BUFSIZE #define LTC_FILE_READ_BUFSIZE 8192 #endif ```
coveralls commented 2017-03-30 15:08:50 -05:00 (Migrated from github.com)

Coverage Status

Coverage increased (+0.2%) to 87.666% when pulling cd5e4e8b83435c81a706048dbb0711909b2afc26 on pr/chacha20poly1305 into 4bcd48411a on develop.

[![Coverage Status](https://coveralls.io/builds/10856050/badge)](https://coveralls.io/builds/10856050) Coverage increased (+0.2%) to 87.666% when pulling **cd5e4e8b83435c81a706048dbb0711909b2afc26 on pr/chacha20poly1305** into **4bcd48411a050f99876057a96179acbc5a6d0e06 on develop**.
karel-m (Migrated from github.com) reviewed 2017-03-30 15:20:23 -05:00
src/headers/tomcrypt_custom.h
@ -535,7 +550,12 @@
karel-m (Migrated from github.com) commented 2017-03-30 15:20:23 -05:00

done e57ca44

done e57ca44
karel-m commented 2017-03-30 15:22:41 -05:00 (Migrated from github.com)

BTW do we have a fixed file which I can use as an input in a test for poly1305_file(..)?

BTW do we have a fixed file which I can use as an input in a test for `poly1305_file(..)`?
sjaeckel commented 2017-03-30 15:27:32 -05:00 (Migrated from github.com)

BTW do we have a fixed file which I can use as an input in a test for poly1305_file(..)?

we don't have tests for any of the ***_file() API's... we can leave it like that I'd say... or use one of the testvector files?

> BTW do we have a fixed file which I can use as an input in a test for `poly1305_file(..)`? we don't have tests for any of the `***_file()` API's... we can leave it like that I'd say... or use one of the testvector files?
karel-m commented 2017-03-30 15:32:08 -05:00 (Migrated from github.com)

test vector files are sometimes changing (see sha3)

I was thinking about creating let's say testprof/fixed-test-file.txt with 10000 bytes of random text.

test vector files are sometimes changing (see sha3) I was thinking about creating let's say `testprof/fixed-test-file.txt` with 10000 bytes of random text.
karel-m commented 2017-03-30 15:35:16 -05:00 (Migrated from github.com)

You are perhaps right the *_file functions need a facelift anyway (to use the same read buffer style) which is worth a separate PR where we can add more tests as well.

You are perhaps right the *_file functions need a facelift anyway (to use the same read buffer style) which is worth a separate PR where we can add more tests as well.
coveralls commented 2017-03-30 16:03:38 -05:00 (Migrated from github.com)

Coverage Status

Coverage increased (+0.2%) to 87.666% when pulling e57ca441cdeb4dc6b5b5033a07630d05ec7f7efd on pr/chacha20poly1305 into 4bcd48411a on develop.

[![Coverage Status](https://coveralls.io/builds/10857071/badge)](https://coveralls.io/builds/10857071) Coverage increased (+0.2%) to 87.666% when pulling **e57ca441cdeb4dc6b5b5033a07630d05ec7f7efd on pr/chacha20poly1305** into **4bcd48411a050f99876057a96179acbc5a6d0e06 on develop**.
coveralls commented 2017-03-30 17:16:22 -05:00 (Migrated from github.com)

Coverage Status

Coverage increased (+0.2%) to 87.739% when pulling b6648e5218877edf3becad2808907642a6d0fb77 on pr/chacha20poly1305 into 77019928dd on develop.

[![Coverage Status](https://coveralls.io/builds/10858541/badge)](https://coveralls.io/builds/10858541) Coverage increased (+0.2%) to 87.739% when pulling **b6648e5218877edf3becad2808907642a6d0fb77 on pr/chacha20poly1305** into **77019928ddd2dd810601b33db045722f312b8618 on develop**.
coveralls commented 2017-03-31 01:04:58 -05:00 (Migrated from github.com)

Coverage Status

Coverage increased (+0.2%) to 87.739% when pulling 5b40b62c0229631cf5e3d0f338c12ff73f72c528 on pr/chacha20poly1305 into 77019928dd on develop.

[![Coverage Status](https://coveralls.io/builds/10862933/badge)](https://coveralls.io/builds/10862933) Coverage increased (+0.2%) to 87.739% when pulling **5b40b62c0229631cf5e3d0f338c12ff73f72c528 on pr/chacha20poly1305** into **77019928ddd2dd810601b33db045722f312b8618 on develop**.
karel-m commented 2017-04-02 11:38:29 -05:00 (Migrated from github.com)

@sjaeckel can I merge this PR?

@sjaeckel can I merge this PR?
coveralls commented 2017-04-02 11:43:09 -05:00 (Migrated from github.com)

Coverage Status

Coverage increased (+0.2%) to 87.799% when pulling 08a028ab04 on pr/chacha20poly1305 into 6844275e82 on develop.

[![Coverage Status](https://coveralls.io/builds/10892542/badge)](https://coveralls.io/builds/10892542) Coverage increased (+0.2%) to 87.799% when pulling **08a028ab04eb854c2d1a722de4266e941535baae on pr/chacha20poly1305** into **6844275e82d2da20973c2a169e86cd29e19e7619 on develop**.
sjaeckel (Migrated from github.com) approved these changes 2017-04-03 02:03:16 -05:00
sjaeckel (Migrated from github.com) left a comment

Looks like I missed these in the first review... sorry :)

After these changes I think it's fine to merge

Looks like I missed these in the first review... sorry :) After these changes I think it's fine to merge
src/mac/poly1305/poly1305_file.c
@ -0,0 +1,70 @@
/* LibTomCrypt, modular cryptographic library -- Tom St Denis
sjaeckel (Migrated from github.com) commented 2017-04-03 01:55:50 -05:00

sizeof(buf) is either 4 or 8... :)

`sizeof(buf)` is either 4 or 8... :)
src/prngs/chacha.c
@ -0,0 +1,219 @@
/* LibTomCrypt, modular cryptographic library -- Tom St Denis
sjaeckel (Migrated from github.com) commented 2017-04-03 01:50:00 -05:00

I'd say we should also rename all these occurrences to chacha20_prng***

I'd say we should also rename all these occurrences to `chacha20_prng***`
sjaeckel (Migrated from github.com) commented 2017-04-03 01:56:58 -05:00

Sure, can you then please also link the existing implementation(s).

Sure, can you then please also link the existing implementation(s).
karel-m (Migrated from github.com) reviewed 2017-04-03 12:52:17 -05:00
src/prngs/chacha.c
@ -0,0 +1,219 @@
/* LibTomCrypt, modular cryptographic library -- Tom St Denis
karel-m (Migrated from github.com) commented 2017-04-03 12:52:17 -05:00

fixed ce37498

fixed ce37498
karel-m (Migrated from github.com) reviewed 2017-04-03 12:55:07 -05:00
src/mac/poly1305/poly1305_file.c
@ -0,0 +1,70 @@
/* LibTomCrypt, modular cryptographic library -- Tom St Denis
karel-m (Migrated from github.com) commented 2017-04-03 12:55:07 -05:00

OMG, fixed 7b93f04

OMG, fixed 7b93f04
coveralls commented 2017-04-03 14:11:35 -05:00 (Migrated from github.com)

Coverage Status

Coverage increased (+0.2%) to 87.799% when pulling 2520e6c061 on pr/chacha20poly1305 into 6844275e82 on develop.

[![Coverage Status](https://coveralls.io/builds/10907758/badge)](https://coveralls.io/builds/10907758) Coverage increased (+0.2%) to 87.799% when pulling **2520e6c0618d561abc64e1df875af7e56d8aa021 on pr/chacha20poly1305** into **6844275e82d2da20973c2a169e86cd29e19e7619 on develop**.
Sign in to join this conversation.
Reviewers
No reviewers
sjaeckel
Labels
Clear labels   
bug

   
duplicate

   
enhancement

   
feature

   
help wanted

   
improve API

   
invalid

   
needs documentation

   
not-planned
This ticket is not planned to be implemented and/or fixed from the core team.

  
rtfm

   
stale

   
support

   
wishlist
No labels
bug
duplicate
enhancement
feature
help wanted
improve API
invalid
needs documentation
not-planned
rtfm
stale
support
wishlist
Milestone
Clear milestone
No items
No milestone
v1.18.0
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-libtom/libtomcrypt!171
No description provided.