Skip to content

Commit 280b876

Browse files
authored
fix(iam): oidc provider fetches leaf certificate thumbprint instead of root (#22802)
Currently, the implementation of the OIDC provider custom resource fetches the certificate chain using: https://github.com/aws/aws-cdk/blob/9bde9f3149cbfa6e7b97204f54e7cef5c9127971/packages/%40aws-cdk/aws-iam/lib/oidc-provider/external.ts#L40 It then extracts the root certificate by detecting a circular reference in the `cert.issuerCertificate` property. https://github.com/aws/aws-cdk/blob/9bde9f3149cbfa6e7b97204f54e7cef5c9127971/packages/%40aws-cdk/aws-iam/lib/oidc-provider/external.ts#L46 As it turns out, this results in the wrong certificate being extracted. I observed this while running an [EKS integration test](https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk/aws-eks/test/integ.eks-service-account-sdk-call.ts). The current certificate thumbprint that is extracted is: `AD7E1C28B064EF8F6003402014C3D0E3370EB58A`. While the expected thumbprint is: `9E99A48A9960B14926BB7F3B02E22DA2B0AB7280`. (this is the value we used to [hardcode](https://github.com/aws/aws-cdk/blob/v2.50.0/packages/%40aws-cdk/aws-eks/lib/oidc-provider.ts#L49)) The [recommended way](https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_providers_create_oidc_verify-thumbprint.html) for extracting the correct thumbprint according to AWS is using `openssl s_client -showcerts`. When I tried this, I did in fact see that the last certificate returned by this command has the correct thumbprint. During investigation, I noticed that `socket.getPeerCertificate(true)` returns another additional certificate, acting as the root one. This is aligned with what the [comment](#8607 (comment)) made in the original issue. This additional certificate is not the correct one, and we should be using the one just before it in the chain. There is however no way to detect that "second to last" certificate in this way, because it doesn't resolve to a circular reference. After some digging, I switched the implementation to use `socket.getPeerX509Certificate()`, a new method that only exists in Node16. This method skips over the incorrect certificate, and results in the correct thumbprint. <img width="539" alt="Screen Shot 2022-11-06 at 10 04 51 PM" src="https://user-images.githubusercontent.com/1428812/200195623-6735377b-a82f-472f-884d-7bec450c32c6.png"> Fixes #8607 ---- ### All Submissions: * [X] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 0e97c15 commit 280b876

File tree

105 files changed

+1943
-1830
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

105 files changed

+1943
-1830
lines changed

packages/@aws-cdk/aws-eks/lib/oidc-provider.ts

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,18 +41,11 @@ export class OpenIdConnectProvider extends iam.OpenIdConnectProvider {
4141
* @param props Initialization properties
4242
*/
4343
public constructor(scope: Construct, id: string, props: OpenIdConnectProviderProps) {
44-
/**
45-
* For some reason EKS isn't validating the root certificate but a intermediate certificate
46-
* which is one level up in the tree. Because of the a constant thumbprint value has to be
47-
* stated with this OpenID Connect provider. The certificate thumbprint is the same for all the regions.
48-
*/
49-
const thumbprints = ['9e99a48a9960b14926bb7f3b02e22da2b0ab7280'];
5044

5145
const clientIds = ['sts.amazonaws.com'];
5246

5347
super(scope, id, {
5448
url: props.url,
55-
thumbprints,
5649
clientIds,
5750
});
5851
}

packages/@aws-cdk/aws-eks/test/bucket-pinger/bucket-pinger.ts

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,27 @@
1-
import * as ec2 from '@aws-cdk/aws-ec2';
21
import * as iam from '@aws-cdk/aws-iam';
32
import * as lambda from '@aws-cdk/aws-lambda';
43
import { CustomResource, Token, Duration } from '@aws-cdk/core';
54
import * as cr from '@aws-cdk/custom-resources';
65
import { Construct } from 'constructs';
76

8-
export interface PingerProps {
9-
readonly securityGroup?: ec2.SecurityGroup;
10-
readonly vpc?: ec2.IVpc;
11-
readonly subnets?: ec2.ISubnet[];
7+
export interface BucketPingerProps {
8+
readonly bucketName: string;
129
}
1310
export class BucketPinger extends Construct {
1411

1512
private _resource: CustomResource;
1613

17-
constructor(scope: Construct, id: string, props: PingerProps) {
14+
constructor(scope: Construct, id: string, props: BucketPingerProps) {
1815
super(scope, id);
1916

2017
const func = new lambda.Function(this, 'Function', {
2118
code: lambda.Code.fromAsset(`${__dirname}/function`),
2219
handler: 'index.handler',
2320
runtime: lambda.Runtime.PYTHON_3_9,
24-
vpc: props.vpc,
25-
vpcSubnets: props.subnets ? { subnets: props.subnets } : undefined,
26-
securityGroups: props.securityGroup ? [props.securityGroup] : undefined,
2721
timeout: Duration.minutes(1),
22+
environment: {
23+
BUCKET_NAME: props.bucketName,
24+
},
2825
});
2926

3027
if (!func.role) {
@@ -33,7 +30,7 @@ export class BucketPinger extends Construct {
3330

3431
func.role.addToPrincipalPolicy(new iam.PolicyStatement({
3532
actions: ['s3:DeleteBucket', 's3:ListBucket'],
36-
resources: ['arn:aws:s3:::*'],
33+
resources: [`arn:aws:s3:::${props.bucketName}`],
3734
}));
3835

3936
const provider = new cr.Provider(this, 'Provider', {

packages/@aws-cdk/aws-eks/test/bucket-pinger/function/index.py

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import json
22
import logging
33
import boto3
4+
import time
5+
import os
46

57
logger = logging.getLogger()
68
logger.setLevel(logging.INFO)
@@ -11,17 +13,19 @@ def handler(event, context):
1113
request_type = event['RequestType']
1214
props = event['ResourceProperties']
1315

14-
s3_bucket_name = 'amazingly-made-sdk-call-created-eks-bucket'
16+
s3_bucket_name = os.environ['BUCKET_NAME']
1517
s3 = boto3.client('s3')
1618

1719
if request_type in ['Create', 'Update']:
1820
logger.info(f'making sdk call to check if bucket with name {s3_bucket_name} exists')
21+
while (True): # lambda will eventually time this out in case of consistent failures
22+
try:
23+
s3.head_bucket(Bucket=s3_bucket_name)
24+
return {'Data': {'Value': f'confirmed that bucket with name {s3_bucket_name} exists' }}
25+
except Exception as error:
26+
logger.error(f'failed to head bucket with error: {str(error)}')
27+
time.sleep(5)
1928

20-
try:
21-
s3.head_bucket(Bucket=s3_bucket_name)
22-
except Exception as error:
23-
raise RuntimeError(f'failed to head bucket with error: {str(error)}')
24-
return {'Data': {'Value': f'confirmed that bucket with name {s3_bucket_name} exists' }}
2529

2630
elif request_type == 'Delete':
2731
logger.info(f'making sdk call to delete bucket with name {s3_bucket_name}')

packages/@aws-cdk/aws-eks/test/cluster.test.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2156,9 +2156,6 @@ describe('cluster', () => {
21562156
ClientIDList: [
21572157
'sts.amazonaws.com',
21582158
],
2159-
ThumbprintList: [
2160-
'9e99a48a9960b14926bb7f3b02e22da2b0ab7280',
2161-
],
21622159
Url: {
21632160
'Fn::GetAtt': [
21642161
'Cluster9EE0221C',

packages/@aws-cdk/aws-eks/test/integ.alb-controller.js.snapshot/asset.56b85a7bb756e34ab12549549eb40d34151db41531599e8f2be6c04e8ae66057/external.js

Lines changed: 94 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)