-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
OcCryptoLib: Add Streebog support #377
base: master
Are you sure you want to change the base?
Conversation
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.
Commented about most obvious issues with the code. There also are several codestyle issues with the code. Check https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/.
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.
CI is broken again, please fix. Also added more messages inline.
@@ -8,6 +8,8 @@ | |||
#include <Library/OcCryptoLib.h> | |||
#include <BigNumLib.h> | |||
|
|||
#define HASHSIZE 64 |
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.
This must come from OcCryptoLib.h, not be defined here.
#elif defined (__BYTE_ORDER__) && __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ | ||
buf.QWORD[0] = BSWAP64 (CTX->bufsize << 3); | ||
#else | ||
#if (defined (__BYTE_ORDER__) && __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__) || defined(MDE_CPU_IA32) || defined(MDE_CPU_X64) |
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.
Please introduce separate macros STREEBOG_LITTLE_ENDIAN and STREEBOG_BIG_ENDIAN in Streebog.h to reduce the length.
STREEBOG_CONTEXT *Context | ||
) | ||
{ | ||
for (INT32 i = 0; i < 64; ++i) { |
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.
EDK-II convention follows the rule where variables are declared at the beginning of functions; the same idea follows throughout the code.
Also, for indices, UINTN is used.
@@ -394,6 +409,56 @@ Sha384 ( | |||
UINTN Len | |||
); | |||
|
|||
VOID | |||
Streebog256Init ( | |||
STREEBOG_CONTEXT *Context |
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.
IN
and OUT
specifiers are missing, for all.
CONST UINT32 digest_size | ||
) | ||
{ | ||
UINT32 i; |
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.
Should be UINTN.
GOST34112012Cleanup (Context); | ||
Context->digest_size = digest_size; | ||
|
||
for (i = 0; i < 8; i++) { |
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.
Even though clear, is it better if a macro is assigned to the magic number8?
@lakreite Hello! Is this PR still alive? If not, I can start working on this. Thanks! UPDATE - Assuming the original author will not work on this; I will start it. |
is this pull request ever going to get merged? |
Well, currently the review comments are not addressed yet. If you are willing to pick it up and complete, you are gladly welcome ^_^ |
i could try but i would have to learn C |
Can I land a hand here? |
Well, we do not mind, please start with fixing up the CI. |
okie doke |
Streebog support added into OcCryptoLib.