Skip to content

Custom auth fixes, tests, and CI framework #422

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

Merged
merged 16 commits into from
Mar 22, 2023
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions .builder/actions/crt-ci-test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import Builder
import re

class CiTest(Builder.Action):

def _write_environment_script_secret_to_env(self, env, secret_name):
mqtt5_ci_environment_script = env.shell.get_secret(secret_name)
env_line = re.compile('^export\s+(\w+)=(.+)')

lines = mqtt5_ci_environment_script.splitlines()
for line in lines:
env_pair_match = env_line.match(line)
if env_pair_match.group(1) and env_pair_match.group(2):
env.shell.setenv(env_pair_match.group(1), env_pair_match.group(2), quiet=True)


def run(self, env):

actions = []

try:
self._write_environment_script_secret_to_env(env, "ci/sdk-unit-testing")

env.shell.exec(["python3", "-m", "unittest", "discover", "--verbose"], check=True)
except:
print(f'Failure while running tests')
actions.append("exit 1")

return Builder.Script(actions, name='ci-test')
96 changes: 33 additions & 63 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ env:
CI_UTILS_FOLDER: "./aws-iot-device-sdk-python-v2/utils"
CI_SAMPLES_CFG_FOLDER: "./aws-iot-device-sdk-python-v2/.github/workflows"
CI_SAMPLES_FOLDER: "./aws-iot-device-sdk-python-v2/samples"
CI_IOT_CONTAINERS_ROLE: ${{ secrets.AWS_CI_IOT_CONTAINERS }}
CI_PUBSUB_ROLE: ${{ secrets.AWS_CI_PUBSUB_ROLE }}
CI_COGNITO_ROLE: ${{ secrets.AWS_CI_COGNITO_ROLE }}
CI_X509_ROLE: ${{ secrets.AWS_CI_X509_ROLE }}
Expand All @@ -31,77 +30,27 @@ env:
CI_FLEET_PROVISIONING_ROLE: ${{ secrets.AWS_CI_FLEET_PROVISIONING_ROLE }}
CI_DEVICE_ADVISOR: ${{ secrets.AWS_CI_DEVICE_ADVISOR_ROLE }}
CI_MQTT5_ROLE: ${{ secrets.AWS_CI_MQTT5_ROLE }}
CI_BUILD_AND_TEST_ROLE: arn:aws:iam::180635532705:role/V2_SDK_Unit_Testing
Copy link
Contributor

@TwistedTwigleg TwistedTwigleg Mar 22, 2023

Choose a reason for hiding this comment

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

Minor: I'd prefer if this was hidden in a GitHub secret. Technically there is no reason to put it in a secret, as it cannot be used without authenticating the IAM role on a per-repo basis, but I feel some additional security through obscurity couldn't hurt. It also makes it easier to change the IAM role in the future without needing to make a commit/PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well I have the opposite conviction. I'm convinced we need to remove all Github secrets for maintainability reasons (do you know what IAM policy is bound to the credentials embedded in some of the repos? I don't and I have no way of finding out).

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, fair point. That is something I had not considered and I can see how having it hidden away in secrets would make it impossible to know exactly what credentials are bound to what IAM role. Over time, I could see this being an issue, especially as it is forgotten why/when/who setup what.

Hiding it in secrets provides only a tiny bit of extra security, so, there's that too. It does expose the AWS account ID, but I don't think there is any harm in that? I cannot think of any right off at least. Since the IAM roles cannot be used outside our repositories, I do not think there is any worries there either.


In that case though, should we change this for all the IAM role secrets in all 4 of the V2 IoT SDK repositories? I think we should be consistent with how we do it, and so if we want to not use GitHub secrets for IAM roles, then we should apply this change across the board in our repositories.

Obviously this is outside the scope of this particular PR, but if we want to take this approach, then I can make PRs to do this as I know what IAM roles go to what secrets.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we should aim for no secrets and yes that requires quite a bit more work. I also think we should move tests away from requiring the test fixture to query secrets and instead make everything environmental injection via script from builder actions. That gives us a pattern that is the same across all repos, regardless of language, and the amount of resources and set up is significantly lessened.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay. As a first step, I'll work on removing the IAM role secrets first 👍


jobs:
all-python-versions:
runs-on: ubuntu-latest
permissions:
id-token: write # This is required for requesting the JWT
steps:
- name: Build ${{ env.PACKAGE_NAME }}
env:
AWS_ACCESS_KEY_ID: ${{ secrets.AWS_DA_ROLE_KEY }}
AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_DA_ROLE_PRIVATE_KEY }}
# There's hackery in builder.json so that when we run on manylinux
# we build and test using every version of python that we support.
run: |
aws s3 cp s3://aws-crt-test-stuff/ci/${{ env.BUILDER_VERSION }}/linux-container-ci.sh ./linux-container-ci.sh && chmod a+x ./linux-container-ci.sh
./linux-container-ci.sh ${{ env.BUILDER_VERSION }} aws-crt-manylinux2014-x64 build -p ${{ env.PACKAGE_NAME }}


al2:
runs-on: ubuntu-latest
windows:
runs-on: windows-latest
permissions:
id-token: write # This is required for requesting the JWT
steps:
- name: configure AWS credentials (containers)
uses: aws-actions/configure-aws-credentials@v1
with:
role-to-assume: ${{ env.CI_IOT_CONTAINERS_ROLE }}
aws-region: ${{ env.AWS_DEFAULT_REGION }}
# We can't use the `uses: docker://image` version yet, GitHub lacks authentication for actions -> packages
- name: Build ${{ env.PACKAGE_NAME }}
- name: Install boto3
run: |
aws s3 cp s3://aws-crt-test-stuff/ci/${{ env.BUILDER_VERSION }}/linux-container-ci.sh ./linux-container-ci.sh && chmod a+x ./linux-container-ci.sh
./linux-container-ci.sh ${{ env.BUILDER_VERSION }} aws-crt-al2-x64 build -p ${{ env.PACKAGE_NAME }}


raspberry:
runs-on: ubuntu-latest # latest
permissions:
id-token: write # This is required for requesting the JWT
strategy:
fail-fast: false
matrix:
image:
- raspbian-bullseye
steps:
python -m pip install boto3
- name: configure AWS credentials (containers)
uses: aws-actions/configure-aws-credentials@v1
uses: aws-actions/configure-aws-credentials@v2
with:
role-to-assume: ${{ env.CI_IOT_CONTAINERS_ROLE }}
role-to-assume: ${{ env.CI_BUILD_AND_TEST_ROLE }}
aws-region: ${{ env.AWS_DEFAULT_REGION }}
# set arm arch
- name: Install qemu/docker
run: docker run --rm --privileged multiarch/qemu-user-static --reset -p yes
- name: Build ${{ env.PACKAGE_NAME }}
run: |
aws s3 cp s3://aws-crt-test-stuff/ci/${{ env.BUILDER_VERSION }}/linux-container-ci.sh ./linux-container-ci.sh && chmod a+x ./linux-container-ci.sh
./linux-container-ci.sh ${{ env.BUILDER_VERSION }} aws-crt-${{ matrix.image }} build -p ${{ env.PACKAGE_NAME }}


windows:
runs-on: windows-latest
permissions:
id-token: write # This is required for requesting the JWT
steps:
- name: Build ${{ env.PACKAGE_NAME }}
run: |
python -c "from urllib.request import urlretrieve; urlretrieve('${{ env.BUILDER_HOST }}/${{ env.BUILDER_SOURCE }}/${{ env.BUILDER_VERSION }}/builder.pyz?run=${{ env.RUN }}', 'builder.pyz')"
python builder.pyz build -p ${{ env.PACKAGE_NAME }}
- name: Running samples in CI setup
run: |
python -m pip install boto3
- name: configure AWS credentials (PubSub)
uses: aws-actions/configure-aws-credentials@v1
with:
Expand Down Expand Up @@ -136,6 +85,14 @@ jobs:
permissions:
id-token: write # This is required for requesting the JWT
steps:
- name: Install boto3
run: |
python3 -m pip install boto3
- name: configure AWS credentials (containers)
uses: aws-actions/configure-aws-credentials@v2
with:
role-to-assume: ${{ env.CI_BUILD_AND_TEST_ROLE }}
aws-region: ${{ env.AWS_DEFAULT_REGION }}
- name: Build ${{ env.PACKAGE_NAME }}
run: |
python3 -c "from urllib.request import urlretrieve; urlretrieve('${{ env.BUILDER_HOST }}/${{ env.BUILDER_SOURCE }}/${{ env.BUILDER_VERSION }}/builder.pyz?run=${{ env.RUN }}', 'builder')"
Expand Down Expand Up @@ -175,6 +132,14 @@ jobs:
permissions:
id-token: write # This is required for requesting the JWT
steps:
- name: Running samples in CI setup
run: |
python -m pip install boto3
- name: configure AWS credentials (containers)
uses: aws-actions/configure-aws-credentials@v2
with:
role-to-assume: ${{ env.CI_BUILD_AND_TEST_ROLE }}
aws-region: ${{ env.AWS_DEFAULT_REGION }}
- name: Build ${{ env.PACKAGE_NAME }}
run: |
python3 -c "from urllib.request import urlretrieve; urlretrieve('${{ env.BUILDER_HOST }}/${{ env.BUILDER_SOURCE }}/${{ env.BUILDER_VERSION }}/builder.pyz?run=${{ env.RUN }}', 'builder')"
Expand Down Expand Up @@ -215,17 +180,22 @@ jobs:
permissions:
id-token: write # This is required for requesting the JWT
steps:
- name: Build ${{ env.PACKAGE_NAME }}
run: |
python3 -c "from urllib.request import urlretrieve; urlretrieve('${{ env.BUILDER_HOST }}/${{ env.BUILDER_SOURCE }}/${{ env.BUILDER_VERSION }}/builder.pyz?run=${{ env.RUN }}', 'builder')"
chmod a+x builder
./builder build -p ${{ env.PACKAGE_NAME }}
- name: Running samples in CI setup
run: |
python3 -m pip install boto3
sudo apt-get update -y
sudo apt-get install softhsm -y
softhsm2-util --version
- name: configure AWS credentials (containers)
uses: aws-actions/configure-aws-credentials@v2
with:
role-to-assume: ${{ env.CI_BUILD_AND_TEST_ROLE }}
aws-region: ${{ env.AWS_DEFAULT_REGION }}
- name: Build ${{ env.PACKAGE_NAME }}
run: |
python3 -c "from urllib.request import urlretrieve; urlretrieve('${{ env.BUILDER_HOST }}/${{ env.BUILDER_SOURCE }}/${{ env.BUILDER_VERSION }}/builder.pyz?run=${{ env.RUN }}', 'builder')"
chmod a+x builder
./builder build -p ${{ env.PACKAGE_NAME }}
- name: configure AWS credentials (Connect and PubSub)
uses: aws-actions/configure-aws-credentials@v1
with:
Expand Down
72 changes: 55 additions & 17 deletions awsiot/mqtt5_client_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -569,6 +569,8 @@ def direct_with_custom_authorizer(
auth_authorizer_name=None,
auth_authorizer_signature=None,
auth_password=None,
auth_token_key_name=None,
auth_token_value=None,
**kwargs) -> awscrt.mqtt5.Client:
"""
This builder creates an :class:`awscrt.mqtt5.Client`, configured for an MQTT5 Client using a custom
Expand All @@ -581,17 +583,30 @@ def direct_with_custom_authorizer(
auth_username (`str`): The username to use with the custom authorizer.
If provided, the username given will be passed when connecting to the custom authorizer.
If not provided, it will check to see if a username has already been set (via username="example")
and will use that instead.
If no username has been set then no username will be sent with the MQTT connection.

auth_authorizer_name (`str`): The name of the custom authorizer.
If not provided, then "x-amz-customauthorizer-name" will not be added with the MQTT connection.

auth_authorizer_signature (`str`): The signature of the custom authorizer.
If not provided, then "x-amz-customauthorizer-name" will not be added with the MQTT connection.
and will use that instead. Custom authentication parameters will be appended as appropriate
to any supplied username value.

auth_password (`str`): The password to use with the custom authorizer.
If not provided, then no passord will be set.
If not provided, then no password will be sent in the initial CONNECT packet.

auth_authorizer_name (`str`): Name of the custom authorizer to use.
Required if the endpoint does not have a default custom authorizer associated with it. It is strongly
suggested to URL-encode this value; the SDK will not do so for you.

auth_authorizer_signature (`str`): The digital signature of the token value in the `auth_token_value`
parameter. The signature must be based on the private key associated with the custom authorizer. The
signature must be base64 encoded.
Required if the custom authorizer has signing enabled. It is strongly suggested to URL-encode this value;
the SDK will not do so for you.

auth_token_key_name (`str`): Key used to extract the custom authorizer token from MQTT username query-string
properties.
Required if the custom authorizer has signing enabled. It is strongly suggested to URL-encode
this value; the SDK will not do so for you.

auth_token_value (`str`): An opaque token value. This value must be signed by the private key associated with
the custom authorizer and the result passed in via the `auth_authorizer_signature` parameter.
Required if the custom authorizer has signing enabled.
"""

_check_required_kwargs(**kwargs)
Expand All @@ -606,10 +621,14 @@ def direct_with_custom_authorizer(
if auth_authorizer_name is not None:
username_string = _add_to_username_parameter(
username_string, auth_authorizer_name, "x-amz-customauthorizer-name=")

if auth_authorizer_signature is not None:
username_string = _add_to_username_parameter(
username_string, auth_authorizer_signature, "x-amz-customauthorizer-signature=")

if auth_token_key_name is not None and auth_token_value is not None:
username_string = _add_to_username_parameter(username_string, auth_token_value, auth_token_key_name + "=")

kwargs["username"] = username_string
kwargs["password"] = auth_password

Expand All @@ -628,6 +647,8 @@ def websockets_with_custom_authorizer(
auth_authorizer_signature=None,
auth_password=None,
websocket_proxy_options=None,
auth_token_key_name=None,
auth_token_value=None,
**kwargs) -> awscrt.mqtt5.Client:
"""
This builder creates an :class:`awscrt.mqtt5.Client`, configured for an MQTT5 Client using a custom
Expand All @@ -640,17 +661,30 @@ def websockets_with_custom_authorizer(
auth_username (`str`): The username to use with the custom authorizer.
If provided, the username given will be passed when connecting to the custom authorizer.
If not provided, it will check to see if a username has already been set (via username="example")
and will use that instead.
If no username has been set then no username will be sent with the MQTT connection.
and will use that instead. Custom authentication parameters will be appended as appropriate
to any supplied username value.

auth_password (`str`): The password to use with the custom authorizer.
If not provided, then no password will be sent in the initial CONNECT packet.

auth_authorizer_name (`str`): The name of the custom authorizer.
If not provided, then "x-amz-customauthorizer-name" will not be added with the MQTT connection.
auth_authorizer_name (`str`): Name of the custom authorizer to use.
Required if the endpoint does not have a default custom authorizer associated with it. It is strongly
suggested to URL-encode this value; the SDK will not do so for you.

auth_authorizer_signature (`str`): The signature of the custom authorizer.
If not provided, then "x-amz-customauthorizer-name" will not be added with the MQTT connection.
auth_authorizer_signature (`str`): The digital signature of the token value in the `auth_token_value`
parameter. The signature must be based on the private key associated with the custom authorizer. The
signature must be base64 encoded.
Required if the custom authorizer has signing enabled. It is strongly suggested to URL-encode this value;
the SDK will not do so for you.

auth_password (`str`): The password to use with the custom authorizer.
If not provided, then no passord will be set.
auth_token_key_name (`str`): Key used to extract the custom authorizer token from MQTT username query-string
properties.
Required if the custom authorizer has signing enabled. It is strongly suggested to URL-encode
this value; the SDK will not do so for you.

auth_token_value (`str`): An opaque token value. This value must be signed by the private key associated with
the custom authorizer and the result passed in via the `auth_authorizer_signature` parameter.
Required if the custom authorizer has signing enabled.

websocket_proxy_options (awscrt.http.HttpProxyOptions): Deprecated,
for proxy settings use `http_proxy_options` (described in
Expand All @@ -669,10 +703,14 @@ def websockets_with_custom_authorizer(
if auth_authorizer_name is not None:
username_string = _add_to_username_parameter(
username_string, auth_authorizer_name, "x-amz-customauthorizer-name=")

if auth_authorizer_signature is not None:
username_string = _add_to_username_parameter(
username_string, auth_authorizer_signature, "x-amz-customauthorizer-signature=")

if auth_token_key_name is not None and auth_token_value is not None:
username_string = _add_to_username_parameter(username_string, auth_token_value, auth_token_key_name + "=")

kwargs["username"] = username_string
kwargs["password"] = auth_password

Expand Down
Loading