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

Restrict iam policy by tagging SG rules and shield protection #3799

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
68 changes: 66 additions & 2 deletions docs/install/iam_policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,18 +61,82 @@
"wafv2:DisassociateWebACL",
"shield:GetSubscriptionState",
"shield:DescribeProtection",
"shield:CreateProtection",
"shield:DeleteProtection"
],
"Resource": "*"
},
{
"Effect": "Allow",
"Action": [
"shield:CreateProtection"
],
"Resource": "*",
"Condition": {
"Null": {
"aws:RequestTag/elbv2.k8s.aws/cluster": "false"
}
}
},
{
"Effect": "Allow",
"Action": [
"shield:TagResource"
],
"Resource": "arn:aws:shield::*:protection/*",
"Condition": {
"Null": {
"aws:RequestTag/elbv2.k8s.aws/cluster": "false"
}
}
},
{
"Effect": "Allow",
"Action": [
"shield:DeleteProtection"
],
"Resource": "arn:aws:shield::*:protection/*",
"Condition": {
"Null": {
"aws:ResourceTag/elbv2.k8s.aws/cluster": "false"
}
}
},
{
"Sid": "MutateSecurityGroupRules",
"Effect": "Allow",
"Action": [
"ec2:AuthorizeSecurityGroupIngress",
"ec2:RevokeSecurityGroupIngress"
],
"Resource": "*"
"Resource": "arn:aws:ec2:*:*:security-group-rule/*",
"Condition": {
"Null": {
"aws:ResourceTag/elbv2.k8s.aws/cluster": "false"
}
}
},
{
"Sid": "TagSecurityGroupRule",
"Effect": "Allow",
"Action": [
"ec2:CreateTags"
],
"Resource": "arn:aws:ec2:*:*:security-group-rule/*",
"Condition": {
"Null": {
"aws:RequestTag/elbv2.k8s.aws/cluster": "false",
"aws:ResourceTag/elbv2.k8s.aws/cluster": "true"
}
}
},
{
"Sid": "MutateSGRulesOnWorkerNodeSG",
"Effect": "Allow",
"Action": [
"ec2:AuthorizeSecurityGroupIngress",
"ec2:RevokeSecurityGroupIngress"
],
"Resource": "arn:aws:ec2:*:*:security-group/*"
},
{
"Effect": "Allow",
Expand Down
4 changes: 2 additions & 2 deletions pkg/deploy/ec2/security_group_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func (m *defaultSecurityGroupManager) Create(ctx context.Context, resSG *ec2mode
"resourceID", resSG.ID(),
"securityGroupID", sgID)

if err := m.networkingSGReconciler.ReconcileIngress(ctx, sgID, permissionInfos); err != nil {
if err := m.networkingSGReconciler.ReconcileIngress(ctx, sgID, permissionInfos, m.trackingProvider.GetClusterName()); err != nil {
return ec2model.SecurityGroupStatus{}, err
}

Expand All @@ -107,7 +107,7 @@ func (m *defaultSecurityGroupManager) Update(ctx context.Context, resSG *ec2mode
if err := m.updateSDKSecurityGroupGroupWithTags(ctx, resSG, sdkSG); err != nil {
return ec2model.SecurityGroupStatus{}, err
}
if err := m.networkingSGReconciler.ReconcileIngress(ctx, sdkSG.SecurityGroupID, permissionInfos); err != nil {
if err := m.networkingSGReconciler.ReconcileIngress(ctx, sdkSG.SecurityGroupID, permissionInfos, m.trackingProvider.GetClusterName()); err != nil {
return ec2model.SecurityGroupStatus{}, err
}
return ec2model.SecurityGroupStatus{
Expand Down
13 changes: 11 additions & 2 deletions pkg/deploy/shield/protection_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const (
// service subscription rarely changes, cache it with longer period.
defaultSubscriptionStateCacheTTL = 2 * time.Hour
subscriptionStateCacheKey = "subscriptionState"
tagKeyK8sCluster = "elbv2.k8s.aws/cluster"
)

type ProtectionManager interface {
Expand All @@ -33,9 +34,10 @@ type ProtectionManager interface {
IsSubscribed(ctx context.Context) (bool, error)
}

func NewDefaultProtectionManager(shieldClient services.Shield, logger logr.Logger) *defaultProtectionManager {
func NewDefaultProtectionManager(shieldClient services.Shield, clusterName string, logger logr.Logger) *defaultProtectionManager {
return &defaultProtectionManager{
shieldClient: shieldClient,
clusterName: clusterName,
logger: logger,
protectionInfoByResourceARNCache: cache.NewExpiring(),
protectionInfoByResourceARNCacheTTL: defaultProtectionInfoByResourceARNCacheTTL,
Expand All @@ -48,6 +50,7 @@ var _ ProtectionManager = &defaultProtectionManager{}

type defaultProtectionManager struct {
shieldClient services.Shield
clusterName string
logger logr.Logger

protectionInfoByResourceARNCache *cache.Expiring
Expand All @@ -65,10 +68,16 @@ func (m *defaultProtectionManager) CreateProtection(ctx context.Context, resourc
req := &shieldsdk.CreateProtectionInput{
ResourceArn: awssdk.String(resourceARN),
Name: awssdk.String(protectionName),
Tags: []*shieldsdk.Tag{
&shieldsdk.Tag{
Key: awssdk.String(tagKeyK8sCluster),
Value: awssdk.String(m.clusterName),
},
},
}
m.logger.Info("enabling shield protection",
"resourceARN", resourceARN,
"protectionName", protectionName)
"protectionName", protectionName, "TagKey", tagKeyK8sCluster, "TagValue", m.clusterName)
resp, err := m.shieldClient.CreateProtectionWithContext(ctx, req)
if err != nil {
return "", err
Expand Down
4 changes: 3 additions & 1 deletion pkg/deploy/stack_deployer.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func NewDefaultStackDeployer(cloud aws.Cloud, k8sClient client.Client,
elbv2TGBManager: elbv2.NewDefaultTargetGroupBindingManager(k8sClient, trackingProvider, logger),
wafv2WebACLAssociationManager: wafv2.NewDefaultWebACLAssociationManager(cloud.WAFv2(), logger),
wafRegionalWebACLAssociationManager: wafregional.NewDefaultWebACLAssociationManager(cloud.WAFRegional(), logger),
shieldProtectionManager: shield.NewDefaultProtectionManager(cloud.Shield(), logger),
shieldProtectionManager: shield.NewDefaultProtectionManager(cloud.Shield(), config.ClusterName, logger),
featureGates: config.FeatureGates,
vpcID: cloud.VpcID(),
logger: logger,
Expand Down Expand Up @@ -106,6 +106,8 @@ func (d *defaultStackDeployer) Deploy(ctx context.Context, stack core.Stack) err
d.logger.Error(err, "unable to determine AWS Shield subscription state, skipping AWS shield reconciliation")
} else if shieldSubscribed {
synthesizers = append(synthesizers, shield.NewProtectionSynthesizer(d.shieldProtectionManager, d.logger, stack))
} else if !shieldSubscribed {
d.logger.Info("Shield enabled but not subscribed, skipping AWS shield reconciliation.")
}
}

Expand Down
7 changes: 7 additions & 0 deletions pkg/deploy/tracking/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ type Provider interface {
// StackTags provide the tags for stack.
StackTags(stack core.Stack) map[string]string

// GetClusterName gets the cluster name
GetClusterName() string

// ResourceTags provide the tags for stack resources
ResourceTags(stack core.Stack, res core.Resource, additionalTags map[string]string) map[string]string

Expand Down Expand Up @@ -90,6 +93,10 @@ func (p *defaultProvider) StackTags(stack core.Stack) map[string]string {
}
}

func (p *defaultProvider) GetClusterName() string {
return p.clusterName
}

func (p *defaultProvider) ResourceTags(stack core.Stack, res core.Resource, additionalTags map[string]string) map[string]string {
stackTags := p.StackTags(stack)
resourceIDTags := map[string]string{
Expand Down
18 changes: 15 additions & 3 deletions pkg/networking/security_group_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ type SecurityGroupManager interface {
FetchSGInfosByRequest(ctx context.Context, req *ec2sdk.DescribeSecurityGroupsInput) (map[string]SecurityGroupInfo, error)

// AuthorizeSGIngress will authorize Ingress permissions to SecurityGroup.
AuthorizeSGIngress(ctx context.Context, sgID string, permissions []IPPermissionInfo) error
AuthorizeSGIngress(ctx context.Context, sgID string, permissions []IPPermissionInfo, clusterName string) error

// RevokeSGIngress will revoke Ingress permissions from SecurityGroup.
RevokeSGIngress(ctx context.Context, sgID string, permissions []IPPermissionInfo) error
Expand Down Expand Up @@ -125,15 +125,27 @@ func (m *defaultSecurityGroupManager) FetchSGInfosByRequest(ctx context.Context,
return sgInfosByID, nil
}

func (m *defaultSecurityGroupManager) AuthorizeSGIngress(ctx context.Context, sgID string, permissions []IPPermissionInfo) error {
func (m *defaultSecurityGroupManager) AuthorizeSGIngress(ctx context.Context, sgID string, permissions []IPPermissionInfo, clusterName string) error {
sdkIPPermissions := buildSDKIPPermissions(permissions)
tags := []*ec2sdk.Tag{
&ec2sdk.Tag{
Key: awssdk.String(tagKeyK8sCluster),
Value: awssdk.String(clusterName),
},
}
req := &ec2sdk.AuthorizeSecurityGroupIngressInput{
GroupId: awssdk.String(sgID),
IpPermissions: sdkIPPermissions,
TagSpecifications: []*ec2sdk.TagSpecification{
{
ResourceType: awssdk.String("security-group-rule"),
Tags: tags,
},
},
}
m.logger.Info("authorizing securityGroup ingress",
"securityGroupID", sgID,
"permission", sdkIPPermissions)
"permission", sdkIPPermissions, "Tags", tags)
if _, err := m.ec2Client.AuthorizeSecurityGroupIngressWithContext(ctx, req); err != nil {
return err
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/networking/security_group_manager_mocks.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 6 additions & 6 deletions pkg/networking/security_group_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func WithAuthorizeOnly(authorizeOnly bool) SecurityGroupReconcileOption {
// SecurityGroupReconciler manages securityGroup rules on securityGroup.
type SecurityGroupReconciler interface {
// ReconcileIngress will reconcile Ingress permission on SecurityGroup to be desiredPermission.
ReconcileIngress(ctx context.Context, sgID string, desiredPermissions []IPPermissionInfo, opts ...SecurityGroupReconcileOption) error
ReconcileIngress(ctx context.Context, sgID string, desiredPermissions []IPPermissionInfo, clusterName string, opts ...SecurityGroupReconcileOption) error
}

// NewDefaultSecurityGroupReconciler constructs new defaultSecurityGroupReconciler.
Expand All @@ -66,7 +66,7 @@ type defaultSecurityGroupReconciler struct {
logger logr.Logger
}

func (r *defaultSecurityGroupReconciler) ReconcileIngress(ctx context.Context, sgID string, desiredPermissions []IPPermissionInfo, opts ...SecurityGroupReconcileOption) error {
func (r *defaultSecurityGroupReconciler) ReconcileIngress(ctx context.Context, sgID string, desiredPermissions []IPPermissionInfo, clusterName string, opts ...SecurityGroupReconcileOption) error {
reconcileOpts := SecurityGroupReconcileOptions{
PermissionSelector: labels.Everything(),
}
Expand All @@ -77,7 +77,7 @@ func (r *defaultSecurityGroupReconciler) ReconcileIngress(ctx context.Context, s
return err
}
sgInfo := sgInfoByID[sgID]
if err := r.reconcileIngressWithSGInfo(ctx, sgInfo, desiredPermissions, reconcileOpts); err != nil {
if err := r.reconcileIngressWithSGInfo(ctx, sgInfo, desiredPermissions, clusterName, reconcileOpts); err != nil {
if !r.shouldRetryWithoutCache(err) {
return err
}
Expand All @@ -86,14 +86,14 @@ func (r *defaultSecurityGroupReconciler) ReconcileIngress(ctx context.Context, s
return err
}
sgInfo := sgInfoByID[sgID]
if err := r.reconcileIngressWithSGInfo(ctx, sgInfo, desiredPermissions, reconcileOpts); err != nil {
if err := r.reconcileIngressWithSGInfo(ctx, sgInfo, desiredPermissions, clusterName, reconcileOpts); err != nil {
return err
}
}
return nil
}

func (r *defaultSecurityGroupReconciler) reconcileIngressWithSGInfo(ctx context.Context, sgInfo SecurityGroupInfo, desiredPermissions []IPPermissionInfo, reconcileOpts SecurityGroupReconcileOptions) error {
func (r *defaultSecurityGroupReconciler) reconcileIngressWithSGInfo(ctx context.Context, sgInfo SecurityGroupInfo, desiredPermissions []IPPermissionInfo, clusterName string, reconcileOpts SecurityGroupReconcileOptions) error {
extraPermissions := diffIPPermissionInfos(sgInfo.Ingress, desiredPermissions)
permissionsToRevoke := make([]IPPermissionInfo, 0, len(extraPermissions))
for _, permission := range extraPermissions {
Expand All @@ -108,7 +108,7 @@ func (r *defaultSecurityGroupReconciler) reconcileIngressWithSGInfo(ctx context.
}
}
if len(permissionsToGrant) > 0 {
if err := r.sgManager.AuthorizeSGIngress(ctx, sgInfo.SecurityGroupID, permissionsToGrant); err != nil {
if err := r.sgManager.AuthorizeSGIngress(ctx, sgInfo.SecurityGroupID, permissionsToGrant, clusterName); err != nil {
return err
}
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/targetgroupbinding/networking_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ func (m *defaultNetworkingManager) reconcileWithIngressPermissionsPerSG(ctx cont
permissionSelector := labels.SelectorFromSet(labels.Set{tgbNetworkingIPPermissionLabelKey: tgbNetworkingIPPermissionLabelValue})
var sgReconciliationErrors []error
for sgID, permissions := range aggregatedIngressPermissionsPerSG {
if err := m.sgReconciler.ReconcileIngress(ctx, sgID, permissions,
if err := m.sgReconciler.ReconcileIngress(ctx, sgID, permissions, m.clusterName,
networking.WithPermissionSelector(permissionSelector),
networking.WithAuthorizeOnly(!computedForAllTGBs)); err != nil {
sgReconciliationErrors = append(sgReconciliationErrors, err)
Expand Down Expand Up @@ -485,7 +485,7 @@ func (m *defaultNetworkingManager) gcIngressPermissionsFromUnusedEndpointSGs(ctx

permissionSelector := labels.SelectorFromSet(labels.Set{tgbNetworkingIPPermissionLabelKey: tgbNetworkingIPPermissionLabelValue})
for sgID := range unusedEndpointSGs {
err := m.sgReconciler.ReconcileIngress(ctx, sgID, nil,
err := m.sgReconciler.ReconcileIngress(ctx, sgID, nil, m.clusterName,
networking.WithPermissionSelector(permissionSelector))
if err != nil {
if isEC2SecurityGroupNotFoundError(err) {
Expand Down
Loading