Tiny Encryption Algorithm implementation in C

Question

I was just looking for a general feedback about this code, which encrypts a file with the Tiny Encryption Algorithm. I'm currently studying C, and I wanted to know if there was anything I could improve in my coding style or anything. Sorry for my english.

#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>
#include <string.h>
#include <errno.h>
#include <sys/stat.h>
#include <fcntl.h>

#define BASE_DELTA 0x9e3779b9
#define KEY_VALUE 0x7A24432646294A404E635266556A586E //128bits value


void encrypt (u_int32_t* v, u_int32_t* k) {
  u_int32_t delta = BASE_DELTA * 1;
  for (int i = 1; i <= 32; i++) {
    v[0] += (v[1] * delta) ^ ((v[1] << 4) + k[0]) ^ ((v[1] >> 5) + k[1]);
    v[1] += (v[0] * delta) ^ ((v[0] << 4) + k[2]) ^ ((v[0] >> 5) + k[3]);
    delta += BASE_DELTA;
  }
}

void encryptFile(const char file_to_encrypt_path[], u_int32_t key[4]){

  u_int32_t block[2];

  int nb_bytes_read = 0;
  int fd_to_encrypt = open(file_to_encrypt_path, O_RDONLY, 0655); //RDONLY permissions are 0655
  int fd_result_file = open("encrypted_file.dec", O_CREAT|O_TRUNC|O_WRONLY, 0666);


  while(1){
    memset(block, 0x0, sizeof(u_int64_t));

    nb_bytes_read = read(fd_to_encrypt, block, sizeof(u_int64_t));
    if(nb_bytes_read == -1){
      perror("read");
      exit(errno);
    }
    if(nb_bytes_read == 0){
      break; //EOF
    }
    encrypt(block, key);

    if(write(fd_result_file, block, sizeof(u_int64_t)) ==-1){
      perror("write");
      exit(errno);
    }
  }
}



int main(int argc, char const *argv[]) {
  if (argc < 2){
        printf("usage : %s <file to encrypt> \n", argv[0]);
        exit(EXIT_FAILURE);
  }

  u_int32_t key[4];
  u_int32_t block[2];
  memset(key, 0x0, sizeof(u_int32_t) * 4);
  memset(block, 0x0, sizeof(u_int32_t) * 2);

  __uint128_t key_val = KEY_VALUE; //Note that __uint128_t requiers gcc version > 4.4
  *key = key_val;


  encryptFile(argv[1], key);

  return 0;
}

Answer 1

Good formatting and layout.

Aside from bug, good code.

Bug?

I'd expect v[1] + delta, v[0] + delta per Tiny Encryption Algorithm.

// Reference code
// for (i=0; i < 32; i++) {                       /* basic cycle start */
//     sum += delta;
//     v0 += ((v1<<4) + k0) ^ (v1 + sum) ^ ((v1>>5) + k1);
//     v1 += ((v0<<4) + k2) ^ (v0 + sum) ^ ((v0>>5) + k3);
// }  

  for (int i = 1; i <= 32; i++) {
    //            v--- ???
    v[0] += (v[1] * delta) ^ ((v[1] << 4) + k[0]) ^ ((v[1] >> 5) + k[1]);
    v[1] += (v[0] * delta) ^ ((v[0] << 4) + k[2]) ^ ((v[0] >> 5) + k[3]);
    delta += BASE_DELTA;
  }

Questionable sample code.

*key = key_val; only sets the first element of key[4], Certainly the below functionality was desired.

// *key = key_val;
key[0] = key_val >> 0;
key[1] = key_val >> 32;
key[2] = key_val >> 64;
key[3] = key_val >> 96;

Use standard types

Use standard types from #include <stdint.h> like uint32_t instead of u_int32_t. They are better known and more portable.

---

Minor

const

Use const as able to convey code's intent.

// void encrypt(u_int32_t* v, u_int32_t* k) {
void encrypt(u_int32_t* v, const u_int32_t* k) {

// void encryptFile(const char file_to_encrypt_path[], u_int32_t key[4]) {
void encryptFile(const char file_to_encrypt_path[], const u_int32_t key[4]) {

Make constants unsigned

Make constants unsigned for unsigned variables by adding u.

// #define BASE_DELTA 0x9e3779b9
#define BASE_DELTA 0x9e3779b9u

Unneeded *1

Perhaps there is some reason for this, - I do not see it.

// u_int32_t delta = BASE_DELTA * 1;
u_int32_t delta = BASE_DELTA;

Debug code?

Clearing the block serves little aside from cleaner debug. Don't use a type unrelated to the variable.

// memset(block, 0x0, sizeof(u_int64_t));
memset(block, 0x0, sizeof block);

0 base

Starting from 0 is more C-ish, yet I have no real issue with OP's code.

// for (int i = 1; i <= 32; i++) {
for (int i = 0; i < 32; i++) {

Avoid naked magic numbers

TEA uses 64 rounds, nice to self-document that in code

#define TEA_ROUNDS 64
for (int i = 0; i < TEA_ROUNDS/2; i++) {

open() test, close()?

Missing tests if open() success.
Missing close(), x2, in encryptFile().

---

Advanced

restrict

Use restrict (C99) to let encrypt() know that v, k do not overlap. This allows for some optimizations.

// void encrypt(u_int32_t* v, u_int32_t* k) {
void encrypt(u_int32_t* restrict v, const u_int32_t* restrict k) {

Out of range constant.

0x7A24432646294A404E635266556A586E is out of range of most compilers, even when __uint128_t exists.

Portable code would use a different way to set the key as well and a different type. Good that it work for you.