ctz / cifra Goto Github PK

Hi, thanks for this great library!
I recently became aware of https://github.com/BjoernMHaase/fe25519 (public domain) and https://github.com/hayatofujii/curve25519-cortex-m4 (unfortunately not open source licensed).
Would be interesting to see comparisons, particularly of the first implementation, with those you already import, using your benchmark :)

Link in "More measurements are available for AEAD modes on my blog post: Benchmarking Modern Authenticated Encryption on €1 devices." shows 404

Hi, I went through the implementation of the Salsa20 algorithm and I found out, that the table nonce (16 bytes long) is written to the matrix on positions z6, z7, z8 and z9 (in cf_salsa20_core() ):

           x6 = z6 = read32_le(nonce + 0),
           x7 = z7 = read32_le(nonce + 4),
           x8 = z8 = read32_le(nonce + 8),
           x9 = z9 = read32_le(nonce + 12),

The first two parts (z6 and z7) are set to zero during the initialization and z8 and z9 contain the nonce from the input:

         memset(ctx->nonce, 0, sizeof ctx->nonce);
         memcpy(ctx->nonce + 8, nonce, 8);

Later in the algorithm positions z6 and z7 are incremented every 64-byte block of the encrypted message:

         incr_le(ctx->nonce, ctx->ncounter);

So as I understand z6 and z7 contain the 64-bit counter and z8 and z9 contain the actual nonce.

The issue I found is that the documentation of Salsa20 says that the 64-bit counter is supposed to be on positions z8 and z9, and the 8-byte nonce on positions z6 and z7. So to me it looks like the positions are switched and the algorithm is not compatible with the standard.
Refs:
https://en.wikipedia.org/wiki/Salsa20
http://www.ecrypt.eu.org/stream/papersdir/2007/010.pdf

If the same algorithm is used on both sides, the message is encrypted and decrypted correctly. However, the problems may occur if the message encrypted with Cifra implementation is decrypted with some other implementation that handles the nonce/pos as it is described in the documents.

Hi,
The Cifra library produces an different TAG when AES-EAX encoding is used when no Header is set (length is 0 bytes) then other two souces: online tool and Bouncy Castle impl in Java. If Header is set to any non 0 data they all give the same result. See details below.

I'm encrypting a message using AES-EAX with the following data (all in HEX):

Plain data:
0x00001C400000004800000073

Key:
0x589417B0324B1B71D7A6751852867AE8

Nonce:
0x00010000F683

Header:
0 bytes

I get a correct cipher and, in my opinion, not correct TAG.

Cipher:
0xD5D89979AE79EBEE4E385FA5

Tag:
0x46A9F4BE8F4C92659DA6CD12368D8127

Listing from the app:

When I use the same input data and encrypt them using this: http://artjomb.github.io/cryptojs-extension/ website, or in Java:

try {
    cipher = Cipher.getInstance("AES/EAX/NoPadding", "SC"); // SC = Spongy Castle, an Android variation of Bouncy Castle
    cipher.init(Cipher.ENCRYPT_MODE, keySpec, new GCMParameterSpec(128, nonce));
    ret = cipher.doFinal(toBeEncrypted);
    Log.d("AA2", "Encoded->" + Arrays.toString(ret));
} catch (Exception e) {
    Log.e("AA2", e.getLocalizedMessage());
}

I get a different Tag (the same in both cases):
0x0EFB21FAD714A25B44145F79221A2C9A

However, if I set the Header to any(?) non-null data all 3 libraries give the same result:

Plain text, key and nonce as above

Header:
0x0123

Cipher (same as above)

Tag:
0xCCDA2E4E0698E24E0377E3CD3ED61391

Online tool:

I was poking through the chacha20 implementation and noticed that the key is copied into the cf_chacha20_ctx struct at key0 and key1

or

I am trying to avoid copying the key into RAM as the MCU protection level I'm implementing doesn't stop a debugger reading out RAM, but does prevent reading out flash. I can't go to the next level as this will disable field upgrades and prevent me from fixing security bugs.

If the key0 and key1 were pointers, instead of copying the key, this would prevent a debugger gaining access to the key, they would only be able to access the address, with the positive side effect of reducing RAM. I can't see any reason why this would break the code otherwise, but it will have an impact on users of the library that rely on this copy.

I should be able to manage a pull request, but would a change like this be accepted? Or is there some issue I'm over looking?

I've been working from an older snapshot of your code and found a serious bug. The version I have returns the wrong hash (SHA256) for inputs of length 55, 119 and probably any: len%64 == 55

It was a case of calculating the padding incorrectly for the final block. The bug has been fixed already in your commit d62aa26 so no action is needed.

However, it's such a serious bug and cost me a lot of time, so I respectfully suggest you add more test vectors that check different inputs lengths, in the full range from 0 to 128 or so. It's so easy to make test vectors for a library like this.

It might also be appropriate to warn users that versions before Sept 15, 2015 will return incorrect results for about 1.5% of variable-length inputs.

Thanks!

Are you planning any? Or recommend any? :)
If not, I will give it a try; my use case is extending the firmware for our open source FIDO2 key called Solo, which runs on an STM32L432.

My project (https://github.com/guidovranken/cryptofuzz) fuzzers cryptographic libraries and it has support for cifra.

We can run a cifra fuzzer on OSS-Fuzz (https://github.com/google/oss-fuzz) but this requires that the project is being maintained. Is this the case?

I am using void cf_chacha20poly1305_encrypt(const uint8_t key[32], const uint8_t nonce[12], const uint8_t *header, size_t nheader, const uint8_t *plaintext, size_t nbytes, uint8_t *ciphertext, uint8_t tag[16]) from cifra and
int crypto_aead_chacha20poly1305_ietf_encrypt_detached(unsigned char *c, unsigned char *mac, unsigned long long *maclen_p, const unsigned char *m, unsigned long long mlen, const unsigned char *ad, unsigned long long adlen, const unsigned char *nsec, const unsigned char *npub, const unsigned char *k); from libsodium. Both of them are suppossed to produce the same ciphertext which they are doing but the tag is coming out to be different. I am not able to understand why this is happening. Please help.

I have a problem with building the ARM version.

When I run the make command, it fails building:

arm-none-eabi-gcc -I./ext -I../ext -I.. -Os -ffunction-sections -g -Wall -Werror -std=gnu99 -mthumb  -nostartfiles -nostdlib -Wl,-gc-sections -T linkscript.stm32f0.ld -mcpu=cortex-m0 -DCORTEX_M0 -o testmodes.stm32f0.elf boot.c memcpy.s memset.s semihost.c semihost.s ../sha1.c ../sha256.c ../sha512.c ../sha3.c ../blockwise.c ../chash.c ../curve25519.c ../poly1305.c ../aes.c ../eax.c ../gcm.c ../cbcmac.c ../ccm.c ../modes.c ../cmac.c ../gf128.c ../hmac.c ../pbkdf2.c ../salsa20.c ../chacha20.c ../norx.c ../chacha20poly1305.c ../drbg.c ../testmodes.c -DTEST=testmodes -lgcc
/tmp/ccJ3ea3q.o: In function `check_ocb':
/root/cifra/src/arm/../testmodes.c:631: undefined reference to `cf_ocb_encrypt'
/root/cifra/src/arm/../testmodes.c:643: undefined reference to `cf_ocb_decrypt'
/root/cifra/src/arm/../testmodes.c:654: undefined reference to `cf_ocb_decrypt'
collect2: error: ld returned 1 exit status
make: *** [testmodes.stm32f0.elf] Error 1

I didn't make ANY changes to the repository.

Please respond ASAP.

As I started using this library in a C++ project I got some surprising error messages at first about unresolved symbols. Turns out that the compiler assumed the Cifra functions were C++ and would be name-mangled accordingly, but of course the implementation files are all C instead and so it wasn't finding the functions under the right names.

It would be convenient if this project added the usual #ifdef __cplusplus boilerplate to each header meant to be part of the public API. See https://isocpp.org/wiki/faq/mixing-c-and-cpp#include-c-hdrs-personal for some discussion.

Meanwhile the workaround isn't so bad but may not be obvious at first. Instead of e.g.

#include <lib/cifra/src/sha2.h>

wrap the #include itself in the block when including anything from cifra, which in the end does the same thing as if they were in the headers:

extern "C" {
    #include <lib/cifra/src/sha2.h>
}

Note this simple wrapping assumes a header file used only from C++. For a header file that itself saw mixed usage it would need the full boilerplate:

#ifdef __cplusplus
  extern "C" {
#endif
    #include <lib/cifra/src/sha2.h>
#ifdef __cplusplus
  }
#endif

Hello.
It seems that there is a signed integer overflow in bitops.h:54 :

static inline uint32_t read32_be(const uint8_t buf[4])
{
*  return (buf[0] << 24) |
         (buf[1] << 16) |
         (buf[2] << 8) |
         (buf[3]);
}

It could be triggered by:

const uint8_t key[] =   {0xd9, 0x63, 0xca, 0xf9} ;
const uint8_t msg[] =  {0xb8, 0x02, 0x6b, 0xa9};
cf_hmac_init(&ctx, &cf_sha256, key, sizeof(key));
cf_hmac_update(&ctx, msg, sizeof(msg));

Hello,
In cf_hmac_init() key k and block blk is allocated with CF_CHASH_MAXBLK (144 bytes). However, for SHA3HMAC and SHAKE-128 use block size up to 168 bytes.
With change of macro CF_CHASH_MAXBLK to 168, we can HMAC for these algorithms.

Hello and kudos for the awesome project.

While investigating a bug report in our platform we think we have found a bug in the hmac implementation.

cf_hmac_init uses temporary buffers k and blk that are initialized at a size of CF_CHASH_MAXBLK=128. However, the block sizes for sha3-224 and sha3-256 are 144 and 136, causing a buffer overflow on the memset at hmac.c:53

A simple fix is setting CF_CHASH_MAXBLK at 144

In line 44 of cifra/src/hmac.c, there is an assert(nkey <= hash->blocksz) because that case is not covered by the standard.

It kind-of depends which standard. 😄

While that case is not covered in RFC-2104, it is covered in FIPS-198-1. (Basically, you just zero-pad, as you would naïvely expect.)

Sorry for no patch, but I wrote my own C++ code and came upon your (awesome!) repo while hunting around.

Cheers!

aes-gcm supports one-shot interface at the moment:
cf_gcm_encrypt(...)
cf_gcm_decrypt(...)

It would be handy to have also incremental interface implemented.