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.

Sign up for GitHub

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

Refactor remote / local crypto implementation #14129

Closed
maorleger opened this issue Mar 5, 2021 · 0 comments · Fixed by #14077
Closed

Refactor remote / local crypto implementation #14129

maorleger opened this issue Mar 5, 2021 · 0 comments · Fixed by #14077
Assignees
@maorleger
Labels
Client This issue points to a problem in the data-plane of the library. KeyVault
Milestone
[2021] April

Comments

@maorleger
Copy link
Member

To allow for separation of concerns and better extensibility replace the existing implementation with a single client that maintains a list of crypto providers. Each provider is responsible for reporting the methods / algorithms it support and for doing cryptography.

Criteria:

  • Keyvault interaction should happen through a remote provider
  • RSA algorithms that are supported locally should happen through an RSA provider
  • The CryptographyClient should allow for extensibility by registering more providers
  • Each provider is unaware of other providers and cannot call other providers
@maorleger maorleger added this to the [2021] April milestone Mar 5, 2021
@ghost ghost added the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Mar 5, 2021
@maorleger maorleger self-assigned this Mar 5, 2021
@maorleger maorleger added Client This issue points to a problem in the data-plane of the library. KeyVault labels Mar 5, 2021
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Mar 5, 2021
@maorleger maorleger mentioned this issue Mar 5, 2021
Merged
maorleger added a commit that referenced this issue Mar 5, 2021
@maorleger
[KeyVault] - Cryptography client refactoring (#14077)
426d0db 
## What

This PR does a pure refactor of local cryptography from the existing implementation
to provide some flexibility for upcoming AES algorithms and allow for a cleaner separation
of concerns between local / remote providers.

## Why

The existing implementation worked well but I hit a few bumps when trying to add support
for AES algorithms which require different parameters crypto providers to conform to the 
same API our top-level clients do and allows for ease of extensibility when AES algorithms 
are added in the future.

## High level changes

- Replaced the crypto algorithms with a crypto provider interface that supplies a common set of operations
- Top level crypto client maintains a list of "providers" which are responsible for the crypto operations. Each provider isn't aware of other providers and the crypto client is responsible for sending the request to the right one.
- Centralized validation to top-level client.

Resolves #14129
openapi-sdkautomation bot pushed a commit to AzureSDKAutomation/azure-sdk-for-js that referenced this issue May 10, 2021
@VMMicrosoft @rewongmicrosoft
@brandong-ms @akning-ms
CodeGen from PR 14339 in Azure/azure-rest-api-specs
0944ce1 
Release microsoft.sql 2021 02 01 preview (Azure#14339)

* Adds base for updating Microsoft.Sql from version preview/2020-11-01-preview to version 2021-02-01-preview

* Updates readme

* Updates API version in new specs and examples

* Fix Integer format: S360 swagger lint issues in 2021-02-01 (Azure#13855)

* Update integer format

* update typo

* Swagger Documentation for Outbound Firewall Rules (Azure#13820)

* Swagger Documentation for Outbound Firewall Rules

* Add the new file to v5

Co-authored-by: Vineet Mahadik <[email protected]>

* Swagger Documentation For Database Resource With Ledger (Azure#13916)

* adding database swagger files

* modifying example

* Fixing to be boolean

* adding database extensions and database operations

* adding other database examples

* removing database_legacy, adding usages

* removing usages

* removing databases_legacy from readme

* Swagger Documentation For Ledger Digest Upload (Azure#13871)

* adding ledger api

* Fixes from validation

* removing required endpoint parameter, adding ledgerName (current) to id

* removing 201 response from resource, renaming ledgerName

* adding location to 202 results

* fixing 202 response location

* renaming files

* updating files with new controller name + correct entity name

* fixing readme

* Dev brandong getrestorabledropped (Azure#14129)

* Update RestorableDroppedDatabases API to add BackupStorageAccountType property

* Update readme.md

* Re-add elasticPoolId as a deprecated property

* Update elasticPoolId description and remove trailing comma causing failures

* Remove the unsupported deprecated property

* Add 2021-02-01-preview minor changes (Azure#13942)

* add 2021-02-01-preview for test

* update with 2021-04-19 latest

* update readme.md

* update with latest master in DSMainDev

* Carry IsInfraEncryptionEnabled to Database.json (Azure#14322)

* carry latest minor changes.

* re-format readme.md

* remove 2020 11 01 RestorableDroppedManagedDatabases in V5

* Update readme file in 2021 02 01 dev branch to match the master branch (Azure#14336)

* Carry IsInfraEncryptionEnabled to Database.json (Azure#14322)

* carry latest minor changes.

* re-format readme.md

* remove 2020 11 01 RestorableDroppedManagedDatabases in V5

* update to match master branch readme.md

Co-authored-by: Vineet Mahadik <[email protected]>
Co-authored-by: Vineet Mahadik <[email protected]>
Co-authored-by: rewongmicrosoft <[email protected]>
Co-authored-by: brandong-ms <[email protected]>
Co-authored-by: Arthur Ning <[email protected]>
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@maorleger maorleger

Labels
Client This issue points to a problem in the data-plane of the library. KeyVault
Projects
None yet
Milestone
[2021] April
Development

Successfully merging a pull request may close this issue.

[KeyVault] - Cryptography client refactoring maorleger/azure-sdk-for-js
1 participant
@maorleger