-
Notifications
You must be signed in to change notification settings - Fork 1
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
CL-522 | Add SignerSigningJob
module to revoke signing jobs
#3
Conversation
Signing jobs are viewable by the ListSigningJobs operation for two years after they are performed [1] As a precaution we are updating Signing jobs statuses to revoked. This indicates that the signature is no longer valid. [1] https://awscli.amazonaws.com/v2/documentation/api/latest/reference/signer/start-signing-job.html Signed-off-by: Gabriela S. Soria <gsoria@oreilly.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great PR, @gsoria ! I left a few minor suggestions inline that I think may help align some functionality with other upstream modules.
|
||
func (j *SignerSigningJob) Properties() types.Properties { | ||
properties := types.NewProperties() | ||
properties.Set("JobId", j.jobId) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think any of the other (many) properties returned by ListSigningJobs would be good to add to properties for users to be able to filter on?
I was thinking maybe createdAt
, profile*
, platform*
, job*
attributes? I know I see a lot of aws nuke code changes to add more and more properties to give users more powerful filtering.
{
'jobs': [
{
'jobId': 'string',
'source': {
's3': {
'bucketName': 'string',
'key': 'string',
'version': 'string'
}
},
'signedObject': {
's3': {
'bucketName': 'string',
'key': 'string'
}
},
'signingMaterial': {
'certificateArn': 'string'
},
'createdAt': datetime(2015, 1, 1),
'status': 'InProgress'|'Failed'|'Succeeded',
'isRevoked': True|False,
'profileName': 'string',
'profileVersion': 'string',
'platformId': 'string',
'platformDisplayName': 'string',
'signatureExpiresAt': datetime(2015, 1, 1),
'jobOwner': 'string',
'jobInvoker': 'string'
},
],
'NextToken': 'string'
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my first approach, I didn't think much about filtering these jobs. Thanks for the suggestions, I included more properties as part of 2fae9cc. As we discuss in the meeting yesterday, instead of including the job in the struct, I included only the properties I wanted to use. Let me know what you think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great to my untrained eye! I'm still a bit unsure when to use a pointer like you have in your type def. I think the way you have it should keep it performant.
Signed-off-by: Gabriela S. Soria <gsoria@oreilly.com>
Signed-off-by: Gabriela S. Soria <gsoria@oreilly.com>
Signed-off-by: Gabriela S. Soria <gsoria@oreilly.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Following your testing procedure with your branches, I was able to recreate your intended behavior.
Before aws-nuke:
$ aws signer list-signing-jobs --status Succeeded
{
"jobs": [
{
"jobId": "db882524-902e-4bd6-ac03-a66df08e73a0",
"source": {
"s3": {
"bucketName": "awscookbook505-src-pl42eg",
"key": "lambda_function.zip",
"version": "cbIsFsfeGezV8l6IkH2bDLhAAe_5sjgk"
}
},
"signedObject": {
"s3": {
"bucketName": "awscookbook505-dst-pl42eg",
"key": "signed-db882524-902e-4bd6-ac03-a66df08e73a0.zip"
}
},
"signingMaterial": {},
"createdAt": "2023-04-28T22:03:08+00:00",
"status": "Succeeded",
"isRevoked": false,
"profileName": "AWSCookbook505_pl42eg",
"profileVersion": "ELAcw04kNv",
"platformId": "AWSLambda-SHA384-ECDSA",
"platformDisplayName": "AWS Lambda",
"signatureExpiresAt": "2034-07-28T22:03:08+00:00",
"jobOwner": "430037344291",
"jobInvoker": "430037344291"
}
]
}
Found by aws-nuke:
us-east-1 - SignerSigningJob - [CreatedAt: "2023-04-28T22:03:08Z", JobId: "db882524-902e-4bd6-ac03-a66df08e73a0", JobInvoker: "430037344291", JobOwner: "430037344291", PlatformDisplayName: "AWS Lambda", PlatformId: "AWSLambda-SHA384-ECDSA", ProfileName: "AWSCookbook505_pl42eg", ProfileVersion: "ELAcw04kNv"] - would remove
Successfully nuked:
us-east-1 - SignerSigningJob - [CreatedAt: "2023-04-28T22:03:08Z", JobId: "db882524-902e-4bd6-ac03-a66df08e73a0", JobInvoker: "430037344291", JobOwner: "430037344291", PlatformDisplayName: "AWS Lambda", PlatformId: "AWSLambda-SHA384-ECDSA", ProfileName: "AWSCookbook505_pl42eg", ProfileVersion: "ELAcw04kNv"] - removed
Subsequent list-signing-jobs
output from the same account:
{
"jobs": [
{
"jobId": "db882524-902e-4bd6-ac03-a66df08e73a0",
"source": {
"s3": {
"bucketName": "awscookbook505-src-pl42eg",
"key": "lambda_function.zip",
"version": "cbIsFsfeGezV8l6IkH2bDLhAAe_5sjgk"
}
},
"signedObject": {
"s3": {
"bucketName": "awscookbook505-dst-pl42eg",
"key": "signed-db882524-902e-4bd6-ac03-a66df08e73a0.zip"
}
},
"signingMaterial": {},
"createdAt": 1682719388,
"status": "Succeeded",
"isRevoked": true,
"profileName": "AWSCookbook505_pl42eg",
"profileVersion": "ELAcw04kNv",
"platformId": "AWSLambda-SHA384-ECDSA",
"platformDisplayName": "AWS Lambda",
"signatureExpiresAt": 2037736988,
"jobOwner": "430037344291",
"jobInvoker": "430037344291"
}
]
}
👍 👍 Two thumbs up from me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything looks good to me now! Great job, Gaby!
Thanks for the review @sstoops @danarbaugh ❤️ |
This PR includes a new module to handle Signer signing jobs. Signing jobs are viewable by the
ListSigningJobs
operation for two years after they are performed [1] As a precaution we are updating Signing jobs statuses to revoked. This indicates that the signature is no longer valid.[1] https://awscli.amazonaws.com/v2/documentation/api/latest/reference/signer/start-signing-job.html
Testing
Tests were performed using existing test scripts for AWS signer. Credits to @corybekk
Listing the signing jobs, we can see the signing job
f79fa22e-e4be-4751-a91b-93184bf68d57
with"isRevoked": false,
.Using this new module,
aws-nuke
the signing job was revoked as expected:When verifying the signing jobs again, we can see the signing job
f79fa22e-e4be-4751-a91b-93184bf68d57
with"isRevoked": true,