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.

I've been playing around with 8051 assembly lately and thought I would make a little project of implementing RC4, since it is pretty interesting and the algorithm doesn't seem too hard. Plus, taking mod 256 is REALLY easy when you're only working with single bytes.

Below I've included my (hopefully well-commented) code. I based my algorithm on the description given on Wikipedia here and use the same naming conventions (i, j, S[i]). I am new to the 8051 and assembly in general and am just looking for any optimization advice. Do my uses of subroutines and the way I assigned i-l make sense? Am I failing to use important standard conventions?

(BTW, I have simulated the key schedule algorithm and the keystream matches the test vector here for a 5-byte key 0x0102030405, so I am at least mostly confident in my code's math.)

$NOSYMBOLS
$INCLUDE (C:\RIDE\INC\51\REG51.INC)
$INCLUDE (C:\RIDE\INC\51\VECTORS51.INC)

i DATA 06h;
j DATA 07h;
k DATA 08h;
l DATA 09h;

ORG 0;

LJMP STARTUP;

ORG 600h;

STARTUP: MOV SP, #6Fh; move stack pointer to a safely high address

ACALL READ_KEY;
ACALL FILL_S;
ACALL KSA;
ACALL INIT_INDICIES; clear i and j for the last time before encryption begins. from here on out, they change only when the PRNG cycles
MOV k, #03h; run DROP 3 times = 768 cycles
ACALL DROP;
ACALL ENCRYPT;
SJMP STOP; finished, loop forever

; read the key from 3100h to R1-R5
READ_KEY: MOV DPTR, #3100h; set DPTR to address of first byte of key
MOVX A, @DPTR; move first byte of key into A
MOV R1, A; copy first byte of key to R1
INC DPTR; set DPTR to address of second byte of key
MOVX A, @DPTR; etc
MOV R2, A; etc
INC DPTR; etc
MOVX A, @DPTR;
MOV R3, A;
INC DPTR;
MOVX A, @DPTR;
MOV R4, A;
INC DPTR;
MOVX A, @DPTR;
MOV R5, A;
RET;

; place the identity permutation (0, 1, 2, ..., FF) at 3000h-30FFh
FILL_S: MOV DPTR #3000h; S array begins at 3000h
CLR A; known startup state
LOOP_S: MOVX @DPTR, A; fill S
INC A; get ready for the next number...
INC DPTR; ...and the next address
CJNE A, #0FFh, LOOP_S; jump back until all 256 values are written
RET;

KSA: ACALL INIT_INDICIES; zero i and j
MOV DPTR, #3000h; head back to the start of the S array
KSA_SHUFFLE: MOVX A, @DPTR; grab S[i] and bring it to A
ADD A, j; get S[i] + j
PUSH ACC; store S[i] + j for later
;time to get (i mod 5) +1
MOV A, i;
MOV B, #05h; prepare for division
DIV AB; B will contain the remainder which is i mod 5
MOV A, B; get the remainder into a more convenient register
ADD A, #01h; add 1 b/c we start at R1, not R0
MOV R0, A; put address of key[i mod 5] in R0
MOV A, @R0; get the chosen key byte
POP 0; get S[i] + j back, this time into R0
ADD A, R0; finally, S[i] + j + key[i mod 5]
MOV j, A; store the new j
; and now the actual shuffle (swap S[i] and S[j])
; the top byte of DPTR is still 30h
MOV DPL, i; set DPTR to location of S[i]
MOVX A, @DPTR; read in S[i]...
PUSH R0; ...and save it for later
MOV DPL, j; now go to location of S[j]
MOVX A, @DPTR; read in S[j]...
XCH A, R0; ...save S[j] for later, get S[i] back in A
MOVX @DPTR, A; put the old value of S[i] at S[j]
MOV DPL, i; head on back to the location of S[i]
MOV A, R0; put the old S[j] value in A
MOVX @DPTR, A; and finally write old S[j] to its new home in S[i]
INC i; we did it once
MOV DPL, i; set address of the next S[i] to grab
CJNE R6, #0FFh, KSA_SHUFFLE; lather, rinse, and repeat (R6 = i)
RET;

; "warm up" the prng by running a multiple of 256 times based on the value of k
DROP: CLR 20.1; clear the "output keystream" flag
DROP_LOOP: ACALL PRNG_CYCLE; cycle the PRNG
CJNE R6, #FFh DROP_LOOP; go back 256 times
DJNZ 08, DROP_LOOP; decrement k (at address 8) and jump back if not zero
RET;

; encrypt the message stored at 3200h-32FFh;
ENCRYPT: MOV DPH, #32h; head over to where the message is stored
MOV l, #0h; known starting state
SETB 20.1; we want to get the keystream!
MOV DPL, l; get the right byte of the message
ENCRYPT_LOOP: MOVX A, @DPTR; get the first byte of the message
MOV DPH, #30h; get ready for PRNG_CYCLE--it expects to be around 0x30xx
ACALL PRNG_CYCLE; gets the next byte of the keystream, store it in k
XRL A, 08; (08 = k) perform the encryption!
MOV DPH, #32h; move to where we need to be to write the ciphertext
MOVX @DPTR, A; write back the encrypted byte
INC l; get ready for the next byte
CJNE 09, #0FFh, ENCRYPT_LOOP; do it again, unless we've finished the message (09 = l)
RET;

PRNG_CYCLE: INC i;
MOV DPL, i; set DPTR to location of S[i]
MOVX A, @DPTR; get S[i]
PUSH ACC; save S[i] for a moment
ADD A, j; get S[i] + j
MOV j, A; store the new j
POP ACC; get S[i] back
MOV R1, A; ...and save it for later
MOV DPL, j; now go to location of S[j]
MOVX A, @DPTR; read in S[j]...
XCH A, R0; save S[j] for later, get back S[i]
MOVX @DPTR, A; put the old value of S[i] at S[j]
MOV DPL, i; head on back to the location of S[i]
MOV A, R0; put the old S[j] value in A
MOVX @DPTR, A; and finally write old S[j] to its new home in S[i]
JNB 20.1, END_PRNG_CYCLE; don't output the keystream value if we're just warming up the PRNG
ADD A, R1; S[i] + S[j]
MOV DPL, A; put address of S[S[i] + S[j] mod 256] in DPTR
MOVX A, @DPTR; read in S[S[i] + S[j] mod 256]
MOV k, A; store they keystream value in k for use by ENCRYPT
END_PRNG_CYCLE: RET;

; zero my indicies i and j
INIT_INDICIES: MOV i, #00h; zero i (R6)
MOV j, #00h; zero j (R7)
RET;

STOP: NOP;
SJMP STOP;

END
share|improve this question
add comment

migrated from programmers.stackexchange.com Apr 5 '13 at 16:27

This question came from our site for professional programmers interested in conceptual questions about software development.

1 Answer

I'm not familiar with 8051 assembly in particular, but there are some readability things I want to point out.

Commenting

It would be more readable to vertically-align all the comments provided for the individual lines. With the code and comments condensed like this, it's hard to distinguish them from the code. Both should easily be able to stand out from each other.

It may also be beneficial to have a description and register usage given at the top of each procedure.

Consider something in this form:

; procedure name
; procedure description
;
; register 1 - what it stores, if used
; ...
; register n - what it stores, if used

Procedures

I like that there are breaks between the procedures, which allows me to tell apart the separate procedures. However, you can do more here for readability.

I'd recommend keeping the procedure labels on their own lines. The accompanying commands, arguments, and comments could also be vertically-aligned with each other.

Here's an example:

procedure:
; command   arguments   comments
; command   arguments   comments
; command   arguments   comments

This would especially be useful with your jump commands, as you should clearly be able to tell which labels are used in your program.

share|improve this answer
    
IMO it's not a bad idea to comment every line of assembly; but it would be more readable if the comments were vertically aligned. Vertical alignment is a bore to write and maintain, but then again it is assembly. See e.g. here for an example of comments. –  ChrisW Mar 7 at 10:35
    
@ChrisW: Oh yeah. I was thinking about that, but I forgot about vertically-aligned. I'll make the changes. –  Jamal Mar 7 at 14:57
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.