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

feat: remove hashing domain and update key manager #4367

Conversation

hansieodendaal
Copy link
Contributor

@hansieodendaal hansieodendaal commented Aug 1, 2022

Description

  • Removed hashing domain in favour of recent changes in tari_crypto "v0.15.3".
  • Added a specialized marker trait for tight coupling of DomainSeparatedHasher to Digest; this is useful when a low level hashing function that accepts Digest as type needs to guarantee that domain separated hashing is implemented.
  • Added a MAC domain hasher that guarantees implementation of DomainSeparatedHasher and LengthExtensionAttackResistant.
  • Updated the key manager to use the MAC domain hasher and to implement the LengthExtensionAttackResistant trait.

Motivation and Context

See above

How Has This Been Tested?

Passed unit tests
Passed cucumber tests

@hansieodendaal hansieodendaal changed the title featRemove hashing domain feat: remove hashing domain Aug 1, 2022
@hansieodendaal hansieodendaal changed the title feat: remove hashing domain [wip] feat: remove hashing domain Aug 1, 2022
@hansieodendaal hansieodendaal force-pushed the ho_remove_hashing_domain branch from 84ceff5 to 6e4cfa9 Compare August 1, 2022 08:34
@hansieodendaal hansieodendaal changed the title [wip] feat: remove hashing domain [wip] feat: remove hashing domain and update key manager Aug 1, 2022
@hansieodendaal hansieodendaal force-pushed the ho_remove_hashing_domain branch 2 times, most recently from b0450f4 to b8a8f2d Compare August 1, 2022 09:44
@hansieodendaal hansieodendaal changed the title [wip] feat: remove hashing domain and update key manager feat: remove hashing domain and update key manager Aug 1, 2022
- Removed hashing domain in favour of recent changes in `tari_crypto "v0.15.3"`.
- Added a specialized marker trait for tight coupling of `DomainSeparatedHasher`
  to `Digest`; this is useful when a low level hashing function that accepts
  `Digest` as type needs to guarantee that domain separated hashing is implemented.
- Added a MAC domain hasher that guarantees implementation of
  `DomainSeparatedHasher` and `LengthExtensionAttackResistant`.
- Upadated the key manager to use the MAC domain hasher and to implement
  the `LengthExtensionAttackResistant` trait.
@hansieodendaal hansieodendaal force-pushed the ho_remove_hashing_domain branch from b8a8f2d to 6c680dd Compare August 2, 2022 12:07
DomainSeparatedHasher::<Blake256, KeyManagerHashDomain>::new("cipher_seed.chacha20_encoding")
}
hash_domain!(
KeyManagerMacGeneration,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
KeyManagerMacGeneration,
KeyManagerMacGenerationDomain,

1
);
hash_domain!(
KeyManagerArgon2Encoding,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
KeyManagerArgon2Encoding,
KeyManagerArgon2EncodingDomain,

1
);
hash_domain!(
KeyManagerChacha20Encoding,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
KeyManagerChacha20Encoding,
KeyManagerChacha20EncodingDomain,

@@ -495,7 +503,7 @@ async fn test_utxo_selection_no_chain_metadata() {

// test if a fee estimate would be possible with pending funds included
// at this point 52000 uT is still spendable, with pending change incoming of 1690 uT
// so instead of returning "not enough funds", return "funds pending"
// so instead of returning "not enough funds".to_string(), return "funds pending"
Copy link
Collaborator

Choose a reason for hiding this comment

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

weird, but ok

@aviator-app aviator-app bot added the mq-failed label Aug 2, 2022
@stringhandler stringhandler merged commit e805f1f into tari-project:development Aug 2, 2022
@hansieodendaal hansieodendaal deleted the ho_remove_hashing_domain branch August 2, 2022 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants