Skip to content

Conversation

@chief-micco
Copy link
Contributor

The future of the Asherah libaries includes an initiative to move specific implementations of the interfaces in AppEncryption into separate "PlugIn" libraries. This will allow us to provide more flexibility and better separations of various Asherah components.

For example:
Aws PlugIns will contain implementations that are using AWS SDK, While implementations using ADO may have their own PlugIns library.
We could have future plugin libraries that handle optimized implementations for specific framework versions (in case supporting netstandard2.0 and newer dotnet versions requires this).

This change introduces the following new items:

  1. Library: GoDaddy.Asherah.AppEncryption.PlugIns.Aws
  2. Class: GoDaddy.Asherah.AppEncryption.Plugins.Aws.Kms.KeyManagementService (a newer implementation of the IKeyManagementService interface)
  3. Obsolete tag added to the AwsKeyManagementServiceImpl class. It will be removed in the future.
  4. Tests for the new code and to ensure backwards compatibility

The new implementation in the PlugIns library allows for better integration with standard dotnet startup patterns like using configuration and IServicesCollection.

To help us get this pull request reviewed and merged quickly, please be sure to include the following items:

  • Tests (if applicable)
  • Documentation (if applicable)
  • Changelog entry
  • A full explanation here in the PR description of the work done

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Tests
  • Other

Backward Compatibility

Is this change backward compatible with the most recently released version? Does it introduce changes which might change the user experience in any way? Does it alter the API in any way?

  • Yes (backward compatible)
  • No (breaking changes)

Issue Linking

…rary and obsolete the implementation that is in the main AppEncryption library
* main:
  Bump the minor-and-patch group across 1 directory with 19 updates
  Bump the minor-and-patch group across 1 directory with 24 updates
  Bump the minor-and-patch group across 1 directory with 10 updates
  Bump Microsoft.Extensions.Logging from 9.0.8 to 9.0.10
  Bump Microsoft.Extensions.Configuration from 9.0.8 to 9.0.10
  Bump the minor-and-patch group with 7 updates
  update go.work - bump go version to 1.24.0
  Bump the minor-and-patch group across 2 directories with 8 updates
  Bump the minor-and-patch group across 2 directories with 5 updates
...
};
"AsherahKmsOptions": {
"regionKeyArns": [
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to support something that uses your AWS Region more naturally or a primary region like before.

Copy link
Contributor

Choose a reason for hiding this comment

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

Added a helper method on the AsherahKmsOptions that allows startup code to optimize the the options based on a given preferred region (example: use the running region).

// pass keyManagementService to the SessionFactory builder
}
```

Copy link
Contributor

Choose a reason for hiding this comment

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

Section for migrating to plugins or a doc or something else?

Copy link
Contributor

Choose a reason for hiding this comment

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

Added a migration doc linked from the csharp readme

using var key = crypto.GenerateKey(keyCreationTime);

// Act
var result = await _keyManagementServiceEast.EncryptKeyAsync(key);
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this the same as the other one?

Copy link
Contributor

Choose a reason for hiding this comment

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

removed the duplicate test

public async Task DecryptKeyAsync_ShouldFail_BetweenRegions_WhenNoRegionsMatch()
{
// Arrange
using var crypto = new BouncyAes256GcmCrypto();
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 setup the options in each test

Copy link
Contributor

Choose a reason for hiding this comment

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

Created a testsetup builder, and moved setup code into each test.

var result = await _keyManagementServiceSomeBroken.EncryptKeyAsync(key);

// Assert
ValidateEncryptedKey(result);
Copy link
Contributor

Choose a reason for hiding this comment

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

We should also see the error logged here for the failed ERROR kms and check what region was encrypted.

* main:
  Release new version of Secure Memory
  Updated to use the proper bouncycastle. Support for modern dotnet was added years ago, and this is just catching us up to that
* main:
  egg
  Update securememory reference and set version for appencryption release. Also updated the sample application references that will be valid after release happens.
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