-
Notifications
You must be signed in to change notification settings - Fork 505
Deduplicate crypto libraries, saving ~49KB flash #1632
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
ViezeVingertjes
wants to merge
2
commits into
meshcore-dev:dev
Choose a base branch
from
ViezeVingertjes:reduce-crypto-duplication
base: dev
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+3,076
−17
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,269 @@ | ||
| /* | ||
| * Copyright (C) 2015,2018 Southern Storm Software, Pty Ltd. | ||
| * | ||
| * Permission is hereby granted, free of charge, to any person obtaining a | ||
| * copy of this software and associated documentation files (the "Software"), | ||
| * to deal in the Software without restriction, including without limitation | ||
| * the rights to use, copy, modify, merge, publish, distribute, sublicense, | ||
| * and/or sell copies of the Software, and to permit persons to whom the | ||
| * Software is furnished to do so, subject to the following conditions: | ||
| * | ||
| * The above copyright notice and this permission notice shall be included | ||
| * in all copies or substantial portions of the Software. | ||
| * | ||
| * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS | ||
| * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||
| * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE | ||
| * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||
| * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING | ||
| * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER | ||
| * DEALINGS IN THE SOFTWARE. | ||
| */ | ||
|
|
||
| #ifndef CRYPTO_AES_h | ||
| #define CRYPTO_AES_h | ||
|
|
||
| #include "BlockCipher.h" | ||
|
|
||
| // Determine which AES implementation to export to applications. | ||
| #if defined(ESP32) | ||
| #define CRYPTO_AES_ESP32 1 | ||
| #else | ||
| #define CRYPTO_AES_DEFAULT 1 | ||
| #endif | ||
|
|
||
| #if defined(CRYPTO_AES_DEFAULT) || defined(CRYPTO_DOC) | ||
|
|
||
| class AESTiny128; | ||
| class AESTiny256; | ||
| class AESSmall128; | ||
| class AESSmall256; | ||
|
|
||
| class AESCommon : public BlockCipher | ||
| { | ||
| public: | ||
| virtual ~AESCommon(); | ||
|
|
||
| size_t blockSize() const; | ||
|
|
||
| void encryptBlock(uint8_t *output, const uint8_t *input); | ||
| void decryptBlock(uint8_t *output, const uint8_t *input); | ||
|
|
||
| void clear(); | ||
|
|
||
| protected: | ||
| AESCommon(); | ||
|
|
||
| /** @cond aes_internal */ | ||
| uint8_t rounds; | ||
| uint8_t *schedule; | ||
|
|
||
| static void subBytesAndShiftRows(uint8_t *output, const uint8_t *input); | ||
| static void inverseShiftRowsAndSubBytes(uint8_t *output, const uint8_t *input); | ||
| static void mixColumn(uint8_t *output, uint8_t *input); | ||
| static void inverseMixColumn(uint8_t *output, const uint8_t *input); | ||
| static void keyScheduleCore(uint8_t *output, const uint8_t *input, uint8_t iteration); | ||
| static void applySbox(uint8_t *output, const uint8_t *input); | ||
| /** @endcond */ | ||
|
|
||
| friend class AESTiny128; | ||
| friend class AESTiny256; | ||
| friend class AESSmall128; | ||
| friend class AESSmall256; | ||
| }; | ||
|
|
||
| class AES128 : public AESCommon | ||
| { | ||
| public: | ||
| AES128(); | ||
| virtual ~AES128(); | ||
|
|
||
| size_t keySize() const; | ||
|
|
||
| bool setKey(const uint8_t *key, size_t len); | ||
|
|
||
| private: | ||
| uint8_t sched[176]; | ||
| }; | ||
|
|
||
| class AES192 : public AESCommon | ||
| { | ||
| public: | ||
| AES192(); | ||
| virtual ~AES192(); | ||
|
|
||
| size_t keySize() const; | ||
|
|
||
| bool setKey(const uint8_t *key, size_t len); | ||
|
|
||
| private: | ||
| uint8_t sched[208]; | ||
| }; | ||
|
|
||
| class AES256 : public AESCommon | ||
| { | ||
| public: | ||
| AES256(); | ||
| virtual ~AES256(); | ||
|
|
||
| size_t keySize() const; | ||
|
|
||
| bool setKey(const uint8_t *key, size_t len); | ||
|
|
||
| private: | ||
| uint8_t sched[240]; | ||
| }; | ||
|
|
||
| class AESTiny256 : public BlockCipher | ||
| { | ||
| public: | ||
| AESTiny256(); | ||
| virtual ~AESTiny256(); | ||
|
|
||
| size_t blockSize() const; | ||
| size_t keySize() const; | ||
|
|
||
| bool setKey(const uint8_t *key, size_t len); | ||
|
|
||
| void encryptBlock(uint8_t *output, const uint8_t *input); | ||
| void decryptBlock(uint8_t *output, const uint8_t *input); | ||
|
|
||
| void clear(); | ||
|
|
||
| private: | ||
| uint8_t schedule[32]; | ||
| }; | ||
|
|
||
| class AESSmall256 : public AESTiny256 | ||
| { | ||
| public: | ||
| AESSmall256(); | ||
| virtual ~AESSmall256(); | ||
|
|
||
| bool setKey(const uint8_t *key, size_t len); | ||
|
|
||
| void decryptBlock(uint8_t *output, const uint8_t *input); | ||
|
|
||
| void clear(); | ||
|
|
||
| private: | ||
| uint8_t reverse[32]; | ||
| }; | ||
|
|
||
| class AESTiny128 : public BlockCipher | ||
| { | ||
| public: | ||
| AESTiny128(); | ||
| virtual ~AESTiny128(); | ||
|
|
||
| size_t blockSize() const; | ||
| size_t keySize() const; | ||
|
|
||
| bool setKey(const uint8_t *key, size_t len); | ||
|
|
||
| void encryptBlock(uint8_t *output, const uint8_t *input); | ||
| void decryptBlock(uint8_t *output, const uint8_t *input); | ||
|
|
||
| void clear(); | ||
|
|
||
| private: | ||
| uint8_t schedule[16]; | ||
| }; | ||
|
|
||
| class AESSmall128 : public AESTiny128 | ||
| { | ||
| public: | ||
| AESSmall128(); | ||
| virtual ~AESSmall128(); | ||
|
|
||
| bool setKey(const uint8_t *key, size_t len); | ||
|
|
||
| void decryptBlock(uint8_t *output, const uint8_t *input); | ||
|
|
||
| void clear(); | ||
|
|
||
| private: | ||
| uint8_t reverse[16]; | ||
| }; | ||
|
|
||
| #endif // CRYPTO_AES_DEFAULT | ||
|
|
||
| #if defined(CRYPTO_AES_ESP32) | ||
|
|
||
| /** @cond aes_esp_rename */ | ||
|
|
||
| // The esp32 SDK keeps moving where aes.h is located, so we have to | ||
| // declare the API functions ourselves and make the context opaque. | ||
| // | ||
| // About the only thing the various SDK versions agree on is that the | ||
| // first byte is the length of the key in bytes. | ||
| // | ||
| // Some versions of esp-idf have a 33 byte AES context, and others 34. | ||
| // Allocate up to 40 to make space for future expansion. | ||
| #define CRYPTO_ESP32_CONTEXT_SIZE 40 | ||
|
|
||
| // Some of the esp-idf system headers define enumerations for AES128, | ||
| // AES192, and AES256 to identify the hardware-accelerated algorithms. | ||
| // These can cause conflicts with the names we use in our library. | ||
| // Define our class names to something else to work around esp-idf. | ||
| #undef AES128 | ||
| #undef AES192 | ||
| #undef AES256 | ||
| #define AES128 AES128_ESP | ||
| #define AES192 AES192_ESP | ||
| #define AES256 AES256_ESP | ||
|
|
||
| /** @endcond */ | ||
|
|
||
| class AESCommon : public BlockCipher | ||
| { | ||
| public: | ||
| virtual ~AESCommon(); | ||
|
|
||
| size_t blockSize() const; | ||
| size_t keySize() const; | ||
|
|
||
| bool setKey(const uint8_t *key, size_t len); | ||
|
|
||
| void encryptBlock(uint8_t *output, const uint8_t *input); | ||
| void decryptBlock(uint8_t *output, const uint8_t *input); | ||
|
|
||
| void clear(); | ||
|
|
||
| protected: | ||
| AESCommon(uint8_t keySize); | ||
|
|
||
| private: | ||
| uint8_t ctx[CRYPTO_ESP32_CONTEXT_SIZE]; | ||
| }; | ||
|
|
||
| class AES128 : public AESCommon | ||
| { | ||
| public: | ||
| AES128() : AESCommon(16) {} | ||
| virtual ~AES128(); | ||
| }; | ||
|
|
||
| class AES192 : public AESCommon | ||
| { | ||
| public: | ||
| AES192() : AESCommon(24) {} | ||
| virtual ~AES192(); | ||
| }; | ||
|
|
||
| class AES256 : public AESCommon | ||
| { | ||
| public: | ||
| AES256() : AESCommon(32) {} | ||
| virtual ~AES256(); | ||
| }; | ||
|
|
||
| // The ESP32 AES context is so small that it already qualifies as "tiny". | ||
| typedef AES128 AESTiny128; | ||
| typedef AES256 AESTiny256; | ||
| typedef AES128 AESSmall128; | ||
| typedef AES256 AESSmall256; | ||
|
|
||
| #endif // CRYPTO_AES_ESP32 | ||
|
|
||
| #endif |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Making these
staticsaves ~1.8KB of stack which fixes the overflow, but it also makesge_double_scalarmult_vartime()non-reentrant. Ifed25519_verify()is ever called concurrently from multiple FreeRTOS tasks, these shared buffers will be silently corrupted.Currently this looks safe —
Identity::verify()is only called from the packet receive path inMesh.cpp— but it's worth documenting so future maintainers don't get bitten.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you thnk about an Assert? just in case its not read... (guilty of it sometimes)
48019bf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, the assert makes the constraint very visible. Two minor notes:
assert()is typically compiled out in release builds (NDEBUG), so this is really a development-time guard. That's fine for this purpose — the comment does the heavy lifting for future readers.static volatile boolcheck + set isn't atomic, so there's a theoretical TOCTOU race. In practice this doesn't matter — ESP32 FreeRTOS tasks on a single core won't preempt mid-statement, and even on dual-core the window is vanishingly small. Just mentioning it for completeness.Overall this looks good to me.