Add AWS KMS support to SignerStore #452

lukpueh · 2024-02-06T15:51:53Z

[EDIT 02/14/24: remove poc/draft status]

Add AWS KMS support to SignerStore and test with localstack (tox -e local-aws-kms)

Change details

Whitelist required ambient settings in SignerStore (see tox.ini for required settings)
Add independent tox environment to init/cleanup localstack, configure ambient AWS KMS credentials, create a test key, and run the test.
Add test to "import" test public key from AWS KMS and configure private key URI - this would typically happen in a key management UI (e.g. RSTUF CLI) - and use SignerStore.get to load the signer.

Todo

This PR adds a single unit test method, which is ran against a 3rd party service. This doesn't fit into the current test architecture, where 3rd party services are consistently mocked in unit tests, and only included in more comprehensive functional tests.

Please advise how to best test this!

codecov · 2024-02-07T17:37:53Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (714a29d) to head (60e7514).
Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##              main      #452   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           15        15           
  Lines         1071      1085   +14     
=========================================
+ Hits          1071      1085   +14

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

lukpueh · 2024-02-14T13:23:48Z

#455 changed SignerStore to parse ambient signer settings from Dynaconf and re-export them into an isolated environment. Rebasing required the following changes (squashed into original commit):

Rename AWS_* ambient settings (environment variables) to RSTUF_AWS_* in tox.ini so that Dynaconf can pick them up.
Update SignerStore to re-export ambient settings as AWS_* into the isolated environment used by (AWSSigner)
(The PR now includes an unrelated preparatory commit simplify the settings translation: c0c9a4e)

Unfortunately, this also means that "0 worker code changes" is no longer fully accurate, but I think that doesn't matter. :)

lukpueh · 2024-02-14T13:27:34Z

Now that the important blockers are resolved, I think we can actually treat this as AWS KMS support PR. Let me update the PR title/description and mark this ready for review...

Preparatory work to support other ambient settings in the future. Note that this commit also changes the environment variable name expected in FileNameSigner.from_priv_key_uri, in order to allow re-using the dynaconf setting name. The change does not affect the user. Signed-off-by: Lukas Puehringer <[email protected]>

Add test setup and test to load a signer from AWS KMS (localstack) via SignerStore (pending in repository-service-tuf#451), with 0 worker code changes. Run as `tox -e local-aws-kms` **Change details** * Add independent tox environment to init/cleanup localstack, configure ambient AWS KMS credentials, create a test key, and run the test. * Add test to "import" test public key from AWS KMS and configure private key URI - this would typically happen in a key management UI (e.g. RSTUF CLI) - and use `SignerStore.get` to load the signer. Signed-off-by: Lukas Puehringer <[email protected]>

kairoaraujo

@lukpueh, it is great work.
I added a commit to simplify the AWS usage in general.

LGTM, adding @MVrachev to review as well.

This commit aligns the AWS S3 Storage services to the AWS KMS The user will not require to set two sets of different env variables. It makes easier to combine the two AWS services during the deployment. Example: ``` - RSTUF_STORAGE_BACKEND=AWSS3 - RSTUF_AWS_STORAGE_BUCKET=tuf-metadata - RSTUF_AWS_ACCESS_KEY_ID=keyid - RSTUF_AWS_SECRET_ACCESS_KEY=ACCESS_KEY - RSTUF_AWS_DEFAULT_REGION=us-east-1 - RSTUF_AWS_ENDPOINT_URL=http://localstack:4566 ``` This commit also include updates in the Docker_README Signed-off-by: Kairo Araujo <[email protected]>

MVrachev

I have a question and a nit.
Otherwise, the changes are elegant and small.
I like that.

docs/source/guide/Docker_README.md

MVrachev · 2024-03-07T14:14:48Z

repository_service_tuf_worker/signer.py

@@ -25,20 +25,20 @@ class FileNameSigner(CryptoSigner):
    Provide method to load **unencrypted** PKCS8/PEM private key from file.

    File path is constructed by joining base path in environment variable
-    ``RSTUF_ONLINE_KEY_DIR`` with file in ``priv_key_uri``.
+    ``ONLINE_KEY_DIR`` with file in ``priv_key_uri``.


You said this was done to allow re-using the Dynaconf setting name. Could you give an example where it's possible for ONLINE_KEY_DIR name to be reused?

I thought we want to try and keep all RSTUF related env variables with the RSTUF prefix.

PS: In your changes in docs/source/guide/Docker_README.md line 230 we describe with its full name RSTUF_ONLINE_KEY_DIR.

You said this was done to allow re-using the Dynaconf setting name. Could you give an example where it's possible for ONLINE_KEY_DIR name to be reused?

The ONLINE_KEY_DIR is not reused. Only the RSTUF_AWS* ones, which are AWS common env variables.

I thought we want to try and keep all RSTUF related env variables with the RSTUF prefix.

PS: In your changes in docs/source/guide/Docker_README.md line 230 we describe with its full name RSTUF_ONLINE_KEY_DIR.

Dynaconf uses RSTUF_, the prefix, to identify the environment variables it adds to the Dynaconf settings.
All are still RSTUF_, but only inside the function, the usage, the RSTUF_. Dynaconf automatically removes it.

Then why do we rename it here to ONLINE_KEY_DIR only when the full name under which you can find it is RSTUF_ONLINE_KEY_DIR?

I didn't get your question.

On the OS/Container we need to use the RSTUF :

root@3f79f6f8a407:/opt/repository-service-tuf-worker# env | grep RSTUF RSTUF_AWS_ACCESS_KEY_ID=test RSTUF_AWS_ENDPOINT_URL=http://localstack:4566 RSTUF_BROKER_SERVER=redis://redis/1 RSTUF_SQL_SERVER=postgresql://postgres:secret@db:5432 RSTUF_REDIS_SERVER=redis://redis RSTUF_AWS_SECRET_ACCESS_KEY=test RSTUF_STORAGE_BACKEND=AWSS3 RSTUF_AWS_STORAGE_BUCKET=tuf-metadata RSTUF_AWS_DEFAULT_REGION=us-east-1

But, in the Dynaconf it is used only to map these as settings used by Dynaconf

>>> repository._settings.environ environ({'RSTUF_AWS_ACCESS_KEY_ID': 'test', 'RSTUF_AWS_ENDPOINT_URL': 'http://localstack:4566', 'HOSTNAME': '3f79f6f8a407', 'PYTHON_VERSION': '3.10.12', 'RSTUF_BROKER_SERVER': 'redis://redis/1', 'RSTUF_SQL_SERVER': 'postgresql://postgres:secret@db:5432', 'PWD': '/opt/repository-service-tuf-worker', 'PYTHON_SETUPTOOLS_VERSION': '65.5.1', 'HOME': '/root', 'LANG': 'C.UTF-8', 'GPG_KEY': 'A035C8C19219BA821ECEA86B64E628F8D684696D', 'RSTUF_REDIS_SERVER': 'redis://redis', 'RSTUF_AWS_SECRET_ACCESS_KEY': 'test', 'TERM': 'xterm', 'SHLVL': '1', 'WORKER_ID': '3f79f6f8a407', 'RSTUF_STORAGE_BACKEND': 'AWSS3', 'PYTHON_PIP_VERSION': '23.0.1', 'PYTHON_GET_PIP_SHA256': '96461deced5c2a487ddc65207ec5a9cffeca0d34e7af7ea1afc470ff0d746207', 'PYTHON_GET_PIP_URL': 'https://github.com/pypa/get-pip/raw/0d8570dc44796f4369b652222cf176b3db6ac70e/public/get-pip.py', 'RSTUF_AWS_STORAGE_BUCKET': 'tuf-metadata', 'PATH': '/usr/local/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin', 'RSTUF_AWS_DEFAULT_REGION': 'us-east-1', 'RSTUF_WORKER_ID': '3f79f6f8a407', '_': '/usr/local/bin/python'})

But it is not the one you use to map in Dynaconf.

>>> repository._worker_settings.get('AWS_ENDPOINT_URL') 'http://localstack:4566' >>> repository._worker_settings.get('RSTUF_AWS_ENDPOINT_URL') >>>

We use RSTUF_* in docs for the user (Docker_README) but not in the code itself.

MVrachev

We had a discussion with Kairo about ONLINE_KEY_DIR env naming and I got what he meant with that renaming.

Approving.

Signed-off-by: Kairo Araujo <[email protected]>

lukpueh force-pushed the test-aws-signer branch from edcf2e0 to a1ba06c Compare February 7, 2024 17:33

lukpueh mentioned this pull request Feb 13, 2024

Add FileNameSigner and register in SignerStore #455

Merged

lukpueh force-pushed the test-aws-signer branch from a1ba06c to 07496d1 Compare February 14, 2024 13:12

lukpueh changed the title ~~POC: Show how SignerStore already supports AWS KMS~~ Add AWS KMS support to SignerStore Feb 14, 2024

lukpueh marked this pull request as ready for review February 14, 2024 13:40

kairoaraujo force-pushed the test-aws-signer branch from 07496d1 to 5fb1341 Compare March 2, 2024 07:33

lukpueh added 2 commits March 7, 2024 09:29

kairoaraujo force-pushed the test-aws-signer branch from 8e3ad77 to 203407d Compare March 7, 2024 08:29

kairoaraujo requested a review from MVrachev March 7, 2024 08:30

kairoaraujo previously approved these changes Mar 7, 2024

View reviewed changes

kairoaraujo dismissed their stale review via c3ff903 March 7, 2024 08:37

kairoaraujo force-pushed the test-aws-signer branch from 203407d to c3ff903 Compare March 7, 2024 08:37

MVrachev reviewed Mar 7, 2024

View reviewed changes

MVrachev previously approved these changes Mar 8, 2024

View reviewed changes

fix: docker-compose-aws.yml using new env for AWS

60e7514

Signed-off-by: Kairo Araujo <[email protected]>

kairoaraujo dismissed MVrachev’s stale review via 60e7514 March 10, 2024 08:45

kairoaraujo force-pushed the test-aws-signer branch from 9527c5c to 60e7514 Compare March 10, 2024 08:45

kairoaraujo self-requested a review March 10, 2024 08:45

kairoaraujo approved these changes Mar 10, 2024

View reviewed changes

kairoaraujo merged commit 71b74ec into repository-service-tuf:main Mar 10, 2024
11 checks passed

MVrachev mentioned this pull request Mar 11, 2024

Add support for AWSKMS #415

Closed

lukpueh mentioned this pull request May 3, 2024

Task: Implement Key Vault for AWS KMS #344

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add AWS KMS support to SignerStore #452

Add AWS KMS support to SignerStore #452

lukpueh commented Feb 6, 2024 •

edited

Loading

codecov bot commented Feb 7, 2024 •

edited

Loading

lukpueh commented Feb 14, 2024

lukpueh commented Feb 14, 2024

kairoaraujo left a comment

MVrachev left a comment •

edited

Loading

MVrachev Mar 7, 2024

kairoaraujo Mar 7, 2024 •

edited

Loading

MVrachev Mar 7, 2024 •

edited

Loading

kairoaraujo Mar 7, 2024 •

edited

Loading

MVrachev left a comment

Add AWS KMS support to SignerStore #452

Add AWS KMS support to SignerStore #452

Conversation

lukpueh commented Feb 6, 2024 • edited Loading

codecov bot commented Feb 7, 2024 • edited Loading

Codecov Report

lukpueh commented Feb 14, 2024

lukpueh commented Feb 14, 2024

kairoaraujo left a comment

Choose a reason for hiding this comment

MVrachev left a comment • edited Loading

Choose a reason for hiding this comment

MVrachev Mar 7, 2024

Choose a reason for hiding this comment

kairoaraujo Mar 7, 2024 • edited Loading

Choose a reason for hiding this comment

MVrachev Mar 7, 2024 • edited Loading

Choose a reason for hiding this comment

kairoaraujo Mar 7, 2024 • edited Loading

Choose a reason for hiding this comment

MVrachev left a comment

Choose a reason for hiding this comment

lukpueh commented Feb 6, 2024 •

edited

Loading

codecov bot commented Feb 7, 2024 •

edited

Loading

MVrachev left a comment •

edited

Loading

kairoaraujo Mar 7, 2024 •

edited

Loading

MVrachev Mar 7, 2024 •

edited

Loading

kairoaraujo Mar 7, 2024 •

edited

Loading