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

Destination rule to set mTLS ignores specific host #19459

Closed
FrimIdan opened this issue Dec 8, 2019 · 4 comments · Fixed by #21734
Closed

Destination rule to set mTLS ignores specific host #19459

FrimIdan opened this issue Dec 8, 2019 · 4 comments · Fixed by #21734
Assignees

Comments

@FrimIdan
Copy link
Contributor

FrimIdan commented Dec 8, 2019

Bug description
Following the discussion here.
DestinationRule host resolution is wrong when DestinationRule with a specific host is affecting wildcard service entry host.

Expected behavior
Only service entry with the exact host should be affected by the DestinationRule

Steps to reproduce the bug
Create 2 service entries:

apiVersion: networking.istio.io/v1alpha3
kind: ServiceEntry
metadata:
  name: external-svc-http
spec:
  hosts:
  - "*.test.com" 
  location: MESH_EXTERNAL
  ports:
  - number: 8080
    name: http
    protocol: HTTP
  resolution: NONE

apiVersion: networking.istio.io/v1alpha3
kind: ServiceEntry
metadata:
  name: specific-external-svc-http
spec:
  hosts:
  - test.test.com 
  location: MESH_EXTERNAL
  ports:
  - number: 80
    name: http
    protocol: HTTP
  resolution: NONE

Create a DestinationRule to encrypt traffic only for the specific host

apiVersion: networking.istio.io/v1alpha3
kind: DestinationRule
metadata:
  name: test-test-istio-mtls
  namespace: test
spec:
  host: "test.test.com"
  trafficPolicy:
    tls:
      mode: ISTIO_MUTUAL

Look at any service config dump and found that the wildcard service entry was affected

{
 "version_info": "2019-11-26T12:50:00Z/42",
 "cluster": {
  "name": "outbound|8080||*.test.com",
  "type": "ORIGINAL_DST",
  "connect_timeout": "10s",
  "lb_policy": "CLUSTER_PROVIDED",
  "circuit_breakers": {
   "thresholds": [
    {
     "max_retries": 1024
    }
   ]
  },
  "tls_context": {
   "common_tls_context": {
    "tls_certificates": [
     {
      "certificate_chain": {
       "filename": "/etc/certs/cert-chain.pem"
      },
      "private_key": {
       "filename": "/etc/certs/key.pem"
      }
     }
    ],
    "validation_context": {
     "trusted_ca": {
      "filename": "/etc/certs/root-cert.pem"
     }
    },
    "alpn_protocols": [
     "istio"
    ]
   },
   "sni": "outbound_.80_._.*.test.com"
  },
  "metadata": {
   "filter_metadata": {
    "istio": {
     "config": "/apis/networking/v1alpha3/namespaces/test/destination-rule/test-test-istio-mtls"
    }
   }
  }
 },
 "last_updated": "2019-11-26T12:50:00.192Z"
}

Version (include the output of istioctl version --remote and kubectl version and helm version if you used Helm)
Istio 1.3.2

How was Istio installed?
Not relevant to the bug

Environment where bug was observed (cloud vendor, OS, etc)
Not relevant to the bug

@howardjohn
Copy link
Member

Thanks for the clear reproduction steps!

@elieser1101
Copy link

hi, i would like to help with this, can you give me a little guidance as this is my first issue!

@incfly
Copy link

incfly commented Jan 14, 2020

@elieser1101 Here is based on my understanding, hope if helps.

When Pilot builds up the destination rule for a outbound cluster push, we pre-calculate the matching destination rules that match with current service,

// DestinationRule returns a destination rule for a service name in a given domain.

You can see there're some function calls of MostSpecificHostMatch which further invoke the Name.Matches.

Based on this bug description, we should not have test.test.com DestinationRule applies to ServiceEntry whose host is *.test.com.

So a viable suggestion:

  • Configure with above reproducible config (ServiceEntry and DestinationRule)
  • Add some instrument print/debugging log for the matches logic, true/or false, which one is selected during the loop iteration, etc.
  • Build your own Pilot and Docker image, modify istio-pilot deployment to use your own image.
  • Observe the print out and get better understanding, where is going wrong. starting from there.

@yp28
Copy link
Contributor

yp28 commented Mar 2, 2020

/assign

edit: no can do?

yp28 added a commit to yp28/istio that referenced this issue Mar 5, 2020
Given there is a ServiceEntry for *.bar.com, a DestinationRule
for foo.bar.com applies to SerivceEntry/*.bar.com while it shouldn't.
This PR fixes this issue by changing the way of matching
DestinationRules from overlaps to subsets of a ServiceEntry hostname.
istio-testing pushed a commit that referenced this issue Apr 7, 2020
…21857)

* DestinationRule for specific host matches wildcards (fixes #19459)

Given there is a ServiceEntry for *.bar.com, a DestinationRule
for foo.bar.com applies to SerivceEntry/*.bar.com while it shouldn't.
This PR fixes this issue by changing the way of matching
DestinationRules from overlaps to subsets of a ServiceEntry hostname.

* fix formatting for unit test cases
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants