feat: Adds AWS KMS signing.

[ianhundere]

Description of the changes being introduced by the pull request:

This pull request adds AWS signer implementation. though currently the func that gets the scheme and keytype just provides a default signing algorithm for each respective key spec specifically for RSA keys.

I added logic to the sign func that parses the correct signing algorithm which is a bit messy, but I had trouble trying to pull the scheme in from the test elsewhere and didn't want to muddy the way things were done compared to the gcp/azure implementations.

I also redacted the necessary information in the test, but I also see that the azure test isn't currently being used in pipelines.

Please verify and check that the pull request fulfils the following requirements:

• The code follows the Code Style Guidelines
• Tests have been added for the bug fix or new feature
• Docs have been added for the bug fix or new feature

[lukpueh]

Very cool! Thanks for the contribution, @ianhundere! From a quick glance the code looks great. May I ask you to fix ci failures?

• Windows builds fail, because we seem to drop right below the minimum coverage there. As a quick fix, I suggest to just further lower the coverage constraint in tox.ini. We can fix it properly later with Fine-tune code coverage measurement for optional builds #208
• The purepy build fails, because cryptography is also an optional dependency and thus should go into the try block like boto
• Lint build fails, because code does not conform to the black style. You can test all linters locally with tox -e lint.

I added logic to the sign func that parses the correct signing algorithm which is a bit messy, but I had trouble trying to pull the scheme in from the test elsewhere and didn't want to muddy the way things were done compared to the gcp/azure implementatios.

Your code looks fine for a prototype. We can polish later, e.g. with #594 and #593.

I also redacted the necessary information in the test, but I also see that the azure test isn't currently being used in pipelines.

Did you test this somewhere? Would you mind providing some details about how to setup AWS KMS to use AWSSigner?

[ianhundere]

Appreciate the feedback, @lukpueh. Just pushed a commit that ensures the keytype and scheme passed in import_ are consistent with the actual signing algorithm, tests were passing / correct signing algo was being passed, but the scheme value wasn't always consistent with the algo passed to aws kms.

Did you test this somewhere? Would you mind providing some details about how to setup AWS KMS to use AWSSigner?

Yeah, I ensured that all the appropriate values were where they should be and that all tests passed. I also tested both rsa and ecdsa keys.

I'll implement your feedback over the weekend, cheers / thanks. 🍻

[ianhundere]

@lukpueh Okay, documentation updated and all tox -e lint errors should now be resolved.

Success: no issues found in 14 source files
  lint: OK (7.75=setup[0.06]+cmd[0.75,0.31,5.82,0.51,0.31] seconds)
  congratulations :) (7.85 seconds)

[ianhundere]

@lukpueh added one last change to cleanup _get_keytype_and_scheme by adding a new func, _get_aws_signing_algo, and updated some docstrings, too. should be all good now.

Success: no issues found in 14 source files
  lint: OK (7.78=setup[0.05]+cmd[0.71,0.30,5.79,0.49,0.46] seconds)
  congratulations :) (7.84 seconds)

[lukpueh]

Thanks again for the very valuable contribution! The AWS KMS interaction looks great. 💯

I did have a hard time, though, to follow your signing scheme detection code. It looks like you are putting a lot of effort into making the keytypes_and_schemes map bidirectional? ... while taking into account the signing algorithms that are supported by the KMS?

I have two comments about that:

1. The scheme in the public key object that you pass to the aws signer constructor should be the single source of truth.

   This means, you can ask the KMS for available signing algorithms when you import the public key, and just pick any one, e.g the first as you do now. But when you create or use the signer, you should not ask the KMS again, but instead use the signature algorithm appropriate for the public key signing scheme.

2. There are many ways to make keytypes_and_schemes bidirectional. Given the small size of the map, I think a reasonable and very readable option is to just duplicate it. E.g.:

   def _get_keytype_and_scheme(aws_signing_algorithm: str) -> Tuple[str, str]:
       keytypes_and_schemes = {
           "ECDSA_SHA_256": ("ecdsa", "ecdsa-sha2-nistp256"),
           ...
           }
       return keytypes_and_schemes[aws_signing_algorithm]
   
   def _get_aws_signing_algorithm(scheme: str) -> str:
       aws_signing_algorithms = {
           "ecdsa-sha2-nistp256": "ECDSA_SHA_256",
           ...
           }
       return aws_signing_algorithms[scheme]

Apologies, if I am missing something. Also, happy to discuss this in person.

[lukpueh]

Here's a couple of docs and style -related comments. Definitely no blockers, but if you have time, it would be nice to fix them.

[ianhundere]

I have two comments about that:

1. The scheme in the public key object that you pass to the aws signer constructor should be the single source of truth.
   This means, you can ask the KMS for available signing algorithms when you import the public key, and just pick any one, e.g the first as you do now. But when you create or use the signer, you should not ask the KMS again, but instead use the signature algorithm appropriate for the public key signing scheme.
2. There are many ways to make keytypes_and_schemes bidirectional. Given the small size of the map, I think a reasonable and very readable option is to just duplicate it. E.g.:

   def _get_keytype_and_scheme(aws_signing_algorithm: str) -> Tuple[str, str]:
       keytypes_and_schemes = {
           "ECDSA_SHA_256": ("ecdsa", "ecdsa-sha2-nistp256"),
           ...
           }
       return keytypes_and_schemes[aws_signing_algorithm]
   
   def _get_aws_signing_algorithm(scheme: str) -> str:
       aws_signing_algorithms = {
           "ecdsa-sha2-nistp256": "ECDSA_SHA_256",
           ...
           }
       return aws_signing_algorithms[scheme]

awesome, this is terrific feedback and was really helpful. i just implemented your suggestions above, but i'll go ahead and push the rest of your feedback in one commit, should have them up shortly.

[ianhundere]

@lukpueh okay, all suggestions implemented, much prettier to look at now. 😅

thanks again for the feedback. 🙂

edit: i had some failing tests, resolved them...now it should be good. 🚀

[lukpueh]

Thanks for the quick updates, this looks great!

I like that you just let the user decide, which scheme to use. We can make enhancements later, like cross-check with the schemes that are actually supported, or pick a reasonable default from the supported schemes, etc.

I left a few final comments. Otherwise, I think this is ready to merge, and we can take a stab at testing in a follow-up PR (#612).

[lukpueh]

Thanks!