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

[Dual Stack] HTTPS ServiceEntry listener missing additional listen addresses #46743

Closed
2 tasks done
adams-shaun opened this issue Aug 29, 2023 · 1 comment · Fixed by #46795
Closed
2 tasks done

[Dual Stack] HTTPS ServiceEntry listener missing additional listen addresses #46743

adams-shaun opened this issue Aug 29, 2023 · 1 comment · Fixed by #46795
Assignees

Comments

@adams-shaun
Copy link
Contributor

adams-shaun commented Aug 29, 2023

Is this the right place to submit this?

  • This is not a security vulnerability or a crashing bug
  • This is not a question about how to use Istio

Bug Description

Setup:

Install Istio from master in dual-stack mode w/ REGISTRY_ONLY outboundTrafficPolicy:

pilot:
  env:
    ISTIO_DUAL_STACK: "true"

meshConfig:
  accessLogFile: /dev/stdout
  defaultConfig:
    proxyMetadata:
      ISTIO_DUAL_STACK: "true"
  outboundTrafficPolicy: 
    mode: REGISTRY_ONLY

Define an external service entry:

apiVersion: networking.istio.io/v1alpha3
kind: ServiceEntry
metadata:
  name: external-google
spec:
  hosts:
  - www.google.com
  ports:
  - number: 444
    name: https
    protocol: HTTPS
    targetPort: 443
  - number: 81
    name: http
    protocol: HTTP
    targetPort: 80
  location: MESH_EXTERNAL
  resolution: DNS

Inspect the port 444 listener configuration:

    {
     "name": "0.0.0.0_444",
     "active_state": {
      "version_info": "2023-08-29T18:09:12Z/6",
      "listener": {
       "@type": "type.googleapis.com/envoy.config.listener.v3.Listener",
       "name": "0.0.0.0_444",
       "address": {
        "socket_address": {
         "address": "0.0.0.0",
         "port_value": 444
        }
       },
       "filter_chains": [
        {
         "filter_chain_match": {
          "server_names": [
           "www.google.com"
          ]
         },
         "filters": [...],
       "listener_filters": [...],
       "listener_filters_timeout": "0s",
       "traffic_direction": "OUTBOUND",
       "continue_on_listener_filters_timeout": true,
       "default_filter_chain": {...},
       "bind_to_port": false
      },
      "last_updated": "2023-08-29T18:14:00.729Z"
     }
    },

Compare with port 81 (plain http) listener:

{
     "name": "0.0.0.0_81",
     "active_state": {
      "version_info": "2023-08-29T18:20:19Z/7",
      "listener": {
       "@type": "type.googleapis.com/envoy.config.listener.v3.Listener",
       "name": "0.0.0.0_81",
       "address": {
        "socket_address": {
         "address": "0.0.0.0",
         "port_value": 81
        }
       },
       "filter_chains": [...],
       "listener_filters": [...],
       "listener_filters_timeout": "0s",
       "traffic_direction": "OUTBOUND",
       "continue_on_listener_filters_timeout": true,
       "default_filter_chain": {...},
       "bind_to_port": false,
       "additional_addresses": [
        {
         "address": {
          "socket_address": {
           "address": "::",
           "port_value": 81
          }
         }
        }
       ]
      },
      "last_updated": "2023-08-29T18:20:19.681Z"
     }
    }

For the case of REGISTRY_ONLY, something akin to curl -6 https://... will fail.

Analysis:
This seems to be related to an empty return value here: https://github.com/istio/istio/blob/master/pilot/pkg/networking/core/v1alpha3/listener.go#L778
service.GetAddressForProxy will return a default 0.0.0.0 for the ServiceEntry case, perhaps we need a default additional address for these converted services?

Version

HEAD of master + https://github.com/istio/istio/pull/46630 cherry-pick

Additional Information

No response

@keithmattix
Copy link
Contributor

I would be interested in picking this up; sounds like the AddressMap is just missing a default for IPV6? I'll dig into how the map is populated for service entries

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.

3 participants