Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

The TEA is a very simple encryption algorithm requiring little time and space - perfect for embedded systems. There are extensions to it, and every version has its flaws (WEP was based on it), but for casual protection it's perfect.

Also, the encrypt and decrypt routines are exactly as found from Wikipedia, so I don't expect anyone to check that I'm following the TEA correctly - I'm more interested in the block routines I've added, though if you notice something about the TEA routines it would be good to know.

tea.h

#ifndef __TEA.H__
#define __TEA.H__

#include <stdint.h>

void encrypt (uint32_t* v, uint32_t* k);
void decrypt (uint32_t* v, uint32_t* k);
void encryptBlock(uint8_t * data, uint32_t * len, uint32_t * key);
void decryptBlock(uint8_t * data, uint32_t * len, uint32_t * key);

#endif

tea.c

#include "tea.h"
/* encryptBlock
 *   Encrypts byte array data of length len with key key using TEA
 * Arguments:
 *   data - pointer to 8 bit data array to be encrypted - SEE NOTES
 *   len - length of array
 *   key - Pointer to four integer array (16 bytes) holding TEA key
 * Returns:
 *   data - encrypted data held here
 *   len - size of the new data array
 * Side effects:
 *   Modifies data and len
 * NOTES:
 * data size must be equal to or larger than ((len + 7) / 8) * 8 + 8
 * TEA encrypts in 8 byte blocks, so it must include enough space to 
 * hold the entire data to pad out to an 8 byte boundary, plus another
 * 8 bytes at the end to give the length to the decrypt algorithm.
 *
 *  - Shortcut - make sure that data is at least len + 15 bytes in size.
 */
void encryptBlock(uint8_t * data, uint32_t * len, uint32_t * key)
{
   uint32_t blocks, i;
   uint32_t * data32;

   // treat the data as 32 bit unsigned integers
   data32 = (uint32_t *) data;

   // Find the number of 8 byte blocks, add one for the length
   blocks = (((*len) + 7) / 8) + 1;

   // Set the last block to the original data length
   data32[(blocks*2) - 1] = *len;

   // Set the encrypted data length
   *len = blocks * 8;

   for(i = 0; i< blocks; i++)
   {
      encrypt(&data32[i*2], key);
   }
}

/* decryptBlock
 *   Decrypts byte array data of length len with key key using TEA
 * Arguments:
 *   data - pointer to 8 bit data array to be decrypted - SEE NOTES
 *   len - length of array
 *   key - Pointer to four integer array (16 bytes) holding TEA key
 * Returns:
 *   data - decrypted data held here
 *   len - size of the new data array
 * Side effects:
 *   Modifies data and len
 * NOTES:
 *   None
 */
void decryptBlock(uint8_t * data, uint32_t * len, uint32_t * key)
{
   uint32_t blocks, i;
   uint32_t * data32;

   // treat the data as 32 bit unsigned integers
   data32 = (uint32_t *) data;

   // Find the number of 8 byte blocks
   blocks = (*len)/8;

   for(i = 0; i< blocks; i++)
   {
      decrypt(&data32[i*2], key);
   }

   // Return the length of the original data
   *len = data32[(blocks*2) - 1];
}

/* encrypt
 *   Encrypt 64 bits with a 128 bit key using TEA
 *   From http://en.wikipedia.org/wiki/Tiny_Encryption_Algorithm
 * Arguments:
 *   v - array of two 32 bit uints to be encoded in place
 *   k - array of four 32 bit uints to act as key
 * Returns:
 *   v - encrypted result
 * Side effects:
 *   None
 */
void encrypt (uint32_t* v, uint32_t* k) {
    uint32_t v0=v[0], v1=v[1], sum=0, i;           /* set up */
    uint32_t delta=0x9e3779b9;                     /* a key schedule constant */
    uint32_t k0=k[0], k1=k[1], k2=k[2], k3=k[3];   /* cache key */
    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);  
    }                                              /* end cycle */
    v[0]=v0; v[1]=v1;
}

/* decrypt
 *   Decrypt 64 bits with a 128 bit key using TEA
 *   From http://en.wikipedia.org/wiki/Tiny_Encryption_Algorithm
 * Arguments:
 *   v - array of two 32 bit uints to be decoded in place
 *   k - array of four 32 bit uints to act as key
 * Returns:
 *   v - decrypted result
 * Side effects:
 *   None
 */
void decrypt (uint32_t* v, uint32_t* k) {
    uint32_t v0=v[0], v1=v[1], sum=0xC6EF3720, i;  /* set up */
    uint32_t delta=0x9e3779b9;                     /* a key schedule constant */
    uint32_t k0=k[0], k1=k[1], k2=k[2], k3=k[3];   /* cache key */
    for (i=0; i<32; i++) {                         /* basic cycle start */
        v1 -= ((v0<<4) + k2) ^ (v0 + sum) ^ ((v0>>5) + k3);
        v0 -= ((v1<<4) + k0) ^ (v1 + sum) ^ ((v1>>5) + k1);
        sum -= delta;                                   
    }                                              /* end cycle */
    v[0]=v0; v[1]=v1;
}

Issues so far:

Coding

  • Using magic numbers in encrypt and decrypt routines - use #defines instead - Kyle
  • If the 64 bit encoding functions aren't used outside this module, their prototypes should be in the code, not header - Simon
  • Add sanity checking to input - Rob
  • Require that input len is a multiple of 8 bytes - making a requirement we can't enforce or check is a recipe for corruption - Rob

Style

  • Using K&R brace style in some places, ANSI in others - consistency - Simon
  • The long descriptive comment is more useful for the end user if it's in the header file (Header for usage, code for implementation) - Rob

EDIT:

Someone asked for the implementation I indicated in the original post, so here it is. Note that I've not cleaned it up for presentation.

To decode a file use decode inputfilename outputfilename. To encode a file use decode inputfilename outputfilename e. ANSI C implementation, so it should work anywhere, although the algorithm itself might be endian dependant.

THE DECRYPTED FILE WILL NOT EXACTLY MATCH THE ORIGINAL FILE. This particular implementation leaves a number of null bytes on the end of the decrypted file that were not present in the original. For my application this was acceptable and simplified my particular usage, but you are likely going to need to modify this for your use.

It uses a fixed key on line 4.

#include <stdio.h>

typedef unsigned long uint32_t;  
const uint32_t TEAKey[4] = {0x95a8882c, 0x9d2cc113, 0x815aa0cd, 0xa1c489f7};

void encrypt (uint32_t* v, const uint32_t* k);
void decrypt (uint32_t* v, const uint32_t* k);

void btea(uint32_t *v, int n, uint32_t const k[4]);

void simpleencrypt(unsigned char * buffer);
void simpledecrypt(unsigned char * buffer);

int main(int argc, char **argv)
{
   FILE *fpin, *fpout;
   int bytecount;
   unsigned char buffer[9], bufferin[9], bufferout[9];
   int i;

   if(argc < 3)
   {
      printf("Use: %s [filenameinput] [filenameoutput]\n", argv[0]);
      return 0;
   }

   if( (fpin = fopen(argv[1], "rb")) == NULL)
   {
      printf("Problem opening input file %s.\n", argv[1]);
      return 0;
   }

   if( (fpout = fopen(argv[2], "wb")) == NULL)
   {
      printf("Problem opening output file %s.\n", argv[2]);
      return 0;
   }

   bytecount = 0;

   while(fread(buffer, 1, 8, fpin) == 8)
   {
      if(argc>3)
      {
            for(i=0;i<8;i++)
            {
                bufferin[i] = buffer[i];
            }

          simpleencrypt(buffer);


            for(i=0;i<8;i++)
            {
                bufferout[i] = buffer[i];
            }
            simpledecrypt(bufferout);
            for(i=0;i<8;i++)
            {
                if(bufferin[i] != bufferout[i])
                {
                    printf("Internal decode test failed.\n");
                }
            }

      }
      else
      {
          simpledecrypt(buffer);
      }
      fwrite(buffer, 1, 8, fpout);
      bytecount+=8;
   }

   if (!feof(fpin))
   {
       printf("Unexpected input file error encountered.\n");
   }

   fclose(fpin); 
   fclose(fpout); 
   printf("%s complete, %i bytes total\n",((argc>3) ? "Encrypt" : "Decrypt"), bytecount);
   return 0;
}

void simpleencrypt(unsigned char * buffer)
{
    uint32_t datablock[2];

    datablock[0] = (buffer[0] << 24) | (buffer[1] << 16)  | (buffer[2] << 8) | (buffer[3]);
    datablock[1] = (buffer[4] << 24) | (buffer[5] << 16)  | (buffer[6] << 8) | (buffer[7]);

    encrypt (datablock, TEAKey);

    buffer[0] = (char) ((datablock[0] >> 24) & 0xFF);
    buffer[1] = (char) ((datablock[0] >> 16) & 0xFF);
    buffer[2] = (char) ((datablock[0] >> 8) & 0xFF);
    buffer[3] = (char) ((datablock[0]) & 0xFF);
    buffer[4] = (char) ((datablock[1] >> 24) & 0xFF);
    buffer[5] = (char) ((datablock[1] >> 16) & 0xFF);
    buffer[6] = (char) ((datablock[1] >> 8) & 0xFF);
    buffer[7] = (char) ((datablock[1]) & 0xFF);
}

void simpledecrypt(unsigned char * buffer)
{
    uint32_t datablock[2];

    datablock[0] = (buffer[0] << 24) | (buffer[1] << 16)  | (buffer[2] << 8) | (buffer[3]);
    datablock[1] = (buffer[4] << 24) | (buffer[5] << 16)  | (buffer[6] << 8) | (buffer[7]);

    decrypt (datablock, TEAKey);

    buffer[0] = (char) ((datablock[0] >> 24) & 0xFF);
    buffer[1] = (char) ((datablock[0] >> 16) & 0xFF);
    buffer[2] = (char) ((datablock[0] >> 8) & 0xFF);
    buffer[3] = (char) ((datablock[0]) & 0xFF);
    buffer[4] = (char) ((datablock[1] >> 24) & 0xFF);
    buffer[5] = (char) ((datablock[1] >> 16) & 0xFF);
    buffer[6] = (char) ((datablock[1] >> 8) & 0xFF);
    buffer[7] = (char) ((datablock[1]) & 0xFF);
}

/* encrypt
 *   Encrypt 64 bits with a 128 bit key using TEA
 *   From http://en.wikipedia.org/wiki/Tiny_Encryption_Algorithm
 * Arguments:
 *   v - array of two 32 bit uints to be encoded in place
 *   k - array of four 32 bit uints to act as key
 * Returns:
 *   v - encrypted result
 * Side effects:
 *   None
 */
void encrypt (uint32_t* v, const uint32_t* k) {
    uint32_t v0=v[0], v1=v[1], sum=0, i;           /* set up */
    uint32_t delta=0x9e3779b9;                     /* a key schedule constant */
    uint32_t k0=k[0], k1=k[1], k2=k[2], k3=k[3];   /* cache key */
    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);  
    }                                              /* end cycle */
    v[0]=v0; v[1]=v1;
}

/* decrypt
 *   Decrypt 64 bits with a 128 bit key using TEA
 *   From http://en.wikipedia.org/wiki/Tiny_Encryption_Algorithm
 * Arguments:
 *   v - array of two 32 bit uints to be decoded in place
 *   k - array of four 32 bit uints to act as key
 * Returns:
 *   v - decrypted result
 * Side effects:
 *   None
 */
void decrypt (uint32_t* v, const uint32_t* k) {
    uint32_t v0=v[0], v1=v[1], sum=0xC6EF3720, i;  /* set up */
    uint32_t delta=0x9e3779b9;                     /* a key schedule constant */
    uint32_t k0=k[0], k1=k[1], k2=k[2], k3=k[3];   /* cache key */
    for (i=0; i<32; i++) {                         /* basic cycle start */
        v1 -= ((v0<<4) + k2) ^ (v0 + sum) ^ ((v0>>5) + k3);
        v0 -= ((v1<<4) + k0) ^ (v1 + sum) ^ ((v1>>5) + k1);
        sum -= delta;                                   
    }                                              /* end cycle */
    v[0]=v0; v[1]=v1;
}

#define DELTA 0x9e3779b9
  #define MX ((z>>5^y<<2) + (y>>3^z<<4)) ^ ((sum^y) + (k[(p&3)^e] ^ z));

  void btea(uint32_t *v, int n, uint32_t const k[4]) {
    uint32_t y, z, sum;
    unsigned p, rounds, e;
    if (n > 1) {          /* Coding Part */
      rounds = 6 + 52/n;
      sum = 0;
      z = v[n-1];
      do {
        sum += DELTA;
        e = (sum >> 2) & 3;
        for (p=0; p<n-1; p++)
          y = v[p+1], z = v[p] += MX;
        y = v[0];
        z = v[n-1] += MX;
      } while (--rounds);
    } else if (n < -1) {  /* Decoding Part */
      n = -n;
      rounds = 6 + 52/n;
      sum = rounds*DELTA;
      y = v[0];
      do {
        e = (sum >> 2) & 3;
        for (p=n-1; p>0; p--)
          z = v[p-1], y = v[p] -= MX;
        z = v[n-1];
        y = v[0] -= MX;
      } while ((sum -= DELTA) != 0);
    }
  }
share|improve this question
    
Would you mind explaining the following: 1. "pointer to 8 bit data array" means? does that mean the data is read by 8-bits once? 2. In your implementations shows 4 fixed keys, but can it be randomly pick of 4 keys in a file? –  user18545 Oct 20 '12 at 22:53
    
@LimTeikWei It just needs a pointer to the data. It treats the data as an array of bytes. Yes, the 4 keys can be any numbers you like. –  Adam Davis Oct 24 '12 at 18:41
add comment

migrated from stackoverflow.com Apr 23 '11 at 22:02

This question came from our site for professional and enthusiast programmers.

5 Answers

up vote 5 down vote accepted

You should add some sanity checking on the incoming arguments, especially if you intend this to be library code used from multiple projects.

I would recommend you change definition of encryptBlock to require that len is a multiple of 8 bytes. Requiring that the incoming array is sized to a multiple of 8 bytes, but not enforcing that for len is a sure fire way to get memory corruption when someone forgets (e.g. when they are encrypting strings).

Personally I prefer to have the long descriptive comment in the header file rather than the .c file. Anyone using tea.h will find it more useful there.

share|improve this answer
    
I'm going to require the input be a multiple of 8 bytes for some of what I'm encrypting, so thanks for the suggestion! –  Adam Davis Sep 15 '08 at 12:45
add comment

Without looking too closely, I'm struggling to see any major problems with this code. Here are a couple of trivial issues:

  1. Did you mean to use different brace styles? The block functions use (sort of) K&R braces and the 64-bit functions use ANSI braces.
  2. Are the 64-bit functions ever to be used outside the module? If not, I would move their prototypes to the .c file.
share|improve this answer
add comment

I'm actually the opposite if of Rob Walker in that I prefer there to be a nice comment block preceding the functions in the code file as opposed to the header. However, as long as you are constant in where you put your comments, most people shouldn't have an issue unless it goes against your company's manual of style. The only other thing I can think of in terms of style would be a comment block at the start of the files with some general information, for example:

/*-----------------------------------------------------------------------------
 * File      - 
 * Author(s) - 
 *
 * Purpose   -
 *
 *----------------------------------------------------------------------------- 
 * Notes     - 
 *-----------------------------------------------------------------------------
 * Revisions - 
 *---------------------------------------------------------------------------*/

Sometimes you will also see copyright information in these blocks as well. This is just an example of what one looks like though.

In terms of the code itself, I have to agree with the others in regards to putting the "magic numbers" off to the side and making sure they are clearly labeled as to what they do. Ditto for the sanity checking if this is going to be used as part of a library.

share|improve this answer
add comment

I'm not familiar with your algorithm, but in glancing over your code, it looks like sum and delta are magic numbers. Since delta is shared between encrypt and decrypt, it might be a good idea to #define them at the top of the file.

edit: I just looked at the Wikipedia article and the magic numbers make an appearance in the reference implementation, which has been copied verbatim into your code.

share|improve this answer
add comment

Implementing ciphers is like optimisation: the first rule is "Don't do it". Leave it to the experts, because there are many important details which you will miss if you aren't an expert.

The three things which jump out at me about this are:

  1. TEA is broken. It's not "perfect" for "casual protection". Its successors are also broken, and I personally wouldn't use any of them either, but at least they're less broken.

  2. The padding is somewhat unconventional, and guarantees to require an extra half-block. There are several approaches which require an extra block only when the message is an integral number of blocks in length.

  3. The only mode of operation supported is ECB. This is a big red flag. The question of which block mode is the best is not a settled one, but ECB is unquestionably the worst.

share|improve this answer
    
One significant advantage it does have, aside from being freely available even outside the US under very liberal terms, is that it's simple and fast even on tiny micro controllers. Yes, it's insecure relative to its key length, but in many applications it is secure enough and the low processing requirements can sometimes make it a worthwhile choice. On most advanced processors though, advanced instruction sets include cryptography friendly operators and even advanced encryption doesn't require much more power than tea, and they should be used instead if the licensing terms are acceptable. –  Adam Davis Oct 16 '12 at 23:19
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.