Like Tree2Likes
  • 2 Post By anduril462

Critique my mini-AES

This is a discussion on Critique my mini-AES within the C Programming forums, part of the General Programming Boards category; This is a mini-AES program I have been working on partly as an independent project and partly for my internship ...

  1. #1
    Registered User
    Join Date
    May 2011
    Posts
    1

    Critique my mini-AES

    This is a mini-AES program I have been working on partly as an independent project and partly for my internship over the past few months. I'm not sure if it works or not, when I try to run it it takes forever, I waited about 30 min and got impatient. Also, I just started programming C about 6 months ago.

    I am looking for critique and *corrections* of this program. It will be much appreciated. I also realize some of my methods may not be the most efficient, I was just trying to get it done, and will also welcome any critique in this category.

    Code:
    #include <stdio.h>
    #include <string.h>
    #include <stdlib.h>
    
    
    FILE *fp;
    char *buffer;
    char *outputBuffer;
    unsigned long fileLen;
    int c;
    
    //user inputted key - tester
    char* KEY = "hi";
    
    //round keys
    unsigned int w0, w1, w2, w3, w4, w5, w6, w7, w8, w9, w10, w11;
    
    //text nibbles
    unsigned int Nibble1, Nibble2, Nibble3, Nibble4;
    
    int currentBufferPosition = 0;
    
    unsigned int t[2][16];
    
    void readFile(char *name) {
    
    	fp = fopen(name, "rb");
    
    	fseek(fp, 0, SEEK_END);
    	fileLen = ftell(fp);
    	fseek(fp, 0, SEEK_SET);
    
    	buffer = malloc(fileLen+1);
    
    	int i = 0;
    
    	for(i = 0;i < fileLen - 1;++i) {
    		(buffer[i]) = (char)fgetc(fp);;
    	}
    
    	fclose(fp);
    
    	}
    
    
    void getNibbles() {
    
    	Nibble1 = (buffer[currentBufferPosition]) & 0xf;
    	Nibble2 = (buffer[currentBufferPosition] << 4) & 0xf0;
    
    	//printf("%x\n", Nibble1);
    	//printf("%x\n", Nibble2);
    
    	++currentBufferPosition;
    
    	Nibble3 = (buffer[currentBufferPosition]) & 0xf;
    	Nibble4 = (buffer[currentBufferPosition] << 4) & 0xf0;
    
    	//printf("%x\n", Nibble3);
    	//printf("%x\n", Nibble4);
    
    	}
    
    void getKeyNibbles(char *k) {
    	
    	w0 = (k[0]) & 0xf;
    	w1 = (k[0] << 4) & 0xf0;
    
    	//printf("%x\n", Nibble1);
    	//printf("%x\n", Nibble2);
    
    	w2 = (k[1]) & 0xf;
    	w3 = (k[1] << 4) & 0xf0;
    
    	//printf("%x\n", Nibble3);
    	//printf("%x\n", Nibble4);
    
    }
    
    void setChart() {
    
    	t[1][1] = 0x0;
    	t[1][2] = 0x1;
    	t[1][3] = 0x2;
    	t[1][4] = 0x3;
    	t[1][5] = 0x4;
    	t[1][6] = 0x5;
    	t[1][7] = 0x6;
    	t[1][8] = 0x7;
    	t[1][9] = 0x8;
    	t[1][10] = 0x9;
    	t[1][11] = 0xa;
    	t[1][12] = 0xb;
    	t[1][13] = 0xc;
    	t[1][14] = 0xd;
    	t[1][15] = 0xe;
    	t[1][16] = 0xf;
    
    	t[2][1] = 0xe;
    	t[2][2] = 0x4;
    	t[2][3] = 0xd;
    	t[2][4] = 0x1;
    	t[2][5] = 0x2;
    	t[2][6] = 0xf;
    	t[2][7] = 0xb;
    	t[2][8] = 0x8;
    	t[2][9] = 0x3;
    	t[2][10] = 0xa;
    	t[2][11] = 0x6;
    	t[2][12] = 0xc;
    	t[2][13] = 0x5;
    	t[2][14] = 0x9;
    	t[2][15] = 0x0;
    	t[2][16] = 0x7;
    
    }
    
    unsigned int nibbleSub(unsigned int n) {
    	
    	int x;
    
    	for(x = 0; x < 16; x++) {
    
    		if (n == t[1][x]) {
    			n = t[2][x];
    			return n;
    		}
    
    	}
    	
    }
    
    void mixColumn() {
    
    	unsigned int temp1, temp2, temp3, temp4;
    
    	temp1 = Nibble1;
    	temp2 = Nibble2;
    	temp3 = Nibble3;
    	temp4 = Nibble4;
    
    	Nibble1 = (0x3 * temp1) ^ (0x2 * temp2);
    	Nibble2 = (0x2 * temp1) ^ (0x3 * temp2);
    	Nibble3 = (0x3 * temp3) ^ (0x2 * temp4);
    	Nibble4 = (0x2 * temp3) ^ (0x3 * temp4);
    
    }
    
    void shiftRow() {
    
    	unsigned int temp;
    
    	temp = Nibble2;
    	Nibble2 = Nibble4;
    	Nibble4 = temp;
    
    }
    
    //inputted key must be 16 bits(2 chars)
    void setKeys(char *s) {
    
    	unsigned int temp0, temp1, temp2, temp3;
    
    	getKeyNibbles(s);	
    	
    		w4 = w0 ^ nibbleSub(w3) ^ 0x1;
    		w5 = w1 ^ w4;
    		w6 = w2 ^ w5;
    		w7 = w3 ^ w6;
    		w8 = w4 ^ nibbleSub(w7) ^ 0x2;
    		w9 = w5 ^ w8;
    		w10 = w6 ^ w9;
    		w11 = w7 ^ w10;	
    }
    
    void addKey(int round) {
    
    	if (round = 0) {
    		Nibble1 = Nibble1 ^ w0;
    		Nibble2 = Nibble2 ^ w1;
    		Nibble3 = Nibble3 ^ w2;
    		Nibble4 = Nibble4 ^ w3;
    	} else if (round = 1) {
    		Nibble1 = Nibble1 ^ w4;
    		Nibble2 = Nibble2 ^ w5;
    		Nibble3 = Nibble3 ^ w6;
    		Nibble4 = Nibble4 ^ w7;
    	} else if (round = 2) {
    		Nibble1 = Nibble1 ^ w8;
    		Nibble2 = Nibble2 ^ w9;
    		Nibble3 = Nibble3 ^ w10;
    		Nibble4 = Nibble4 ^ w11;
    	}
    
    }
    
    void encrypt(char *name) {
    
    	readFile(name);
    	setKeys(KEY);
    	setChart();
    
    	do {
    		getNibbles;
    		addKey(0);
    		Nibble1 = nibbleSub(Nibble1);
    		Nibble2 = nibbleSub(Nibble2);
    		Nibble3 = nibbleSub(Nibble3);
    		Nibble4 = nibbleSub(Nibble4);
    		shiftRow();
    		mixColumn();
    		addKey(1);
    		Nibble1 = nibbleSub(Nibble1);
    		Nibble2 = nibbleSub(Nibble2);
    		Nibble3 = nibbleSub(Nibble3);
    		Nibble4 = nibbleSub(Nibble4);
    		shiftRow();
    		addKey(2);
    		
    		outputBuffer = outputBuffer + Nibble1 + Nibble2 + Nibble3 + Nibble4;
    		currentBufferPosition++;
    
    	} while (currentBufferPosition < fileLen - 1);
    
    }
    
    void printBuffer() {
    
    	int counter;
    
    	for (counter = 0; counter < fileLen -1; counter++) {
    
    		printf("%x", outputBuffer[counter]);
    
    	}
    
    }
    
    
    void main() {
    	
    	encrypt("text");	
    	
    }

  2. #2
    Banned
    Join Date
    Aug 2010
    Location
    Ontario Canada
    Posts
    9,547
    1) It's int main (void) unless you're working on a microcontroller with it's own C variant.

    2) because it's int main (void) main has to return a value (usually 0) to the operating system.

    3) Global variables are not wrong... but they should be used sparingly to avoid accidental changes in value. It is far smarter to pass a couple of paremeters into a function and have it return a result than to let your functions operate (almost) entirely on global values.

    4) ALWAYS check the return values of I/O operations such as fopen(), fscanf(), etc. Ask what happens if the file doesn't open for some reason...

    5) Instead of reading characters in a loop, you could load your text buffer with a single call to fread()

    6) Your setchart() function could be replaced by using array initializers with your t array when it's defined (no processing overhead)

    7) The t array itself only needs to be single dimensional... like this...
    Code:
    unsigned int t[16] = {0xE,0x4,0xD,0x1,0x2,0xF,0xB,0xA,0x8,0xC,0x5,0x9,0x0,0x7}
    8) With the array initialized as shown you can access it much more efficiently in nibblesub()
    Code:
    unsigned int nibbleSub(unsigned int n) 
      { return t[n]; }
    9) your round keys w0 .. w11 should probably be an array.

    10) in encrypt your nibbles should also probably be an array as this would allow looping.

    11) in encrypt() your assignment to output buffer will fail... you need to access this as an array of characters, C does not "add" strings.

  3. #3
    ATH0 quzah's Avatar
    Join Date
    Oct 2001
    Posts
    14,826
    1. You use globals pretty much everywhere. You really shouldn't.
    2. This:
    Code:
    void main() {
    	
    	encrypt("text");	
    	
    }
    Should be:
    Code:
    int main( void )
    {
        encrypt( "text" );
        return 0;
    }
    3. You are writing to memory you don't have, because you don't understand arrays:
    Code:
    void setChart() {
    
    	t[1][1] = 0x0;
    	t[1][2] = 0x1;
    	t[1][3] = 0x2;
    	t[1][4] = 0x3;
    	t[1][5] = 0x4;
    	t[1][6] = 0x5;
    	t[1][7] = 0x6;
    	t[1][8] = 0x7;
    	t[1][9] = 0x8;
    	t[1][10] = 0x9;
    	t[1][11] = 0xa;
    	t[1][12] = 0xb;
    	t[1][13] = 0xc;
    	t[1][14] = 0xd;
    	t[1][15] = 0xe;
    	t[1][16] = 0xf;
    
    	t[2][1] = 0xe;
    	t[2][2] = 0x4;
    	t[2][3] = 0xd;
    	t[2][4] = 0x1;
    	t[2][5] = 0x2;
    	t[2][6] = 0xf;
    	t[2][7] = 0xb;
    	t[2][8] = 0x8;
    	t[2][9] = 0x3;
    	t[2][10] = 0xa;
    	t[2][11] = 0x6;
    	t[2][12] = 0xc;
    	t[2][13] = 0x5;
    	t[2][14] = 0x9;
    	t[2][15] = 0x0;
    	t[2][16] = 0x7;
    
    }
    Arrays go from 0 to size -1. So you should have t[0] and t[1], not t[1] and t[2].

    4. All of your 'Nibble' stuff could just as easily be another array.

    That will get you started.


    Quzah.
    Hope is the first step on the road to disappointment.

  4. #4
    Registered User
    Join Date
    Nov 2010
    Location
    Long Beach, CA
    Posts
    5,512
    I didn't see this in my scan of Tater's or Quzah's comments:
    Code:
    void addKey(int round) {
    
    	if (round == 0) {
            ...
    	} else if (round == 1) {
            ...
    	} else if (round == 2) {
            ...
    	}
    
    }
    A single = is for assignment, a double == is for comparison. Learn when to use which one. Your if clauses here need a double ==.
    CommonTater and quzah like this.

  5. #5
    Registered User
    Join Date
    Nov 2010
    Location
    Long Beach, CA
    Posts
    5,512
    Oh, and you should declare your functions to specifically take a void parameter. There's a difference.
    Code:
    void foo()
    This declares a function foo that returns nothing and takes any amount and type (including none) parameters. This means the compiler can NOT check to make sure you're calling it properly.
    Code:
    void foo(void)
    This declares a function foo that returns nothing and takes exactly zero parameters. This form makes your intentions clear and lets the compiler do it's job of catching any screw-ups you might have made.

  6. #6
    Banned
    Join Date
    Aug 2010
    Location
    Ontario Canada
    Posts
    9,547
    @anduril ... nice catch!

  7. #7
    ATH0 quzah's Avatar
    Join Date
    Oct 2001
    Posts
    14,826
    Quote Originally Posted by CommonTater View Post
    @anduril ... nice catch!
    Indeed. I didn't even see Tater's made it in before mine. Anyway, all of that should be a good starting point.


    Quzah.
    Hope is the first step on the road to disappointment.

  8. #8
    Banned
    Join Date
    Aug 2010
    Location
    Ontario Canada
    Posts
    9,547
    One more I just noticed.... In encrypt() output buffer is a char* that is used in an uninitialized state.


    And here's the list from Pelles C
    Code:
    Building main.obj.
    E:\c_Code\Experiments\testconsole\main.c(29): warning #2216: The return value from 'fseek' is never used.
    E:\c_Code\Experiments\testconsole\main.c(31): warning #2216: The return value from 'fseek' is never used.
    E:\c_Code\Experiments\testconsole\main.c(41): warning #2216: The return value from 'fclose' is never used.
    E:\c_Code\Experiments\testconsole\main.c(46): warning #2117: Old-style function definition for 'getNibbles'.
    E:\c_Code\Experiments\testconsole\main.c(46): warning #2027: Missing prototype for 'getNibbles'.
    E:\c_Code\Experiments\testconsole\main.c(80): warning #2117: Old-style function definition for 'setChart'.
    E:\c_Code\Experiments\testconsole\main.c(80): warning #2027: Missing prototype for 'setChart'.
    E:\c_Code\Experiments\testconsole\main.c(97): warning #2238: Array index for 'unsigned int [16]' is out-of-bounds.
    E:\c_Code\Experiments\testconsole\main.c(99): warning #2238: Array index for 'unsigned int [2][16]' is out-of-bounds.
    E:\c_Code\Experiments\testconsole\main.c(100): warning #2238: Array index for 'unsigned int [2][16]' is out-of-bounds.
    E:\c_Code\Experiments\testconsole\main.c(101): warning #2238: Array index for 'unsigned int [2][16]' is out-of-bounds.
    E:\c_Code\Experiments\testconsole\main.c(102): warning #2238: Array index for 'unsigned int [2][16]' is out-of-bounds.
    E:\c_Code\Experiments\testconsole\main.c(103): warning #2238: Array index for 'unsigned int [2][16]' is out-of-bounds.
    E:\c_Code\Experiments\testconsole\main.c(104): warning #2238: Array index for 'unsigned int [2][16]' is out-of-bounds.
    E:\c_Code\Experiments\testconsole\main.c(105): warning #2238: Array index for 'unsigned int [2][16]' is out-of-bounds.
    E:\c_Code\Experiments\testconsole\main.c(106): warning #2238: Array index for 'unsigned int [2][16]' is out-of-bounds.
    E:\c_Code\Experiments\testconsole\main.c(107): warning #2238: Array index for 'unsigned int [2][16]' is out-of-bounds.
    E:\c_Code\Experiments\testconsole\main.c(108): warning #2238: Array index for 'unsigned int [2][16]' is out-of-bounds.
    E:\c_Code\Experiments\testconsole\main.c(109): warning #2238: Array index for 'unsigned int [2][16]' is out-of-bounds.
    E:\c_Code\Experiments\testconsole\main.c(110): warning #2238: Array index for 'unsigned int [2][16]' is out-of-bounds.
    E:\c_Code\Experiments\testconsole\main.c(111): warning #2238: Array index for 'unsigned int [2][16]' is out-of-bounds.
    E:\c_Code\Experiments\testconsole\main.c(112): warning #2238: Array index for 'unsigned int [2][16]' is out-of-bounds.
    E:\c_Code\Experiments\testconsole\main.c(113): warning #2238: Array index for 'unsigned int [2][16]' is out-of-bounds.
    E:\c_Code\Experiments\testconsole\main.c(114): warning #2238: Array index for 'unsigned int [16]' is out-of-bounds.
    E:\c_Code\Experiments\testconsole\main.c(118): warning #2235: Not all control paths return a value.
    E:\c_Code\Experiments\testconsole\main.c(133): warning #2117: Old-style function definition for 'mixColumn'.
    E:\c_Code\Experiments\testconsole\main.c(133): warning #2027: Missing prototype for 'mixColumn'.
    E:\c_Code\Experiments\testconsole\main.c(149): warning #2117: Old-style function definition for 'shiftRow'.
    E:\c_Code\Experiments\testconsole\main.c(149): warning #2027: Missing prototype for 'shiftRow'.
    E:\c_Code\Experiments\testconsole\main.c(162): warning #2114: Local 'temp3' is not referenced.
    E:\c_Code\Experiments\testconsole\main.c(162): warning #2114: Local 'temp2' is not referenced.
    E:\c_Code\Experiments\testconsole\main.c(162): warning #2114: Local 'temp1' is not referenced.
    E:\c_Code\Experiments\testconsole\main.c(162): warning #2114: Local 'temp0' is not referenced.
    E:\c_Code\Experiments\testconsole\main.c(178): warning #2030: '=' used in a conditional expression.
    E:\c_Code\Experiments\testconsole\main.c(183): warning #2030: '=' used in a conditional expression.
    E:\c_Code\Experiments\testconsole\main.c(188): warning #2030: '=' used in a conditional expression.
    E:\c_Code\Experiments\testconsole\main.c(179): warning #2154: Unreachable code.
    E:\c_Code\Experiments\testconsole\main.c(180): warning #2154: Unreachable code.
    E:\c_Code\Experiments\testconsole\main.c(181): warning #2154: Unreachable code.
    E:\c_Code\Experiments\testconsole\main.c(182): warning #2154: Unreachable code.
    E:\c_Code\Experiments\testconsole\main.c(183): warning #2154: Unreachable code.
    E:\c_Code\Experiments\testconsole\main.c(189): warning #2154: Unreachable code.
    E:\c_Code\Experiments\testconsole\main.c(190): warning #2154: Unreachable code.
    E:\c_Code\Experiments\testconsole\main.c(191): warning #2154: Unreachable code.
    E:\c_Code\Experiments\testconsole\main.c(192): warning #2154: Unreachable code.
    E:\c_Code\Experiments\testconsole\main.c(204): warning #2046: Expression with no effect removed.
    E:\c_Code\Experiments\testconsole\main.c(227): warning #2117: Old-style function definition for 'printBuffer'.
    E:\c_Code\Experiments\testconsole\main.c(227): warning #2027: Missing prototype for 'printBuffer'.
    E:\c_Code\Experiments\testconsole\main.c(233): warning #2216: The return value from 'printf' is never used.
    E:\c_Code\Experiments\testconsole\main.c(240): warning #2117: Old-style function definition for 'main'.
    E:\c_Code\Experiments\testconsole\main.c(240): warning #2181: Incorrect signature for entry-point 'main'; expected 'int __cdecl function(void)' but found 'void __cdecl function(void)'.
    Building testconsole.exe.
    Done.
    Last edited by CommonTater; 05-04-2011 at 03:52 PM.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Mini If
    By Milhas in forum C Programming
    Replies: 4
    Last Post: 03-27-2008, 04:04 PM
  2. Critique my code?
    By bigbadbowlindud in forum C++ Programming
    Replies: 20
    Last Post: 06-24-2004, 12:54 PM
  3. mini-itx
    By whistlenm1 in forum A Brief History of Cprogramming.com
    Replies: 4
    Last Post: 06-18-2003, 03:58 PM
  4. C 101 critique, please?
    By adobephile in forum C Programming
    Replies: 13
    Last Post: 01-01-2003, 06:05 PM
  5. critique me please
    By ober in forum A Brief History of Cprogramming.com
    Replies: 13
    Last Post: 10-02-2002, 01:59 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21