Skip to content

Apply peer authentication policy at port level#20925

Merged
istio-testing merged 6 commits intoistio:masterfrom
diemtvu:mtls-beta-p4
Feb 8, 2020
Merged

Apply peer authentication policy at port level#20925
istio-testing merged 6 commits intoistio:masterfrom
diemtvu:mtls-beta-p4

Conversation

@diemtvu
Copy link
Copy Markdown
Contributor

@diemtvu diemtvu commented Feb 7, 2020 •
edited by istio-policy-bot
Loading

Issue: #20746

@diemtvu diemtvu requested a review from incfly February 7, 2020 04:06
@diemtvu diemtvu requested a review from a team as a code owner February 7, 2020 04:06
@googlebot googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Feb 7, 2020
@istio-testing istio-testing added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 7, 2020
@diemtvu
Copy link
Copy Markdown
Contributor Author

diemtvu commented Feb 7, 2020

/test integ-security-local-tests_istio

@diemtvu
Copy link
Copy Markdown
Contributor Author

diemtvu commented Feb 7, 2020

/test integ-istioio-k8s-tests_istio
/test lint_istio

yangminzhu
yangminzhu reviewed Feb 7, 2020
View reviewed changes
pilot/pkg/security/authn/v1beta1/policy_applier.go
if a.consolidatedPeerPolicy == nil {
return model.MTLSPermissive
}
if a.consolidatedPeerPolicy.PortLevelMtls != nil {
Copy link
Copy Markdown
Contributor

@yangminzhu yangminzhu Feb 7, 2020

Choose a reason for hiding this comment

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

What's the expected behavior for passthrough filter chain that doesn't have the port?

Copy link
Copy Markdown
Contributor Author

@diemtvu diemtvu Feb 7, 2020

Choose a reason for hiding this comment

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

The behavior should use the workload level mTLS (i.e a.consolidatedPeerPolicy.Mtls). Implementation-wise, you can change this function to treat a special value (i.e 0) to ignore looking up portLevel settings.

@yangminzhu
Copy link
Copy Markdown
Contributor

yangminzhu commented Feb 8, 2020

/test integ-pilot-k8s-tests_istio

@istio-testing istio-testing merged commit b3ffe91 into istio:master Feb 8, 2020
@istio-testing
Copy link
Copy Markdown
Collaborator

istio-testing commented Feb 8, 2020

In response to a cherrypick label: #20925 failed to apply on top of branch "release-1.5":

Using index info to reconstruct a base tree...
M	pilot/pkg/networking/plugin/authn/authentication.go
M	pilot/pkg/security/authn/policy_applier.go
M	pilot/pkg/security/authn/v1alpha1/policy_applier.go
M	pilot/pkg/security/authn/v1alpha1/policy_applier_test.go
M	pilot/pkg/security/authn/v1beta1/policy_applier.go
M	pilot/pkg/security/authn/v1beta1/policy_applier_test.go
M	tests/integration/security/reachability_test.go
Falling back to patching base and 3-way merge...
Auto-merging tests/integration/security/reachability_test.go
CONFLICT (content): Merge conflict in tests/integration/security/reachability_test.go
Auto-merging pilot/pkg/security/authn/v1beta1/policy_applier_test.go
CONFLICT (content): Merge conflict in pilot/pkg/security/authn/v1beta1/policy_applier_test.go
Auto-merging pilot/pkg/security/authn/v1beta1/policy_applier.go
CONFLICT (content): Merge conflict in pilot/pkg/security/authn/v1beta1/policy_applier.go
Auto-merging pilot/pkg/security/authn/v1alpha1/policy_applier_test.go
Auto-merging pilot/pkg/security/authn/v1alpha1/policy_applier.go
Auto-merging pilot/pkg/security/authn/policy_applier.go
CONFLICT (content): Merge conflict in pilot/pkg/security/authn/policy_applier.go
Auto-merging pilot/pkg/networking/plugin/authn/authentication.go
error: Failed to merge in the changes.
Patch failed at 0001 Apply peer authentication policy at port level

diemtvu added a commit to diemtvu/istio that referenced this pull request Feb 8, 2020
@diemtvu
Apply peer authentication policy at port level (istio#20925)
cd1033e 
* Apply peer authentication policy at port level

* Lint

* Fix e2e TestReachability/beta-per-port-mtls

* Fix mix api test

* Add log to test to debug resource apply/delete

* Add require env kube for new tests
istio-testing pushed a commit that referenced this pull request Feb 10, 2020
@diemtvu
Apply peer authentication policy at port level (#20925) (#20970)
70a9efd 
* Apply peer authentication policy at port level

* Lint

* Fix e2e TestReachability/beta-per-port-mtls

* Fix mix api test

* Add log to test to debug resource apply/delete

* Add require env kube for new tests
@diemtvu diemtvu deleted the mtls-beta-p4 branch February 10, 2020 21:48
sdake pushed a commit to sdake/istio that referenced this pull request Feb 21, 2020
@diemtvu
Apply peer authentication policy at port level (istio#20925)
1832b73 
* Apply peer authentication policy at port level

* Lint

* Fix e2e TestReachability/beta-per-port-mtls

* Fix mix api test

* Add log to test to debug resource apply/delete

* Add require env kube for new tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@yangminzhu yangminzhu yangminzhu left review comments

@incfly incfly incfly approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/security cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@diemtvu @yangminzhu @istio-testing @incfly @googlebot @istio-policy-bot