Skip to content
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

Adapt NuCypher to Umbral==0.1.3a0 #620

Merged
merged 3 commits into from
Dec 17, 2018
Merged

Conversation

KPrasch
Copy link
Member

@KPrasch KPrasch commented Dec 17, 2018

Also removes a bunch of pinned Pipfile deps. Be sure to update to the latest pipenv!

@KPrasch KPrasch self-assigned this Dec 17, 2018
@KPrasch KPrasch added the Dependencies 😈 Dependency compatibility, updates, and relocks label Dec 17, 2018
@KPrasch KPrasch changed the title [WIP] Adapt NuCypher to Umbral==0.1.3a0 Adapt NuCypher to Umbral==0.1.3a0 Dec 17, 2018
Copy link
Contributor

@michwill michwill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the tests pass, this looks good otherwise... Didn't expect it to be that simple

Copy link
Contributor

@tuxxy tuxxy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐧

tuxxy
tuxxy previously requested changes Dec 17, 2018
@@ -62,7 +64,7 @@
# Keyring
__WRAPPING_KEY_LENGTH = 32
__WRAPPING_KEY_INFO = b'NuCypher-KeyWrap'
__HKDF_HASH_ALGORITHM = hashes.BLAKE2b
__HKDF_HASH_ALGORITHM = SHA256
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't actually need to be updated to SHA256 for compatibility.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
__HKDF_HASH_ALGORITHM = SHA256
__HKDF_HASH_ALGORITHM = hashes.BLAKE2b

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✔️

__HKDF_HASH_ALGORITHM = SHA256
__HKDF_HASH_LENGTH = 64
__HKDF_HASH_ALGORITHM = BLAKE2B
__HKDF_HASH_LENGTH = BLAKE2B_DIGEST_LENGTH
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unused, and also it doesn't have much sense, since there's no such thing as the hash length in HKDF. This was in fact specific to BLAKE2b only. I think we can safely remove this line.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✔️

@@ -43,9 +43,11 @@

from nucypher.config.constants import DEFAULT_CONFIG_ROOT
from nucypher.crypto.api import generate_self_signed_certificate
from nucypher.crypto.constants import SHA256, BLAKE2B, BLAKE2B_DIGEST_LENGTH
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SHA256 is not used anymore.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✔️

@KPrasch KPrasch dismissed tuxxy’s stale review December 17, 2018 23:25

Made requested changes

@KPrasch KPrasch merged commit 9a5c5d3 into nucypher:master Dec 17, 2018
@KPrasch KPrasch deleted the adapt-signatures branch July 21, 2020 02:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Dependencies 😈 Dependency compatibility, updates, and relocks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants