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.

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

Add script to sign arbitrary files using the key stored in Secrets Manager #3

Merged
merged 3 commits into from
May 31, 2018

Conversation

rix0rrr
Copy link
Contributor

@rix0rrr rix0rrr commented May 30, 2018

This adds a script that loads a secret PGP key from secrets manager and signs arbitrary artifacts with it, creating detached (FILE.sig) signatures.

Not integrated with the buildspec.yaml yet. We should do that when we're builder superzips.

@rix0rrr rix0rrr changed the title Huijbers/signing Add signing May 30, 2018
@rix0rrr rix0rrr changed the title Add signing Add signing script May 30, 2018
@eladb
Copy link
Contributor

eladb commented May 30, 2018

Please add a PR description that describes the motivation and some context

@eladb
Copy link
Contributor

eladb commented May 30, 2018

You might want to change the destination of this PR so it won't be against master as long as you haven't committed your changes to master, because at the moment it seems like this has all the doc changes as well (you can change the destination branch from the UI)

sign.sh Outdated
passphrase=$(python -c "import json; print(json.load(file('$tmpdir/secret.txt'))['Passphrase'])")

echo "Importing key..." >&2
gpg --homedir $tmpdir --import <(python -c "import json; print(json.load(file('$tmpdir/secret.txt'))['PrivateKey'])")
Copy link
Contributor

Choose a reason for hiding this comment

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

is this something we need to install or add to awslabs/superchain?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both gpg and AWS CLI are already in there.

Copy link
Contributor

Choose a reason for hiding this comment

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

okay, cool. I don't have it on my Mac but I guess signing is only done on the build fleet anyway.

sign.sh Outdated


tmpdir=$(mktemp -d)
trap "shred $tmpdir/* && rm -rf $tmpdir" EXIT
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't shred on my machine

Copy link
Contributor Author

Choose a reason for hiding this comment

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

+have ?

Oh, macOS... :/

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes... Try to keep dep surface to a minimum when writing bash scripts. You don't want to know what's up on Windows...

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 yeah, but neither do you have gpg on your machine, apparently. This is designed to run inside the Docker, and the Docker has shred.

sign.sh Outdated
# Use secrets manager to obtain the key and passphrase into a JSON file
echo "Retrieving key..." >&2
aws --region us-east-1 secretsmanager get-secret-value --secret-id "$SECRET" --output text --query SecretString > $tmpdir/secret.txt
passphrase=$(python -c "import json; print(json.load(file('$tmpdir/secret.txt'))['Passphrase'])")
Copy link
Contributor

Choose a reason for hiding this comment

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

I rather use node for JSON-related tasks (it is JS afterall)

sign.sh Outdated

# Use secrets manager to obtain the key and passphrase into a JSON file
echo "Retrieving key..." >&2
aws --region us-east-1 secretsmanager get-secret-value --secret-id "$SECRET" --output text --query SecretString > $tmpdir/secret.txt
Copy link
Contributor

Choose a reason for hiding this comment

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

I rather we don't specify a region here. I believe the default region is the region where the build task runs, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But the secrets do live in a particular region (I've created them there), which is only incidentally the same one as the one where the build runs.

Copy link
Contributor

Choose a reason for hiding this comment

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

Does it make to make a cross-region call if our build fleet is deployed elsewhere or to put the secrets alongside each build fleet? (hypothetical of course).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question. I'll remove the region, with the result that the call will fail if we fail to colocate the build with the secrets. I guess that's preferable to "accidentally" placing them far away.

By the way, I will also add the key provisioning to the CI/CD stack.

@eladb
Copy link
Contributor

eladb commented May 31, 2018

Dry run?

@@ -2,16 +2,13 @@
"name": "docs",
"private": true,
"version": "0.6.0",
"description": "AWS CDK Documentation",
"description": "Generates the documentation for all packages",
Copy link
Contributor

Choose a reason for hiding this comment

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

Rebase

@@ -50,26 +50,32 @@ to instantiate the ``StorageLayer`` construct.

When you initialize a construct,
add the construct to the construct tree by specifying the parent construct as the first initializer parameter,
a unique (to your stack) identifier for the construct as the second parameter,
and an optional set of properties for the final parameter,
a unique identifier for the construct as the second parameter,
Copy link
Contributor

Choose a reason for hiding this comment

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

Still seeing doc updates here... something is weird

@rix0rrr rix0rrr changed the title Add signing script Add script to sign arbitrary files using the key stored in Secrets Manager May 31, 2018
@rix0rrr rix0rrr merged commit 57698c2 into master May 31, 2018
@rix0rrr rix0rrr deleted the huijbers/signing branch May 31, 2018 18:16
mpiroc added a commit that referenced this pull request Jul 11, 2018
john-shaskin added a commit to john-shaskin/aws-cdk that referenced this pull request Jan 18, 2019
# This is the 1st commit message:

Add MethodResponse support for aws-apigateway

# This is the commit message aws#2:

Remove Dockerfile that was no longer needed

# This is the commit message aws#3:

Update python base image from 3.6 to 3.6.5

# This is the commit message aws#4:

Make the dockerfile work
john-shaskin added a commit to john-shaskin/aws-cdk that referenced this pull request Jan 18, 2019
# This is the 1st commit message:

Add MethodResponse support for aws-apigateway

# This is the commit message aws#2:

Remove Dockerfile that was no longer needed

# This is the commit message aws#3:

Update python base image from 3.6 to 3.6.5

# This is the commit message aws#4:

Make the dockerfile work
@NGL321 NGL321 added the contribution/core This is a PR that came from AWS. label Sep 27, 2019
smguggen referenced this pull request in smguggen/aws-cdk Aug 24, 2021
# This is the 1st commit message:

Add Identity Pool construct

# This is the commit message #2:

Bug fixes

# This is the commit message #3:

Bug fixes

# This is the commit message #4:

Formatting

# This is the commit message #5:

Add construct methods

# This is the commit message #6:

Remove flat

# This is the commit message #7:

Fix issues
@Crycham Crycham mentioned this pull request Aug 5, 2022
2 tasks
jayhelios added a commit to jayhelios/aws-cdk that referenced this pull request Sep 28, 2022
Added Note 3 -Additional note that certain expected defaults for parameters may be different. 

**⚠ Note aws#3**: Expected defaults for certain parameters may be different with the hotswap parameter. For example, an ECS service's minimum healthy percentage will currently be set to 0. Please review the source accordingly if this occurs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants