8 ms·

The "security" of this device is a joke, just look at how randomness is derived: unsigned int analog1 = analogRead(ANALOGPIN1); RNG.stir((uint8_t *)analog1

by Tharre 7y ago

The "security" of this device is a joke, just look at how randomness is derived: unsigned int analog1 = analogRead(ANALOGPIN1); RNG.stir((uint8_t *)analog1, sizeof(analog1), sizeof(analog1)*2); unsigned int analog2 = analogRead(ANALOGPIN2); RNG.stir((uint8_t *)analog2, sizeof(analog2), sizeof(analog2)*2); (See [0] for a comprehensive summary of why this is a terrible thing to do) And yeah, analogRead() is a function from the Arduino library because .. well, apparently there's an Arduino compatible chip inside that does all the cryptographic operations. Meaning that there is no hardware security whatsoever and it's trivial to extract all your keys from the device if you ever lose it. Whoops. [0] https://arxiv.org/pdf/1212.3777.pdf https://arxiv.org/pdf/1212.3777.pdf

tinus_hn 7y ago
Did I misunderstand the article or is it just wrong? Although a certain entropy source might not be completely random, it can still be a part of of a complete solution, right? Network packet timings aren’t random either and might be attacker controlled as well.
g_p 7y ago
Doing a quick look through the library repository, I spotted another more scary function. It doesn't appear (at a quick glance) to be used anywhere, but still... https://github.com/trustcrypto/libraries/blob/master/randombytes/randombytes.c https://github.com/trustcrypto/libraries/blob/master/randomb... For anyone wanting to try this out (it will compile with plain GCC if you add): #include <stdlib.h> #include <stdio.h> to the start of the file, and declare a main function: void main() { unsigned char* buffer; buffer = malloc(32 * sizeof(char)); randombytes(buffer, 32); for (int i=0; i < 32; i++) { printf("%02x", buffer[i]); } printf("\n"); free(buffer); } And you'll (of course) get some rather deterministic output. As I say though, doesn't look to be used (that I could see), but strange to have something like this there. RE the RNG implementation, looks to be at https://github.com/trustcrypto/libraries/blob/master/Crypto/RNG.cpp https://github.com/trustcrypto/libraries/blob/master/Crypto/..., and looks to have some support for hardware RNGs on certain boards, but not others. Does sound like there's no hardware protection involved.
- cr7pt0 7y ago
  If you search the repo you can see the randombytes library is just there for reference and isn't used, that library was created by the same guy who made NaCl https://en.wikipedia.org/wiki/NaCl_(software) https://en.wikipedia.org/wiki/NaCl_(software)
05 7y ago
It's not 'trivial' to extract the keys - all modern uCs have flash readout protection bits. It's probably easier to do than to read the secure element from your iPhone or extract keys from your SIM card or your credit cards, but it's not something you can do without specialized skills and equipment (although there are companies that provide commercial flash readout services).
- IshKebab 7y ago
  Given that they're using the Arduino APIs, what are the chances they know what flash readout protection is?
- marcan_42 7y ago
  Flash readout protection on most microcontrollers is a joke. They are almost always vulnerable to attacks ranging from power/clock glitching to asking nicely with the right combination of flash management commands (I'm looking at you, some PICs from the PIC18 series with blockwise erasable Flash including protection bits). I've seen some things disable their read protection by accident because the power supply wasn't hooked up properly and they glitched themselves. There's a reason we have real secure elements with anti-tamper mechanisms. The problem is that as far as I know there aren't any that you can develop for without signing an NDA.
  - RL_Quine 7y ago
    I've signed a ot of these NDAs. Dirty little secret, most of them are DUAL_ED_DRBG which is backdoored. None of them have any meaningful protection, and usually they have sidehcannels the size of mountains. There's none of the secure element chips I would consider to be stronger than cryptography in software. They're the same as passing certifications, good to corporate management but a joke to anybody who knows what they're talking about.
    - blattimwind 7y ago
      Can you comment, perhaps vaguely, on the Infineon SLE 78 series?
cr7pt0 7y ago
You might want to check out this - https://docs.crp.to/security.html#cryptographically-secure-random-number-generator https://docs.crp.to/security.html#cryptographically-secure-r... If you read further into the source you will see that analog read is only one of the sources of entropy, it uses capacitive touch from a user's skin and this TRNG passed dieharder tests - https://webhome.phy.duke.edu/~rgb/General/dieharder.php https://webhome.phy.duke.edu/~rgb/General/dieharder.php
- tptacek 7y ago
  Passing dieharder doesn't mean anything at all with respect to cryptographic security. It's trivial to define a random bit generator that passes randomness tests and has no real security.
  - nemonemo 7y ago
    I would like to learn more about practical cryptographic issues, and I need some help: what are the tests that can prove or disprove stronger guarantees for cryptographic security of a PRG than diehard? A wikipedia page doesn't give me much info about which one provides stronger guarantee and in which criteria: https://en.wikipedia.org/wiki/Randomness_tests https://en.wikipedia.org/wiki/Randomness_tests Also, which level of security of a PRG is sufficient for keys like this?
    - tptacek 7y ago
      There aren't such tests, at least not that work like dieharder. You analyze a CSPRNG the same way you'd analyze a cipher construction (they are essentially the same thing, and often we draw our conclusions about the strength of a CSPRNG by noticing that it's built on and thus inherits the formal security commitments of ciphers and hashes run in modes and constructions that themselves have been shown to be trustworthy).
    - JohnStrangeII 7y ago
      There are no automated tests for this, since cryptographic randomness requires unpredictability. A statistical test can only tell you when a random number generator is broken, but no statistical test can tell you whether a random number generator is cryptographically sound.
klhugo 7y ago
I have to agree with you. What impresses me even more is that they are selling it already, and marketing as “open-source”. I would leave a note here that if anybody is interested in doing something similar, please get some feedback from community before starting commercialization.
- lonelappde 7y ago
  There's a dead simple "quack/crank" test for security products. If it hasn't been publicly discussed and analyzed for at least year, but is already for sale as a "usable device" not a "prototype", the seller is either fraud or a fool, and regardless of which, is not to be trusted.
  - cr7pt0 7y ago
    OnlyKey has been in use for about 4 years. It has thousands of active users and is in use in over 40 countries world wide. This is not a new product, and it has a great user community which is not afraid to test, hack, and prove the security of devices.
    - delfinom 7y ago
      >OnlyKey has been in use for about 4 years. It has thousands of active users and is in use in over 40 countries world wide. This is not a new product, and it has a great user community which is not afraid to test, hack, and prove the security of devices. Like literally the first issue was already linked above. Using the psuedo RNG with some analog pin seed isn't really acceptable. It should have a true rng IC that can generate real random numbers from diode bandgap noise or other sources.
      - cr7pt0 7y ago
        Like literally the first post issue is completely incorrect, thats one of the issues in reading a post like this in an online thread, literally that user copied part of but not all of the function that is used for RNG. The part they copied uses analog input as one of the sources of entropy, they failed to also include the 6 capacitive touch inputs that are also inputs to the RNG. Those touch inputs literally change every time you press a button and even with atmospheric changes, i.e. it's cloudy out today, your RNG has changed. RNG.stir((uint8_t )analog1, sizeof(analog1), sizeof(analog1) 4); touchread1 = touchRead(TOUCHPIN1); RNG.stir((uint8_t )touchread1, sizeof(touchread1), sizeof(touchread1)); delay((analog1 % 3) + ((touchread1 + touchread2 + touchread3) % 3)); //delay 0 - 6 ms integrityctr1++; touchread2 = touchRead(TOUCHPIN2); RNG.stir((uint8_t )touchread2, sizeof(touchread2), sizeof(touchread2)); touchread3 = touchRead(TOUCHPIN3); RNG.stir((uint8_t )touchread3, sizeof(touchread3), sizeof(touchread3)); touchread4 = touchRead(TOUCHPIN4); RNG.stir((uint8_t )touchread4, sizeof(touchread4), sizeof(touchread4)); touchread5 = touchRead(TOUCHPIN5); RNG.stir((uint8_t )touchread5, sizeof(touchread5), sizeof(touchread5)); touchread6 = touchRead(TOUCHPIN6); RNG.stir((uint8_t )touchread6, sizeof(touchread6), sizeof(touchread6)); unsigned int analog2 = analogRead(ANALOGPIN2); RNG.stir((uint8_t )analog2, sizeof(analog2), sizeof(analog2) 4); // Perform regular housekeeping on the random number generator. RNG.loop(); delay((analog2 % 3) + ((touchread6 + touchread5 + touchread4) % 3)); //delay 0 - 6 ms integrityctr2++; if (integrityctr1 != integrityctr2) { //Integrity Check unlocked = false; CPU_RESTART(); return; } https://github.com/trustcrypto/libraries/blob/5bd1f8eb15eb0463487089f9df531f3384286886/onlykey/okcore.cpp#L2259 https://github.com/trustcrypto/libraries/blob/5bd1f8eb15eb04...
marcan_42 7y ago
It seems this is literally written in the horrible Arduino "everything in one huge file" style: https://github.com/trustcrypto/OnlyKey-Firmware/blob/master/OnlyKey_Beta/OnlyKey_Beta.ino https://github.com/trustcrypto/OnlyKey-Firmware/blob/master/... The funny thing is they have a "Source code reviewed by Codacy" badge on the readme claiming the code is grade A... but if you actually click through, of course Codacy didn't pick up the .ino file at all, so in fact nothing of substance is being reviewed. That .ino file wouldn't pass any style review... it's a mess. Anyway, looks like that firmware is incomplete (e.g. "onlykey.h" is missing). Just a quick scroll through the code gives me zero confidence in this thing, code quality wise. Someone who can't consistently indent code almost certainly isn't qualified to be writing security-critical software. Edit: looks like the rest of the code is here, and yeah, it doesn't inspire much confidence (7000+ lines of code in okcore.cpp, ouch): https://github.com/trustcrypto/libraries/tree/master/onlykey https://github.com/trustcrypto/libraries/tree/master/onlykey
- blattimwind 7y ago
  > The funny thing is they have a "Source code reviewed by Codacy" badge on the readme claiming the code is grade A... but if you actually click through, of course Codacy didn't pick up the .ino file at all, so in fact nothing of substance is being reviewed. That .ino file wouldn't pass any style review... it's a mess. Meanwhile Nitrokey has actually been audited by Cure53.
  - 0x0359463 7y ago
    The "security" of Nitrokey is a joke. As others have mentioned they were actually hacked https://old.reddit.com/r/crypto/comments/bis3pf/extract_pgp_secret_keys_from_gnuk_nitrokey_start/ https://old.reddit.com/r/crypto/comments/bis3pf/extract_pgp_... Nitrokey does not support half of the features OnlyKey does and even the users on their own forum prefer OnlyKey -https://support.nitrokey.com/t/nitrokey-vs-onlykey/638 https://support.nitrokey.com/t/nitrokey-vs-onlykey/638
- Stratoscope 7y ago
  The 7000 lines don't bother me as much as the complete lack of refactoring, heavy use of magic numbers repeated throughout, and logical expressions that duplicate logic over and over again. Some examples... Compare these two blocks of assignments and memcpy calls: https://github.com/trustcrypto/libraries/blob/5bd1f8eb15eb0463487089f9df531f3384286886/onlykey/okcore.cpp#L2384 https://github.com/trustcrypto/libraries/blob/5bd1f8eb15eb04... https://github.com/trustcrypto/libraries/blob/5bd1f8eb15eb0463487089f9df531f3384286886/onlykey/okcore.cpp#L2488 https://github.com/trustcrypto/libraries/blob/5bd1f8eb15eb04... Yes, they are as identical as they appear. The only differences (other than a couple of lines commented out in one) are the use of 'data' in the first and 'large_resp_buffer+offset' in the second, along with some arbitrary whitespace differences. (The first uses spaces around the + operators, the second does not.) And all the hard coded numbers! What do they mean? Or this block of code that appears to be a limited version of a decimal number formatter: https://github.com/trustcrypto/libraries/blob/5bd1f8eb15eb0463487089f9df531f3384286886/onlykey/okcore.cpp#L1418 https://github.com/trustcrypto/libraries/blob/5bd1f8eb15eb04... Or this code that keeps checking the same flags over and over again instead of combining the tests: https://github.com/trustcrypto/libraries/blob/5bd1f8eb15eb0463487089f9df531f3384286886/onlykey/okcore.cpp#L626 https://github.com/trustcrypto/libraries/blob/5bd1f8eb15eb04... (Scroll horizontally to see all the repeated tests!) Or this code with the same logic repeated 24 times: https://github.com/trustcrypto/libraries/blob/5bd1f8eb15eb0463487089f9df531f3384286886/onlykey/okcore.cpp#L3301 https://github.com/trustcrypto/libraries/blob/5bd1f8eb15eb04... The next function after that one also has 24 copies of duplicate logic. Well, the logic isn't entirely duplicated. The individual cases call functions like onlykey_eeget_urllen1, onlykey_eeget_urllen2, ... onlykey_eeget_urllen24, and onlykey_eeset_urllen1, onlykey_eeset_urllen2, ... onlykey_eeset_urllen24. Here are those functions: https://github.com/trustcrypto/libraries/blob/527113dfeeb20e921f6c8cbf821fe94383843b21/onlykey/okeeprom.c#L700 https://github.com/trustcrypto/libraries/blob/527113dfeeb20e... Yes, they are all identical except for the different constants each one uses: https://github.com/trustcrypto/libraries/blob/527113dfeeb20e921f6c8cbf821fe94383843b21/onlykey/okeeprom.h#L141 https://github.com/trustcrypto/libraries/blob/527113dfeeb20e... This pattern of "24 copies of the same logic with different constants" occurs all through the code. Look through okeeprom.h/cpp for several other examples. None of this inspires confidence that the code can be trusted.
xani 7y ago
You probably will be getting at least one bit of randomness out of it if the input is not saturated (As in not below 0 or above reference voltage ADC is using), just because pretty much all ADC are noisy enough that the last bit will be flipping. Of course that doesn't excuse every other problem with it > Meaning that there is no hardware security whatsoever and it's trivial to extract all your keys from the device if you ever lose it. Whoops. Most micros, ones used in various Arduinos included have fuse bits so there is at least the minimal level of protection. Question whether they used it even...
lonelappde 7y ago
The linked paper breaks analogRead-based encryption in section 5 at the end. But I wonder if you could get better randomnes by, instead of naively pulling one low entropy 10-bit value from analogRead, you pulled 128 bits from successive analogReads and only kept the lowest significant bit.
baybal2 7y ago
A very simple hardware entropy source is a noisy diode. I wonder why so few people are using it
unwind 7y ago
Uh that is treating the value from the A/D conversion as an address, and reading that location for data to stir in. Unless HN markup ate the &s that are missing. That is very bad code indeed.

by Tharre 7y ago

tinus_hn 7y ago

Did I misunderstand the article or is it just wrong? Although a certain entropy source might not be completely random, it can still be a part of of a complete solution, right? Network packet timings aren’t random either and might be attacker controlled as well.

g_p 7y ago

Doing a quick look through the library repository, I spotted another more scary function. It doesn't appear (at a quick glance) to be used anywhere, but still... https://github.com/trustcrypto/libraries/blob/master/randombytes/randombytes.c https://github.com/trustcrypto/libraries/blob/master/randomb... For anyone wanting to try this out (it will compile with plain GCC if you add): #include <stdlib.h> #include <stdio.h> to the start of the file, and declare a main function: void main() { unsigned char* buffer; buffer = malloc(32 * sizeof(char)); randombytes(buffer, 32); for (int i=0; i < 32; i++) { printf("%02x", buffer[i]); } printf("\n"); free(buffer); } And you'll (of course) get some rather deterministic output. As I say though, doesn't look to be used (that I could see), but strange to have something like this there. RE the RNG implementation, looks to be at https://github.com/trustcrypto/libraries/blob/master/Crypto/RNG.cpp https://github.com/trustcrypto/libraries/blob/master/Crypto/..., and looks to have some support for hardware RNGs on certain boards, but not others. Does sound like there's no hardware protection involved.

cr7pt0 7y ago
If you search the repo you can see the randombytes library is just there for reference and isn't used, that library was created by the same guy who made NaCl https://en.wikipedia.org/wiki/NaCl_(software) https://en.wikipedia.org/wiki/NaCl_(software)

05 7y ago

It's not 'trivial' to extract the keys - all modern uCs have flash readout protection bits. It's probably easier to do than to read the secure element from your iPhone or extract keys from your SIM card or your credit cards, but it's not something you can do without specialized skills and equipment (although there are companies that provide commercial flash readout services).

IshKebab 7y ago
Given that they're using the Arduino APIs, what are the chances they know what flash readout protection is?
marcan_42 7y ago
Flash readout protection on most microcontrollers is a joke. They are almost always vulnerable to attacks ranging from power/clock glitching to asking nicely with the right combination of flash management commands (I'm looking at you, some PICs from the PIC18 series with blockwise erasable Flash including protection bits). I've seen some things disable their read protection by accident because the power supply wasn't hooked up properly and they glitched themselves. There's a reason we have real secure elements with anti-tamper mechanisms. The problem is that as far as I know there aren't any that you can develop for without signing an NDA.
- RL_Quine 7y ago
  I've signed a ot of these NDAs. Dirty little secret, most of them are DUAL_ED_DRBG which is backdoored. None of them have any meaningful protection, and usually they have sidehcannels the size of mountains. There's none of the secure element chips I would consider to be stronger than cryptography in software. They're the same as passing certifications, good to corporate management but a joke to anybody who knows what they're talking about.
  - blattimwind 7y ago
    Can you comment, perhaps vaguely, on the Infineon SLE 78 series?

cr7pt0 7y ago

You might want to check out this - https://docs.crp.to/security.html#cryptographically-secure-random-number-generator https://docs.crp.to/security.html#cryptographically-secure-r... If you read further into the source you will see that analog read is only one of the sources of entropy, it uses capacitive touch from a user's skin and this TRNG passed dieharder tests - https://webhome.phy.duke.edu/~rgb/General/dieharder.php https://webhome.phy.duke.edu/~rgb/General/dieharder.php

tptacek 7y ago
Passing dieharder doesn't mean anything at all with respect to cryptographic security. It's trivial to define a random bit generator that passes randomness tests and has no real security.
- nemonemo 7y ago
  I would like to learn more about practical cryptographic issues, and I need some help: what are the tests that can prove or disprove stronger guarantees for cryptographic security of a PRG than diehard? A wikipedia page doesn't give me much info about which one provides stronger guarantee and in which criteria: https://en.wikipedia.org/wiki/Randomness_tests https://en.wikipedia.org/wiki/Randomness_tests Also, which level of security of a PRG is sufficient for keys like this?
  - tptacek 7y ago
    There aren't such tests, at least not that work like dieharder. You analyze a CSPRNG the same way you'd analyze a cipher construction (they are essentially the same thing, and often we draw our conclusions about the strength of a CSPRNG by noticing that it's built on and thus inherits the formal security commitments of ciphers and hashes run in modes and constructions that themselves have been shown to be trustworthy).
  - JohnStrangeII 7y ago
    There are no automated tests for this, since cryptographic randomness requires unpredictability. A statistical test can only tell you when a random number generator is broken, but no statistical test can tell you whether a random number generator is cryptographically sound.

klhugo 7y ago

I have to agree with you. What impresses me even more is that they are selling it already, and marketing as “open-source”. I would leave a note here that if anybody is interested in doing something similar, please get some feedback from community before starting commercialization.

lonelappde 7y ago
There's a dead simple "quack/crank" test for security products. If it hasn't been publicly discussed and analyzed for at least year, but is already for sale as a "usable device" not a "prototype", the seller is either fraud or a fool, and regardless of which, is not to be trusted.
- cr7pt0 7y ago
  OnlyKey has been in use for about 4 years. It has thousands of active users and is in use in over 40 countries world wide. This is not a new product, and it has a great user community which is not afraid to test, hack, and prove the security of devices.
  - delfinom 7y ago
    >OnlyKey has been in use for about 4 years. It has thousands of active users and is in use in over 40 countries world wide. This is not a new product, and it has a great user community which is not afraid to test, hack, and prove the security of devices. Like literally the first issue was already linked above. Using the psuedo RNG with some analog pin seed isn't really acceptable. It should have a true rng IC that can generate real random numbers from diode bandgap noise or other sources.
    - cr7pt0 7y ago
      Like literally the first post issue is completely incorrect, thats one of the issues in reading a post like this in an online thread, literally that user copied part of but not all of the function that is used for RNG. The part they copied uses analog input as one of the sources of entropy, they failed to also include the 6 capacitive touch inputs that are also inputs to the RNG. Those touch inputs literally change every time you press a button and even with atmospheric changes, i.e. it's cloudy out today, your RNG has changed. RNG.stir((uint8_t )analog1, sizeof(analog1), sizeof(analog1) 4); touchread1 = touchRead(TOUCHPIN1); RNG.stir((uint8_t )touchread1, sizeof(touchread1), sizeof(touchread1)); delay((analog1 % 3) + ((touchread1 + touchread2 + touchread3) % 3)); //delay 0 - 6 ms integrityctr1++; touchread2 = touchRead(TOUCHPIN2); RNG.stir((uint8_t )touchread2, sizeof(touchread2), sizeof(touchread2)); touchread3 = touchRead(TOUCHPIN3); RNG.stir((uint8_t )touchread3, sizeof(touchread3), sizeof(touchread3)); touchread4 = touchRead(TOUCHPIN4); RNG.stir((uint8_t )touchread4, sizeof(touchread4), sizeof(touchread4)); touchread5 = touchRead(TOUCHPIN5); RNG.stir((uint8_t )touchread5, sizeof(touchread5), sizeof(touchread5)); touchread6 = touchRead(TOUCHPIN6); RNG.stir((uint8_t )touchread6, sizeof(touchread6), sizeof(touchread6)); unsigned int analog2 = analogRead(ANALOGPIN2); RNG.stir((uint8_t )analog2, sizeof(analog2), sizeof(analog2) 4); // Perform regular housekeeping on the random number generator. RNG.loop(); delay((analog2 % 3) + ((touchread6 + touchread5 + touchread4) % 3)); //delay 0 - 6 ms integrityctr2++; if (integrityctr1 != integrityctr2) { //Integrity Check unlocked = false; CPU_RESTART(); return; } https://github.com/trustcrypto/libraries/blob/5bd1f8eb15eb0463487089f9df531f3384286886/onlykey/okcore.cpp#L2259 https://github.com/trustcrypto/libraries/blob/5bd1f8eb15eb04...

marcan_42 7y ago

It seems this is literally written in the horrible Arduino "everything in one huge file" style: https://github.com/trustcrypto/OnlyKey-Firmware/blob/master/OnlyKey_Beta/OnlyKey_Beta.ino https://github.com/trustcrypto/OnlyKey-Firmware/blob/master/... The funny thing is they have a "Source code reviewed by Codacy" badge on the readme claiming the code is grade A... but if you actually click through, of course Codacy didn't pick up the .ino file at all, so in fact nothing of substance is being reviewed. That .ino file wouldn't pass any style review... it's a mess. Anyway, looks like that firmware is incomplete (e.g. "onlykey.h" is missing). Just a quick scroll through the code gives me zero confidence in this thing, code quality wise. Someone who can't consistently indent code almost certainly isn't qualified to be writing security-critical software. Edit: looks like the rest of the code is here, and yeah, it doesn't inspire much confidence (7000+ lines of code in okcore.cpp, ouch): https://github.com/trustcrypto/libraries/tree/master/onlykey https://github.com/trustcrypto/libraries/tree/master/onlykey

blattimwind 7y ago
> The funny thing is they have a "Source code reviewed by Codacy" badge on the readme claiming the code is grade A... but if you actually click through, of course Codacy didn't pick up the .ino file at all, so in fact nothing of substance is being reviewed. That .ino file wouldn't pass any style review... it's a mess. Meanwhile Nitrokey has actually been audited by Cure53.
- 0x0359463 7y ago
  The "security" of Nitrokey is a joke. As others have mentioned they were actually hacked https://old.reddit.com/r/crypto/comments/bis3pf/extract_pgp_secret_keys_from_gnuk_nitrokey_start/ https://old.reddit.com/r/crypto/comments/bis3pf/extract_pgp_... Nitrokey does not support half of the features OnlyKey does and even the users on their own forum prefer OnlyKey -https://support.nitrokey.com/t/nitrokey-vs-onlykey/638 https://support.nitrokey.com/t/nitrokey-vs-onlykey/638
Stratoscope 7y ago
The 7000 lines don't bother me as much as the complete lack of refactoring, heavy use of magic numbers repeated throughout, and logical expressions that duplicate logic over and over again. Some examples... Compare these two blocks of assignments and memcpy calls: https://github.com/trustcrypto/libraries/blob/5bd1f8eb15eb0463487089f9df531f3384286886/onlykey/okcore.cpp#L2384 https://github.com/trustcrypto/libraries/blob/5bd1f8eb15eb04... https://github.com/trustcrypto/libraries/blob/5bd1f8eb15eb0463487089f9df531f3384286886/onlykey/okcore.cpp#L2488 https://github.com/trustcrypto/libraries/blob/5bd1f8eb15eb04... Yes, they are as identical as they appear. The only differences (other than a couple of lines commented out in one) are the use of 'data' in the first and 'large_resp_buffer+offset' in the second, along with some arbitrary whitespace differences. (The first uses spaces around the + operators, the second does not.) And all the hard coded numbers! What do they mean? Or this block of code that appears to be a limited version of a decimal number formatter: https://github.com/trustcrypto/libraries/blob/5bd1f8eb15eb0463487089f9df531f3384286886/onlykey/okcore.cpp#L1418 https://github.com/trustcrypto/libraries/blob/5bd1f8eb15eb04... Or this code that keeps checking the same flags over and over again instead of combining the tests: https://github.com/trustcrypto/libraries/blob/5bd1f8eb15eb0463487089f9df531f3384286886/onlykey/okcore.cpp#L626 https://github.com/trustcrypto/libraries/blob/5bd1f8eb15eb04... (Scroll horizontally to see all the repeated tests!) Or this code with the same logic repeated 24 times: https://github.com/trustcrypto/libraries/blob/5bd1f8eb15eb0463487089f9df531f3384286886/onlykey/okcore.cpp#L3301 https://github.com/trustcrypto/libraries/blob/5bd1f8eb15eb04... The next function after that one also has 24 copies of duplicate logic. Well, the logic isn't entirely duplicated. The individual cases call functions like onlykey_eeget_urllen1, onlykey_eeget_urllen2, ... onlykey_eeget_urllen24, and onlykey_eeset_urllen1, onlykey_eeset_urllen2, ... onlykey_eeset_urllen24. Here are those functions: https://github.com/trustcrypto/libraries/blob/527113dfeeb20e921f6c8cbf821fe94383843b21/onlykey/okeeprom.c#L700 https://github.com/trustcrypto/libraries/blob/527113dfeeb20e... Yes, they are all identical except for the different constants each one uses: https://github.com/trustcrypto/libraries/blob/527113dfeeb20e921f6c8cbf821fe94383843b21/onlykey/okeeprom.h#L141 https://github.com/trustcrypto/libraries/blob/527113dfeeb20e... This pattern of "24 copies of the same logic with different constants" occurs all through the code. Look through okeeprom.h/cpp for several other examples. None of this inspires confidence that the code can be trusted.

xani 7y ago

You probably will be getting at least one bit of randomness out of it if the input is not saturated (As in not below 0 or above reference voltage ADC is using), just because pretty much all ADC are noisy enough that the last bit will be flipping. Of course that doesn't excuse every other problem with it > Meaning that there is no hardware security whatsoever and it's trivial to extract all your keys from the device if you ever lose it. Whoops. Most micros, ones used in various Arduinos included have fuse bits so there is at least the minimal level of protection. Question whether they used it even...

lonelappde 7y ago

The linked paper breaks analogRead-based encryption in section 5 at the end. But I wonder if you could get better randomnes by, instead of naively pulling one low entropy 10-bit value from analogRead, you pulled 128 bits from successive analogReads and only kept the lowest significant bit.

baybal2 7y ago

A very simple hardware entropy source is a noisy diode. I wonder why so few people are using it

unwind 7y ago

Uh that is treating the value from the A/D conversion as an address, and reading that location for data to stir in. Unless HN markup ate the &s that are missing. That is very bad code indeed.