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

fix(js)!: add argon2i(mod|int) support #127

Conversation

berendsliedrecht
Copy link
Contributor

@berendsliedrecht berendsliedrecht commented Apr 3, 2023

Signed-off-by: blu3beri blu3beri@proton.me

  • StoreKeyMethod can now be properly created with argon2i

blu3beri and others added 2 commits April 3, 2023 14:40
@berendsliedrecht berendsliedrecht changed the title fix(js)add argon2i(mod|int) support fix(js)!: add argon2i(mod|int) support Apr 3, 2023
TimoGlastra
TimoGlastra previously approved these changes Apr 3, 2023
private argon2Level?: Argon2Level

public constructor(method: KdfMethod, argon2Level?: Argon2Level) {
if (method == KdfMethod.Kdf && !argon2Level) {
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
if (method == KdfMethod.Kdf && !argon2Level) {
if (method === KdfMethod.Kdf && !argon2Level) {

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add a check if kdf level is Raw and argon2level is defined? Also should we just add more enum values instead? What's the benefit of having kdf and then mod/int instaed of having kdf_mod and kdf_int?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mainly did it to be inline with how it is done in askar itself and this allows, although not quite good, more kdfs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will change it to Argon2IMod and Argon2IInt as growing with more kdfs would also not be a breaking change then.

blu3beri added 2 commits April 3, 2023 17:09
Signed-off-by: blu3beri <blu3beri@proton.me>
Signed-off-by: blu3beri <blu3beri@proton.me>
Comment on lines +1 to +18
export enum KdfMethod {
Raw = 'raw',
None = 'none',
Argon2IMod = 'kdf:argon2i:mod',
Argon2IInt = 'kdf:argon2i:int',
}

export class StoreKeyMethod {
private method: KdfMethod

public constructor(method: KdfMethod) {
this.method = method
}

public toUri() {
return this.method.toString()
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this for future proofing? We could just use the enum value directly right?!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is for future proofing. Argon could have a salt or other kdf methods might need some custom logic.

@berendsliedrecht berendsliedrecht merged commit 9f088c8 into openwallet-foundation:main Apr 3, 2023
@berendsliedrecht berendsliedrecht deleted the fix/argon2i-support branch April 3, 2023 16:53
jamshale pushed a commit to jamshale/askar that referenced this pull request Aug 18, 2024
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