Arduino code lock

[lolimpol]

I am making a Button lock for school, I need it to send an output when one of two codes get pressed (for example: 1234 = on, 4321 = on, 4231 = off, 1432 = off) here's the setup: https://goo.gl/photos/WpGmVAVoiJkg5LjD6
button 1 = 8
button 2 = 9
button 3 = 10
button 4 = 11
red led = 4
green = 12
if you have any idea, please help me!

[CM800]

Simple really:

Allow each button to add a character to a string:

Button 1 = "1"
Button 2 = "2"
Button 3 = "3"
Button 4 = "4"

then read the string & check against known ones, e.g.:

if (enteredPassword == "1234" || enteredPassword == "4321")
{
      digitalWrite(Output, HIGH);
}
else
{
     digitalWrite(Output, LOW);
}

[Bruce Abbott]

4 pushbutton lock coding

[lolimpol]

4 pushbutton lock coding

this code doesnt work...

[lolimpol]

Simple really:

Allow each button to add a character to a string:

Button 1 = "1"
Button 2 = "2"
Button 3 = "3"
Button 4 = "4"

then read the string & check against known ones, e.g.:

if (enteredPassword == "1234" || enteredPassword == "4321")
{
      digitalWrite(Output, HIGH);
}
else
{
     digitalWrite(Output, LOW);
}

But how would I format that? (I am a complete noob )

[CM800]

Simple really:

Allow each button to add a character to a string:

Button 1 = "1"
Button 2 = "2"
Button 3 = "3"
Button 4 = "4"

then read the string & check against known ones, e.g.:

if (enteredPassword == "1234" || enteredPassword == "4321")
{
      digitalWrite(Output, HIGH);
}
else
{
     digitalWrite(Output, LOW);
}

But how would I format that? (I am a complete noob )

I've given you a good hint here!

...but I dare say that it's YOUR course, not mine.

The point of this is for YOU to learn.

Now, go on youtube, watch and follow along with an arduino tutorial series and LEARN

Play around etc...

[lolimpol]

yes I understand, but i'm kind of under pressure, and i'm really really dumb...
I just download code and put it on an arduino. I really want to learn, but now is not the time!

[ebclr]

Cheat on homework isn't a nice thing on Netherlands.........

[hamster_nz]

Simple really:

Allow each button to add a character to a string:

Button 1 = "1"
Button 2 = "2"
Button 3 = "3"
Button 4 = "4"

then read the string & check against known ones, e.g.:

if (enteredPassword == "1234" || enteredPassword == "4321")
{
      digitalWrite(Output, HIGH);
}
else
{
     digitalWrite(Output, LOW);
}

But how would I format that? (I am a complete noob )

Just watch out - you don't compare strings/arrays like that in C/C++/Arduino. They will be at different addresses in memory - use strcmp() or memcmp() instead...  ...unless of course you use the String class, as shown in https://www.arduino.cc/en/Tutorial/StringComparisonOperators

[Bruce Abbott]

I really want to learn, but now is not the time!

Have you heard the expression "necessity is the mother of invention"? Being under pressure is exactly the time to learn!

Here's some working code I made just for you. This is only the second program I have written for Arduino, so it was a learning experience for me too.

Just one catch though - there's a deliberate mistake in it. If you can figure out what it is then you're good to go!

Code: [Select]

//  4 button combination lock for Arduino 
//  by Bruce Abbott       [http://www.bhabbott.net.nz]

#define button1  8       
#define button2  9
#define button3  10
#define button4  11   

#define KeyLed   3    // button press indicator  (long flash = 4th digit entered)
#define RedLed   4    // on when locked, off when unlocked   
#define GreenLed 12   // on when unlocked, off when locked

#define LOCKCODE1   "1234"
#define LOCKCODE2   "4321"

#define UNLOCKCODE1 "4231"
#define UNLOCKCODE2 "1432"

int bS1 = 0;         // current state of button
int lbS1 = 0;        // previous state of button

int bS2 = 0;         
int lbS2 = 0;

int bS3 = 0;         
int lbS3 = 0;

int bS4 = 0;         
int lbS4 = 0;

int counter = 0;       // number of buttons pressed in current sequence

int lockstate = 0;     // 0 = unlocked, 1 locked

char code[] = "    ";  // string to hold 4 digit code

void setup(){
  pinMode(button1, INPUT);
  pinMode(button2, INPUT);
  pinMode(button3, INPUT);
  pinMode(button4, INPUT);
  pinMode(KeyLed, OUTPUT);
  pinMode(RedLed, OUTPUT);
  pinMode(GreenLed, OUTPUT);
  Serial.begin(9600);
//  digitalWrite(GreenLed,HIGH);
}

void loop(){

    bS1 = digitalRead(button1);
    bS2 = digitalRead(button2);
    bS3 = digitalRead(button3);
    bS4 = digitalRead(button4);

    if (bS1 != lbS1) {
       if ((bS1 == HIGH) && (bS2 == LOW) && (bS3 == LOW) && (bS4 == LOW)) {
          code[counter]='1';
          counter = counter + 1;
          digitalWrite(KeyLed,HIGH);
          Serial.println(code);
       }
       lbS1 = bS1;
       delay(100);
       digitalWrite(KeyLed,LOW);
    }

    if (bS2 != lbS2) {
       if ((bS1 == LOW) && (bS2 == HIGH) && (bS3 == LOW) && (bS4 == LOW)) {
          code[counter]='2';
          counter = counter + 1;
          digitalWrite(KeyLed,HIGH);
          Serial.println(code);
          }
       lbS2 = bS2;
       delay(100);
       digitalWrite(KeyLed,LOW);
    }

    if (bS3 != lbS3) {
       if ((bS1 == LOW) && (bS2 == LOW) && (bS3 == HIGH) && (bS4 == LOW)) {
          code[counter]='3';
          counter = counter + 1;
          digitalWrite(KeyLed,HIGH);
          Serial.println(code);
       }
       lbS3 = bS3;
       delay(100);
       digitalWrite(KeyLed,LOW);
    }

    if (bS4 != lbS4) {
       if ((bS1 == LOW) && (bS2 == LOW) && (bS3 == LOW) && (bS4 == HIGH)) {
          code[counter]='4';
          counter = counter + 1;
          digitalWrite(KeyLed,HIGH);
          Serial.println(code);
       }
       lbS4 = bS4;
       delay(100);
       digitalWrite(KeyLed,LOW);
    }

   if (counter == 4) {
      if (!strcmp(code,UNLOCKCODE1) || !strcmp(code,UNLOCKCODE2)) {
         lockstate = 0;
         digitalWrite(GreenLed,HIGH);
         digitalWrite(RedLed,LOW);
      }
      if (!strcmp(code,LOCKCODE1) || !strcmp(code,LOCKCODE2)) {
         lockstate = 1;
         digitalWrite(GreenLed,LOW);
         digitalWrite(RedLed,HIGH);
      }
      counter = 0;                  // reset digit counter, ready to enter new code
      strcpy(code,"    ");          // reset code string
      digitalWrite(KeyLed,HIGH);
      delay(400);                   // long flash = 4 digits entered
      digitalWrite(KeyLed,LOW);
   }
    // end main loop \
}


[sokoloff]

What is supposed to happen if you enter 1234 after entering 1 (or some other buttons)?

Presumably, the lock is meant to open, rather than remaining perma-locked...

[Bruce Abbott]

What is supposed to happen if you enter 1234 after entering 1 (or some other buttons)?

Once you start, you have to keep going until exactly 4 digits have been entered. That's why I added an LED to show when the complete code has been received (as well as when each digit is pressed). I could have used the green LED for this, but I presumed that red means on and green means off.

If lolimpol wanted it to work some other way then he should have told us...


 

[sokoloff]

Sorry. Wasn't meant as a criticism of your code (though I can see how you read it that way), but rather a specification clarification question.

[hamster_nz]

Once you get it working with 1,2,3,4 as the combination, see if you can you have 1,1,1,1 or 1,2,2,1 as the valid combination.

Unless you you think really really hard about how your code processes the button presses it is quite hard to get this to work cleanly.

[janekm]

As an extra credit question, why shouldn't you use strcmp in this case (assuming it's a lock on something worth protecting)?   

[hamster_nz]

As an extra credit question, why shouldn't you use strcmp in this case (assuming it's a lock on something worth protecting)?   

You can get by with just two bytes of state if you really want to secure it. At least you can then truly verify/audit all the code (if it is really worth protecting)...

[lolimpol]

I really want to learn, but now is not the time!

Have you heard the expression "necessity is the mother of invention"? Being under pressure is exactly the time to learn!

Here's some working code I made just for you. This is only the second program I have written for Arduino, so it was a learning experience for me too.

Just one catch though - there's a deliberate mistake in it. If you can figure out what it is then you're good to go!

Code: [Select]

//  4 button combination lock for Arduino 
//  by Bruce Abbott       [http://www.bhabbott.net.nz]

#define button1  8       
#define button2  9
#define button3  10
#define button4  11   

#define KeyLed   3    // button press indicator  (long flash = 4th digit entered)
#define RedLed   4    // on when locked, off when unlocked   
#define GreenLed 12   // on when unlocked, off when locked

#define LOCKCODE1   "1234"
#define LOCKCODE2   "4321"

#define UNLOCKCODE1 "4231"
#define UNLOCKCODE2 "1432"

int bS1 = 0;         // current state of button
int lbS1 = 0;        // previous state of button

int bS2 = 0;         
int lbS2 = 0;

int bS3 = 0;         
int lbS3 = 0;

int bS4 = 0;         
int lbS4 = 0;

int counter = 0;       // number of buttons pressed in current sequence

int lockstate = 0;     // 0 = unlocked, 1 locked

char code[] = "    ";  // string to hold 4 digit code

void setup(){
  pinMode(button1, INPUT);
  pinMode(button2, INPUT);
  pinMode(button3, INPUT);
  pinMode(button4, INPUT);
  pinMode(KeyLed, OUTPUT);
  pinMode(RedLed, OUTPUT);
  pinMode(GreenLed, OUTPUT);
  Serial.begin(9600);
//  digitalWrite(GreenLed,HIGH);
}

void loop(){

    bS1 = digitalRead(button1);
    bS2 = digitalRead(button2);
    bS3 = digitalRead(button3);
    bS4 = digitalRead(button4);

    if (bS1 != lbS1) {
       if ((bS1 == HIGH) && (bS2 == LOW) && (bS3 == LOW) && (bS4 == LOW)) {
          code[counter]='1';
          counter = counter + 1;
          digitalWrite(KeyLed,HIGH);
          Serial.println(code);
       }
       lbS1 = bS1;
       delay(100);
       digitalWrite(KeyLed,LOW);
    }

    if (bS2 != lbS2) {
       if ((bS1 == LOW) && (bS2 == HIGH) && (bS3 == LOW) && (bS4 == LOW)) {
          code[counter]='2';
          counter = counter + 1;
          digitalWrite(KeyLed,HIGH);
          Serial.println(code);
          }
       lbS2 = bS2;
       delay(100);
       digitalWrite(KeyLed,LOW);
    }

    if (bS3 != lbS3) {
       if ((bS1 == LOW) && (bS2 == LOW) && (bS3 == HIGH) && (bS4 == LOW)) {
          code[counter]='3';
          counter = counter + 1;
          digitalWrite(KeyLed,HIGH);
          Serial.println(code);
       }
       lbS3 = bS3;
       delay(100);
       digitalWrite(KeyLed,LOW);
    }

    if (bS4 != lbS4) {
       if ((bS1 == LOW) && (bS2 == LOW) && (bS3 == LOW) && (bS4 == HIGH)) {
          code[counter]='4';
          counter = counter + 1;
          digitalWrite(KeyLed,HIGH);
          Serial.println(code);
       }
       lbS4 = bS4;
       delay(100);
       digitalWrite(KeyLed,LOW);
    }

   if (counter == 4) {
      if (!strcmp(code,UNLOCKCODE1) || !strcmp(code,UNLOCKCODE2)) {
         lockstate = 0;
         digitalWrite(GreenLed,HIGH);
         digitalWrite(RedLed,LOW);
      }
      if (!strcmp(code,LOCKCODE1) || !strcmp(code,LOCKCODE2)) {
         lockstate = 1;
         digitalWrite(GreenLed,LOW);
         digitalWrite(RedLed,HIGH);
      }
      counter = 0;                  // reset digit counter, ready to enter new code
      strcpy(code,"    ");          // reset code string
      digitalWrite(KeyLed,HIGH);
      delay(400);                   // long flash = 4 digits entered
      digitalWrite(KeyLed,LOW);
   }
    // end main loop \
}


Perfect! I will do my absolute best to find it!
edit: I think I found it,

Code: [Select]

      if (!strcmp(code,LOCKCODE1) || !strcmp(code,LOCKCODE2)) {
         lockstate = 1;
         digitalWrite(GreenLed,LOW);
         digitalWrite(RedLed,HIGH);
      }
      counter = 0;                  // reset digit counter, ready to enter new code
      strcpy(code,"    ");          // reset code string
      digitalWrite(KeyLed,HIGH);
      delay(400);                   // long flash = 4 digits entered
      digitalWrite(KeyLed,LOW);
   }} //<-- one more closing bracket? that's it I think
    // end main loop \
}


[hamster_nz]

You could do something like this, but it would be a little too slick for homework... somebody would know you are pulling their leg.

Code: [Select]

const int code1 = 1*(5*5*5) + 2*(5*5) + 3*5 + 4; // Code of 1234
const int code2 = 4*(5*5*5) + 3*(5*5) + 2*5 + 1; // Code of 4321

int readButtons(void) {
   static int buttons;
   int buttonsLast;

   buttonsLast = buttons;
   buttons = 0;
   if(digitalRead(btn1Pin)) buttons |= 1;
   if(digitalRead(btn2Pin)) buttons |= 2;
   if(digitalRead(btn3Pin)) buttons |= 4;
   if(digitalRead(btn4Pin)) buttons |= 8;

   if(buttonsLast != 0)
     return 0;

   switch(buttons) {
case 1:  return 1;
case 2:  return 2;
case 4:  return 3;
case 8:  return 4;
        default: return 0;
   }
}

void loop() {
   static unsigned int state = 0;
   static int timeout = 0;
   int buttons;

   buttons = readButtons(); // Returns 0 (no button) or 1 through 4
   if(buttons == 0) {
     if(timeout == timeout_max)
       state = 0;
     else
       timeout++;
   } else {
     timeout = 0;
     state = state * 5 + buttons;
   }

   if (state == code1 || state == code2) {
     digitalWrite(errorPin, LOW);
     digitalWrite(unlockPin,HIGH);
   } else if( state > 5*5*5) {
     digitalWrite(errorPin, HIGH);
     digitalWrite(unlockPin,LOW);
     state = 5*5*5*5;
   } else {
     digitalWrite(errorPin, LOW);
     digitalWrite(unlockPin,LOW);
   }
   delay(100); // pause for 1/10th of a second
}

You would need to add your own setup() and set the pin constants and so on

[CM800]

You could do something like this, but it would be a little too slick for homework... somebody would know you are pulling their leg.

I'd agree with that, haha.


What did you mean about the problem with comparing strings that I shown earlier??

I've never had a problem with it!

 

[hamster_nz]

You could do something like this, but it would be a little too slick for homework... somebody would know you are pulling their leg.

I'd agree with that, haha.


What did you mean about the problem with comparing strings that I shown earlier??

I've never had a problem with it!

 

Just the usual issue with beginners comparing points for equivilence, not comparing the contents that they point to.

Code: [Select]

#include <stdio.h>

char test[5] = "abcd";

int main(int argc, char *argv[]) {

   if(test == "abcd")
     printf("Matched '=='\n");
   else
     printf("Did not match '=='\n");

   if(strcmp(test,"abcd")==0)
     printf("Matched 'strcmp()'\n");
   else
     printf("Did not match 'strcmp()'\n");

   return 0;
}

gives

Code: [Select]

C:\Users\Hamster\Desktop\string>strtest
Did not match '=='
Matched 'strcmp()'

C:\Users\Hamster\Desktop\string>


[lolimpol]

well, the code isn't the homework... I could have made a drawing basically, but I decided to be creative.

[Bruce Abbott]

edit: I think I found it,

Code: [Select]

      if (!strcmp(code,LOCKCODE1) || !strcmp(code,LOCKCODE2)) {
         lockstate = 1;
         digitalWrite(GreenLed,LOW);
         digitalWrite(RedLed,HIGH);
      }
      counter = 0;                  // reset digit counter, ready to enter new code
      strcpy(code,"    ");          // reset code string
      digitalWrite(KeyLed,HIGH);
      delay(400);                   // long flash = 4 digits entered
      digitalWrite(KeyLed,LOW);
   }} //<-- one more closing bracket? that's it I think
    // end main loop \
}


Congratulations! Now for extra points, explain why the closing bracket on the last line isn't doing the job.

Hint

 

[lolimpol]

edit: I think I found it,

Code: [Select]

      if (!strcmp(code,LOCKCODE1) || !strcmp(code,LOCKCODE2)) {
         lockstate = 1;
         digitalWrite(GreenLed,LOW);
         digitalWrite(RedLed,HIGH);
      }
      counter = 0;                  // reset digit counter, ready to enter new code
      strcpy(code,"    ");          // reset code string
      digitalWrite(KeyLed,HIGH);
      delay(400);                   // long flash = 4 digits entered
      digitalWrite(KeyLed,LOW);
   }} //<-- one more closing bracket? that's it I think
    // end main loop \
}


Congratulations! Now for extra points, explain why the closing bracket on the last line isn't doing the job.

Hint

because there's a ` before the text?
(the code didn't work...)

[stj]

so much posts over such a simple task.

you setup a 4byte fifo buffer,
each time you read a keypress you shift the buffer left 1 position and put the keypress in the rightmost place.
then you call a compare routine to see if it matches the target string.


that way, you could have 145343222341234 and it would work.
if you dont do it that way, you either need a [clear] button to start the entry, or you need a timeout function to clear the buffer if nothing is entered for a few seconds.

[mariush]

Use an array of 4 bytes / char , initialize them with 0 at the start.
Every time a key is pressed ,  move the four bytes to the left by one  ... ex value[0] = value[1]; value[1] = value[2]; value[2] = value[3];
put the read button in the last position ... value [3] = button;
optionally if the button is "C" for clear or reset , set all the 4 values in the array to 0
After every key is pressed or when a special key is pressed (an enter for example) ... do  value[0] * 1000 + value[1] * 100 + value[2] * 10 + value[3] ; and you have a variable that is a number and can be stored in 2 bytes (unsigned int or something like that).
For reverse , you simply do value[3] * 1000 + value[2]*100 + value[1] * 10 + value[0] and compare it with another pre-stored variable.

You can compare that variable with a number you store in memory in the microcontroller and if they match then you unlock the thing.

This adds more cpu cycles at every key press (as you have to shift those 4 bytes every time to put the last key press in the 4th spot in the array) but saves memory since you don't have to store the password inside as a 4 character string, you can save it as a 2 byte constant number  , and you also don't have to compare two strings .. you compare two numbers.

If you want to reduce memory usage even further, you can use a single 2 byte variable and use 4 bits for every key press .. 4 bits give you ability to store 0..15 in them , so you can easily store 0..9 in 4 bits.
Every time a button is pressed, you shift the value to the left by 4 bits and you add your button (0..9) to the end of the unsigned 2 byte variable.
After every change or after user presses reset , you compare the 2 byte variable to another pre-stored variable in memory.

Example ... let's say the code is   1639   ... 1 in binary as 0001  , 6 is 0110 , 3 is 0011 and 9 is 1001 so  you will have to compare whatever is entered against 00010110 00111001  or 5689 in decimal (stored as unsigned integer 2 bytes.

So  now yuo have a unsigned integer variable that's initially 0.

Code: [Select]

User presses  "1"  ... value = value << 4 + 1  === >      0 in binary is 0000 0000 0000 0000  and shifted to left by 4 bits is 0000 0000 0000 0000  and then you add 1 :  0000 0000 0000 0001  (1)
User presses  "6" ...  value = value << 4 + 6  === >      1 in binary is 0000 0000 0000 0001  and shifted to left by 4 bits is 0000 0000 0001 0000  and then you add 6 :  0000 0000 0001 0110 (22)
User presses  "3" ...  value = value << 4 + 3  === >     22 in binary is 0000 0000 0001 0110  and shifted to left by 4 bits is 0000 0001 0110 0000  and then you add 3 :  0000 0001 0110 0011 (355)
User presses  "9" ...  value = value << 4 + 9  === >    355 in binary is 0000 0001 0110 0011  and shifted to left by 4 bits is 0001 0110 0011 0000  and then you add 9 :  0001 0110 0011 1001 (5689)
After every digit (or when user presses ENTER) you compare the value with the value you originally stored (5689) and you can see then when user eventually presses 1639 in sequence, the value computed will be 5689 and will match with the pre-stored value.
Shifting and adding is done super fast, just a couple of cycles or something like that.. faster than moving bytes in arrays and since the variable can only store 4x4 bits at any time, it works perfectly for 4 digit codes. If you want longer pin codes, you'd have to resort to 4 byte variables (which would be able to store 8 digits) and you'd probably have to use a CLEAR or reset button to reset the variable to 0 if user makes a mistake.
With the above code that just keeps in memory the last 4 digits, if user makes a mistake he can simply start from the beginning knowing that only the last 4 button presses are memorized (for example if user pushes 8211639 and then ENTER, it would be a correct pin since only the last four numbers were memorized.

IF you want the code to work the other way around (for example if you want 9361 to also work, you just store a second value in memory (9361 using 4 bits for every digit) : 1001 0011 0110 0001  or 37,729 in decimal (unsigned 2 bytes)
and now every time a button is pushed, you shift the previously memorized value to the right by 4 bits and you store the button in the first 4 bits :

reverse_value = reverse_value >> 4;
reverse_value = reverse_value + (button << 12)  - shift [0..9] to the left 12 bits and then add value to it ..

[stj]

the code saved by storing as 2bytes is far less than the code needed to implement it.
using a high level language kind of masks such things, but i mostly code in assembly language.

[NivagSwerdna]

I would recommend doing some research on finite state machines, it can really clarify your code in situations like this.

e.g. consider the lock to start in a locked state, say state 0... red on

a specific correct button press takes you to state 1, a bad one takes to back to state 0

subsequently a correct button takes you to state 2, a bad one to state 0

etc

eventually the lock will reach the open state.

[Bruce Abbott]

because there's a ` before the text?

There's a '\' at the end of the previous line. This causes the two lines to be merged, so to the compiler it looks like this...

Code: [Select]

// end main loop }
...and the closing bracket is now part of the comment so the compiler ignores it.

(the code didn't work...)

Be more specific. What do you mean by "didn't work"?

[hamster_nz]

I would recommend doing some research on finite state machines, it can really clarify your code in situations like this.

e.g. consider the lock to start in a locked state, say state 0... red on

a specific correct button press takes you to state 1, a bad one takes to back to state 0

subsequently a correct button takes you to state 2, a bad one to state 0

etc

eventually the lock will reach the open state.

I would conjecture that processing it as a stream of button presses is a far cleaner implementation. The logic this is:
- Get the next button press
- Do the last X button presses have no more than Y seconds between presses?
- Do the last X button presses match the secret code?
- If the last two questions are true, then take the action desired for that secret code

You can then do lots of extra things you can't do cleanly with an FSM implemented solution, like have multiple different codes of different lengths

[Leiothrix]

You also need a count for the number of buttons pressed and a timeout to reset that counter.

You don't want someone sitting there going through all the combinations to get the correct one, so you need a way to either lockout entirely or have a delay between code attempts.

[lolimpol]

because there's a ` before the text?

There's a '\' at the end of the previous line. This causes the two lines to be merged, so to the compiler it looks like this...

Code: [Select]

// end main loop }
...and the closing bracket is now part of the comment so the compiler ignores it.

(the code didn't work...)

Be more specific. What do you mean by "didn't work"?

nothing, the red led doesn't light, neither does the green, or the onboard (all keys entered led) when I press the buttons nothing hapens either, but maybe I should say that the buttons are hooked to gnd with 10k resistors, maybe that's worng?

[Bruce Abbott]

I should say that the buttons are hooked to gnd with 10k resistors, maybe that's worng?

The convention is to have the resistors going to +5V and buttons pulling to Ground (so button pressed = LOW), but your way also works. My test setup is attached below (I used a jumper to simulate the buttons).

If you have a multimeter then measure the voltage on each input pin while pressing the button (should go from 0V to +5V). If you don't have a meter then wire an LED (with current-limiting resistor in series) across the 10k resistor. The LED should light when you press the button.   

The program sends the currently entered code to the serial port each time you press a button. Open the serial monitor in Arduino IDE (menu item 'tools/serial monitor') and you should see the numbers.   
 

[stj]

keep in mind, this is a school project.
it must be simple, clear, and easy for others to understand.

[janekm]

As an extra credit question, why shouldn't you use strcmp in this case (assuming it's a lock on something worth protecting)?   

You can get by with just two bytes of state if you really want to secure it. At least you can then truly verify/audit all the code (if it is really worth protecting)...

That would also be a good reason. What i was getting at is that strcmp will stop the comparison as soon as it sees a non-matching character, so it will take less time to execute than with a valid character, allowing for a timing attack on the lock (as explained in Dave's video on the electronic lock box).

[hamster_nz]

As an extra credit question, why shouldn't you use strcmp in this case (assuming it's a lock on something worth protecting)?   

You can get by with just two bytes of state if you really want to secure it. At least you can then truly verify/audit all the code (if it is really worth protecting)...

That would also be a good reason. What i was getting at is that strcmp will stop the comparison as soon as it sees a non-matching character, so it will take less time to execute than with a valid character, allowing for a timing attack on the lock (as explained in Dave's video on the electronic lock box).

Good point!

I though you were hinting at a beginner not null terminating the non-constant string, or declaring as a four element array and not five, causing nice security issue and/or a subtle bug.

I was worried that you were going to rattle the "use the more secure strncmp()" cage. 

[daveshah]

I was worried that you were going to rattle the "use the more secure strncmp()" cage. 

On that note (slightly OT but interesting)

http://www.theregister.co.uk/2017/05/05/intel_amt_remote_exploit/, about halfway down

[Bruce Abbott]

Code: [Select]

if(strncmp(computed_response, user_response, response_length))
   deny_access();
 

Unfortunately, response_length is calculated from user_response, so if an empty string is supplied, the length is zero, no bytes are checked, no bytes are therefore different, and – as expected – strncmp() returns zero, indicating success, and access is granted. Thus, an empty response string slips through as valid when it's actually invalid.

Here we are worrying about some theoretical vulnerability in a device that will never be subject to sophisticated attacks, while the biggest chip manufacturer in the world makes a blunder so basic a 5 year old could hack their products.   

But hey, Intel made $59.4 Billion last year - so you can trust them. 
 

[janekm]

(snip)

Here we are worrying about some theoretical vulnerability in a device that will never be subject to sophisticated attacks, while the biggest chip manufacturer in the world makes a blunder so basic a 5 year old could hack their products.   

But hey, Intel made $59.4 Billion last year - so you can trust them. 
 

I would argue we are exposing a beginner to the kind of thinking that may one day help them from stumbling into that kind of blunder.

[stj]

the only blunder was getting found out.

it takes some planning to get an arm-core cpu running on standby power 24/7 with access to ram, drive interfaces and ethernet (that also has standby power)

wait till the public finds out about the built-in wireless access port running over the 3g network!!!

[hamster_nz]

the only blunder was getting found out.

it takes some planning to get an arm-core cpu running on standby power 24/7 with access to ram, drive interfaces and ethernet (that also has standby power)

wait till the public finds out about the built-in wireless access port running over the 3g network!!!

The hard bit is making it look like a stupid mistake, not a deliberate feature that was designed in....

[Bruce Abbott]

The hard bit is making it look like a stupid mistake, not a deliberate feature that was designed in....

Then they failed!

In order for us to believe that is was just a stupid mistake, we would have to accept that a) a multi-billion dollar company would let any bozo code critical parts of their products, b) they did no security testing, and c) they didn't think such incompetence would get them into trouble.

In today's environment this is totally unthinkable, therefore the only logical conclusion is that it was done deliberately. Though I don't think Intel itself was to blame, I think they are the victim of a sophisticated hack by an insider. That guy had better hope we don't find out who he is!

[stj]

they dont need to be hacked, they just need to be told by the government to do it and keep their mouths shut, or go to prison for ever.
they can do that under "national security" laws.

[David Chamberlain]

Here's mine, what a fun game 

Code: [Select]

uint16_t input = 0xFFFF; // any invalid init.
uint16_t code = (1 << 12) | (2 << 8) | (3 << 4) | (4 << 0); // code 1234

// keypress returns one when four numbers codes are provided.
// key_code can be number 0 to 9 or ascii code '0' to '9' it's all the same.
uint8_t keypress(uint8_t key_code)
{
input <<= 4;
input |= (key_code & 0x0F);
if(input==code) return 1;
return 0;
}

[EDIT] Yeah, basically what @mariush said

[Bruce Abbott]

they dont need to be hacked, they just need to be told by the government to do it and keep their mouths shut, or go to prison for ever.
they can do that under "national security" laws.

So they put in a back door that a 5 year old could find in 2 seconds? Still going to prison forever!

[stj]

if they put in a backdoor with a password they couldnt deny it if it got found out.

[hamster_nz]

Not that it is Arduino, but it is a combination lock.

A few months ago I built a "enter four digits to light the LED" project for an FPGA development board. So if anybody is interested how this sort of project can be implemented in VHDL I've uploaded the source at http://hamsterworks.co.nz/mediawiki/index.php/Combination_Lock

Here's a short video of it in action:
https://youtu.be/1kWDc_u6D_s