-
Notifications
You must be signed in to change notification settings - Fork 42
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
Use SHA256 bytes without translating to string #60
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.
Thanks for the work, there are minor adjustments in code to do.
I am currently testing it and will give you a feedback later.
Breaking changes are involved so what remains to do ( I can do it on another PR):
- bump to v3.0 with a breaking change warning in changelog
- move com.gradle.plugin-publish to 1.0.0
- move kotest to 5.3.1
@@ -26,20 +26,19 @@ object Utils { | |||
* Encode string to sha256 |
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.
Can you please complete the comment and explain a bit more ?
@@ -8,6 +8,8 @@ import java.io.File | |||
*/ | |||
class UtilsTest : WordSpec({ | |||
|
|||
fun byteArrayOfInts(vararg ints: Int) = ByteArray(ints.size) { pos -> ints[pos].toByte() } |
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 add a comment. Can it be private ?
@@ -35,14 +37,14 @@ class UtilsTest : WordSpec({ | |||
Utils.encodeSecret( | |||
key, | |||
packageName | |||
) shouldBe "{ 0x5b, 0x6, 0x18, 0x31, 0xb, 0x72, 0x57, 0x5, 0x5d, 0x57, 0x3 }" | |||
) shouldBe "{ 0x67, 0xcb, 0xae, 0xcb, 0x4c, 0xbb, 0x42, 0xad, 0x59, 0x19, 0xe2 }" //"{ 0x5b, 0x6, 0x18, 0x31, 0xb, 0x72, 0x57, 0x5, 0x5d, 0x57, 0x3 }" |
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.
Why keep this as a comment ?
@@ -25,7 +27,7 @@ class UtilsTest : WordSpec({ | |||
"Using sha256()" should { | |||
"encode String in sha256" { | |||
val key = "youCanNotFindMySecret!" | |||
Utils.sha256(key) shouldBe "7bdc2b5992ef7b4cce0e06295f564f4fad0c96e5f82a0bcf9cd8323d3a3bcfbd" | |||
Utils.sha256(key) shouldBe byteArrayOfInts( 0x7b, 0xdc, 0x2b, 0x59, 0x92, 0xef, 0x7b, 0x4c, 0xce, 0x0e, 0x06, 0x29, 0x5f, 0x56, 0x4f, 0x4f, 0xad, 0x0c, 0x96, 0xe5, 0xf8, 0x2a, 0x0b, 0xcf, 0x9c, 0xd8, 0x32, 0x3d, 0x3a, 0x3b, 0xcf, 0xbd) /*"7bdc2b5992ef7b4cce0e06295f564f4fad0c96e5f82a0bcf9cd8323d3a3bcfbd"*/ |
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.
Comment on a new line please with a explanation
Current implementation translate the bytes into string , which limits the actual bytes used for XOR'ing and misses the high entropy characteristics of the SHA256 function (as the string will only be constructed of 1-0, a-f).
This PR removes the translation to string in both creating the Kotlin code generating the obfuscated key and the C++ code that decode it