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

Jump to bottom

Argon2id KDF #2309

Merged
merged 20 commits into from Jan 25, 2023
Merged

Argon2id KDF #2309

merged 20 commits into from Jan 25, 2023

Conversation

kspearrin
Copy link
Member

@kspearrin kspearrin commented Jan 19, 2023 •
edited

Type of change

  • Bug fix
  • New feature development
  • Tech debt (refactoring, code cleanup, dependency upgrades, etc)
  • Build/deploy pipeline (DevOps)
  • Other

Objective

Add mobile app support for Argon2id as KDF.

Code changes

  • gitignore: exclude argon2 android libs from being ignored
  • lib/ios: argon2 precompiled c libs for ios
  • Android/lib: argon2 precompiled c libs for android
  • Android.csproj: Add references to argon2 c libs
  • Android/Services/CryptoPrimitiveService: Add argon2 p-invoke to c libs. Expose Argon2 public function.
  • Core/Enums/KdfType: Add type for Argon2
  • Core/Abstractions/ICryptoFunctionService: Add Argon2 interface
  • Core/Abstractions/ICryptoPrimitiveService: Add Argon2 interface
  • Core/Services/CryptoService: Add Argon2 implementation
  • Core/Services/PclCryptoFunctionService: Add Argon2 implementation
  • iOS.Core/CryptoPrimitiveService: Add argon2 p-invoke to c libs. Expose Argon2 public function.
  • iOS.*.csproj: Add references to argon2 c libs

Before you submit

  • Please check for formatting errors (dotnet format --verify-no-changes) (required)
  • Please add unit tests where it makes sense to do so (encouraged but not required)
  • If this change requires a documentation update - notify the documentation team
  • If this change has particular deployment requirements - notify the DevOps team

kspearrin and others added 13 commits January 19, 2023 16:34
@kspearrin
add gcc_flags for libargon2.a
0485c5e
@kspearrin
fix up ios proj
bb1eb1f
@kspearrin
remove unused tag
f917b3d
@kspearrin
add gcc_flags to ios projects
df12e7d
@kspearrin
ios libargon2 binary
7da7238
@kspearrin
fix paths in ios project
c3daa89
@kspearrin
update pathing on other projs
53ebad9
@kspearrin
Argon2id primitive
8b5f9d8
@kspearrin
fix typing issues
c4a7aab
@kspearrin
comment
270bf11
@kspearrin
remove ds store
e0401f4
@quexten
[PS-2249] Implement Argon2 (#2293)
9613019 
* Implement Argon2

* Fix incorrect argon2 type on iOS

* Switch argon2 implementation to native bindings

* Change argon2 to save iterations instead of memory as 'kdfIterations'

* Remove mistakenly added import

* Remove unused library
@kspearrin
cleanup
392aa18
@kspearrin kspearrin changed the title Argon2id native bindings for iOS Argon2id KDF Jan 20, 2023
@kspearrin kspearrin requested a review from a team January 23, 2023 15:46
@kspearrin kspearrin marked this pull request as ready for review January 23, 2023 15:46
@quexten quexten mentioned this pull request Jan 23, 2023
Closed
5 tasks
fedemkr
fedemkr requested changes Jan 23, 2023
View reviewed changes
Copy link
Member

@fedemkr fedemkr left a comment

Choose a reason for hiding this comment

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

Overall I think it's great, just noticed some things

src/Android/Services/CryptoPrimitiveService.cs Outdated Show resolved Hide resolved
src/Android/Services/CryptoPrimitiveService.cs Outdated Show resolved Hide resolved
src/iOS/iOS.csproj Outdated Show resolved Hide resolved
@kspearrin kspearrin merged commit b8d53b0 into master Jan 25, 2023
@kspearrin kspearrin deleted the argon2ios branch January 25, 2023 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@quexten quexten quexten left review comments

@fedemkr fedemkr fedemkr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@kspearrin @quexten @fedemkr