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: Secure atKeys with pass-phrase #703

Merged

Conversation

sitaram-kalluri
Copy link
Member

@sitaram-kalluri sitaram-kalluri commented Oct 30, 2024

- What I did
Support the password protected of atKeys file.

- How I did it

  1. Add pass phrase to the AtOnboardingPreference to capture the pass phrase and propagate it the at_auth and at_chops package to decrypt the atKeys.

  2. In auth_cli_args, add parser options to capture "pass phrase" and "hashing algo type"

  3. In at_onboarding_service_impl.dart, in "_generateAtKeysFile", check if the passPhrase is supplied. If supplied, encrypt the keys with the passphrase.

  4. Removed the at_cli_commons dependency from at_onboarding_cli to prevent from cyclic dependency.

- How to verify it

  1. Added a test to verify the encryption and decryption of atKeys using a passphrase with the argon2id algorithm

  2. A functional test to verify password protected of atKeys file

- Description for the changelog

  • feat: Support password protected of atKeys

packages/at_cli_commons/lib/src/cli_base.dart Outdated Show resolved Hide resolved
@sitaram-kalluri sitaram-kalluri requested a review from gkc November 4, 2024 02:09
packages/at_chops/lib/src/algorithm/algo_type.dart Outdated Show resolved Hide resolved
packages/at_chops/lib/src/at_keys_crypto.dart Outdated Show resolved Hide resolved
packages/at_chops/lib/src/at_keys_crypto.dart Outdated Show resolved Hide resolved
@sitaram-kalluri
Copy link
Member Author

Verified backward compatibility of at_auth and at_chops packages with at_client_sdk and looks fine.

Copy link
Contributor

@gkc gkc left a comment

Choose a reason for hiding this comment

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

LGTM - will leave final approval to Murali

…-protected-encryption-ofatkeys-files

# Conflicts:
#	packages/at_chops/lib/at_chops.dart
#	packages/at_chops/lib/src/algorithm/aes_encryption_algo.dart
#	packages/at_chops/lib/src/at_keys_crypto.dart
@sitaram-kalluri
Copy link
Member Author

The changes in

At this point, this PR contains changes in at_onboarding_cli. The changes are at_cli_commons are moved to a different branch.

@sitaram-kalluri sitaram-kalluri requested a review from gkc November 25, 2024 04:23
String? commitLogStoragePathToUse =
('${storageDir?.path}/commit').replaceAll('/', Platform.pathSeparator);
String downloadPathToUse = ('$homeDir!/.atsign/downloads/$atSign/$nameSpace')
.replaceAll('/', Platform.pathSeparator);

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you extract the code below into its own file as a function which will return an AtClient given some args (similar to what we have in the noports repo here)

Copy link
Member Author

Choose a reason for hiding this comment

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

This change in implemented in 5be8bf9

@@ -50,4 +53,62 @@ class HomeDirectoryUtil {
enrollmentId: enrollmentId),
'hive');
}

/// Generate a path like this:
Copy link
Contributor

Choose a reason for hiding this comment

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

I really don't like that we are duplicating this code, again. I think instead we should move these functions to at_utils from at_cli_commons. Rather than delay this PR further, please create another ticket to take care of this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Created the following ticket: #720

…t_client_cli.dart" and use it in auth_cli.dart
@sitaram-kalluri sitaram-kalluri requested a review from gkc November 25, 2024 08:56
stderr.writeln();
var msg = 'Failed to connect after $attempts attempts';
stderr.writeln(chalk.brightRed(msg));
throw SecondaryServerConnectivityException(msg);
Copy link
Member

Choose a reason for hiding this comment

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

This may not be connectivity error always. Auth can fail for other reasons.
Can you replace with UnAuthenticatedException

Copy link
Member Author

Choose a reason for hiding this comment

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

This is addressed in 0bdb5e5

'${Platform.environment['HOME']}/.atsign/keys/${atSign}_key.atKeys')
// Fetched cram key from the at_demos repo.
..cramSecret =
'15cdce8f92bcf7e742d5b75dc51ec06d798952f8bf7e8ff4c2b6448e5f7c2c12b570fe945f04011455fdc49cacdf9393d9c1ac4609ec71c1a0b0c213578e7ec7');
Copy link
Member

Choose a reason for hiding this comment

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

can we add dependency to at_demos and remove this hard coded cram key

Copy link
Member Author

Choose a reason for hiding this comment

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

This is addressed in 0bdb5e5

gkc
gkc previously approved these changes Nov 29, 2024
Copy link
Contributor

@gkc gkc left a comment

Choose a reason for hiding this comment

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

Please address Murali's comments

@sitaram-kalluri sitaram-kalluri merged commit b80419a into trunk Dec 2, 2024
12 checks passed
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.

Add Encryption Password Option to CLI Commands in at_onboarding_cli for Onboard/Enroll**
3 participants