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

Upgrading to 0.77 on IPv6 cluster results in hostname mismatch #1109

Closed
jpmcb opened this issue Dec 22, 2022 · 6 comments · Fixed by #1184
Closed

Upgrading to 0.77 on IPv6 cluster results in hostname mismatch #1109

jpmcb opened this issue Dec 22, 2022 · 6 comments · Fixed by #1184
Labels
bug Something isn't working rustls rustls-tls related

Comments

@jpmcb
Copy link
Contributor

jpmcb commented Dec 22, 2022

Current and expected behavior

Upgraded to 0.77 of kube for our controller, which uses the rustls-tls feature, and now, we are seeing the follow errors on creating clients:

2022-12-22T19:02:42.827284Z ERROR kube_client::client::builder: failed with error error trying to connect:
error:0A000086:SSL routines:tls_post_process_server_certificate:certificate verify failed:ssl/state/statem_clnt.c:1887:
hostname mismatch at /src/.cargo/registry/src/github.com-1ecc6299db9ec823/kube-client-0.77.0/src/client/builder.rs:157 

We are using

let k8s_client = kube::client::Client::try_default()
  .await
  .context(controller_error::ClientCreateSnafu)?;

to create an inferred client from the environment.

I dropped the following into my code just before the client is created to see what was going on and redeployed the controller:

let infered_config = kube::Config::infer().await;

match infered_config {
    Ok(v) => println!("infered config: {v:?}"),
    Err(e) => println!("error: {e:?}"),
}

Here's what it looks like with 0.76:

infered config: Config {
  cluster_url: https://kubernetes.default.svc/,
  default_namespace: "brupop-bottlerocket-aws",
  root_cert: ...
}

Here's what it looks like with 0.77:

infered config: Config { 
  cluster_url: https://[fdc1:cad0:d971::1]/, 
  default_namespace: "brupop-bottlerocket-aws", 
  root_cert: ...
}

I'm guessing this is likely related to #1003. However, my assumption reading the following code:

/// Load an in-cluster Kubernetes client configuration using
/// [`Config::incluster_env`].
///
/// # Rustls-specific behavior
/// Rustls does not support validating IP addresses (see
/// <https://github.com/kube-rs/kube/issues/1003>).
/// To work around this, when rustls is configured, this function automatically appends
/// `tls-server-name = "kubernetes.default.svc"` to the resulting configuration.
/// Overriding or unsetting `Config::tls_server_name` will avoid this behaviour.
pub fn incluster() -> Result<Self, InClusterError> {
let mut cfg = Self::incluster_env()?;
if cfg!(all(not(feature = "openssl-tls"), feature = "rustls-tls")) {
// openssl takes precedence when both features present, so only do it when only rustls is there
cfg.tls_server_name = Some("kubernetes.default.svc".to_string());
}
Ok(cfg)
}

leads me to believe this should be covered and we'd get a similar behavior to when incluster_dns was being called (instead of manually modifying the cluster_url field). What am I missing? Why is the IP being evaluated here instead of replaced?

Environment

❯ kubectl version --output yaml
clientVersion:
  buildDate: "2022-09-21T14:33:49Z"
  compiler: gc
  gitCommit: 5835544ca568b757a8ecae5c153f317e5736700e
  gitTreeState: clean
  gitVersion: v1.25.2
  goVersion: go1.19.1
  major: "1"
  minor: "25"
  platform: linux/amd64
kustomizeVersion: v4.5.7
serverVersion:
  buildDate: "2022-10-24T20:32:54Z"
  compiler: gc
  gitCommit: b07006b2e59857b13fe5057a956e86225f0e82b7
  gitTreeState: clean
  gitVersion: v1.21.14-eks-fb459a0
  goVersion: go1.16.15
  major: "1"
  minor: 21+
  platform: linux/amd64

Configuration and features

Here's my change in our Cargo.toml:

kube = { version = "0.77.0", default-features = true, features = [ "derive", "runtime", "rustls-tls" ] }
❯ cargo tree -i k8s-openapi
k8s-openapi v0.16.0
├── agent v0.1.0 (/home/ubuntu/workspace/bottlerocket-os/bottlerocket-update-operator/agent)
├── apiserver v0.1.0 (/home/ubuntu/workspace/bottlerocket-os/bottlerocket-update-operator/apiserver)
│   └── agent v0.1.0 (/home/ubuntu/workspace/bottlerocket-os/bottlerocket-update-operator/agent)
├── controller v0.1.0 (/home/ubuntu/workspace/bottlerocket-os/bottlerocket-update-operator/controller)
├── integ v0.1.0 (/home/ubuntu/workspace/bottlerocket-os/bottlerocket-update-operator/integ)
├── kube v0.77.0
│   ├── agent v0.1.0 (/home/ubuntu/workspace/bottlerocket-os/bottlerocket-update-operator/agent)
│   ├── apiserver v0.1.0 (/home/ubuntu/workspace/bottlerocket-os/bottlerocket-update-operator/apiserver) (*)
│   ├── controller v0.1.0 (/home/ubuntu/workspace/bottlerocket-os/bottlerocket-update-operator/controller)
│   ├── integ v0.1.0 (/home/ubuntu/workspace/bottlerocket-os/bottlerocket-update-operator/integ)
│   └── models v0.1.0 (/home/ubuntu/workspace/bottlerocket-os/bottlerocket-update-operator/models)
│       ├── agent v0.1.0 (/home/ubuntu/workspace/bottlerocket-os/bottlerocket-update-operator/agent)
│       ├── apiserver v0.1.0 (/home/ubuntu/workspace/bottlerocket-os/bottlerocket-update-operator/apiserver) (*)
│       ├── controller v0.1.0 (/home/ubuntu/workspace/bottlerocket-os/bottlerocket-update-operator/controller)
│       └── integ v0.1.0 (/home/ubuntu/workspace/bottlerocket-os/bottlerocket-update-operator/integ)
│       [build-dependencies]
│       └── yamlgen v0.1.0 (/home/ubuntu/workspace/bottlerocket-os/bottlerocket-update-operator/yamlgen)
│       [dev-dependencies]
│       ├── apiserver v0.1.0 (/home/ubuntu/workspace/bottlerocket-os/bottlerocket-update-operator/apiserver) (*)
│       └── integ v0.1.0 (/home/ubuntu/workspace/bottlerocket-os/bottlerocket-update-operator/integ)
│   [build-dependencies]
│   └── yamlgen v0.1.0 (/home/ubuntu/workspace/bottlerocket-os/bottlerocket-update-operator/yamlgen)
├── kube-client v0.77.0
│   ├── kube v0.77.0 (*)
│   └── kube-runtime v0.77.0
│       └── kube v0.77.0 (*)
├── kube-core v0.77.0
│   ├── kube v0.77.0 (*)
│   └── kube-client v0.77.0 (*)
├── kube-runtime v0.77.0 (*)
└── models v0.1.0 (/home/ubuntu/workspace/bottlerocket-os/bottlerocket-update-operator/models) (*)
❯ cargo tree | grep kube
│   ├── kube v0.77.0
│   │   ├── kube-client v0.77.0
│   │   │   ├── kube-core v0.77.0
│   │   ├── kube-core v0.77.0 (*)
│   │   ├── kube-derive v0.77.0 (proc-macro)
│   │   └── kube-runtime v0.77.0
│   │       ├── kube-client v0.77.0 (*)
│   │   ├── kube v0.77.0 (*)
├── kube v0.77.0 (*)
├── kube v0.77.0 (*)
├── kube v0.77.0 (*)
├── kube v0.77.0 (*)
@jpmcb jpmcb added the bug Something isn't working label Dec 22, 2022
@jpmcb
Copy link
Contributor Author

jpmcb commented Dec 22, 2022

Also spinning up an IPv4 cluster to test and see if this behavior exists there as well ☁️

@jpmcb
Copy link
Contributor Author

jpmcb commented Dec 22, 2022

IPv4 cluster works fine, but I do notice that the IP is being used instead of the DNS service here as well:

infered config: Config { 
  cluster_url: https://10.100.0.1/,
  default_namespace: "brupop-bottlerocket-aws",
  root_cert: ...
}

@clux
Copy link
Member

clux commented Dec 23, 2022

Ah, that's annoying. I was hoping that the IP based setup would work most places with the tls_server_name set.

If you change the Config constructor to be Config::incluster_dns (or change cluster_url directly in Config) this should probably work for you (if it already worked on rustls) as this is what we did in 0.76.

@clux
Copy link
Member

clux commented Dec 23, 2022

Interesting that it's only for V6 though, wonder if there is something hyper-rustls related (they did some V6 changes AFAICR)

@jpmcb
Copy link
Contributor Author

jpmcb commented Dec 23, 2022

Thanks for the tip! This works:

let incluster_config = kube::Config::incluster_dns().context(agent_error::ConfigCreateSnafu)?;

let k8s_client =
    kube::client::Client::try_from(incluster_config).context(agent_error::ClientCreateSnafu)?;

@clux clux added the rustls rustls-tls related label Jan 7, 2023
@clux
Copy link
Member

clux commented Apr 3, 2023

We are planning on changing incluster_dns back to incluster_env as the default configuration setting for rustls as it looks as though the new IP support (#153) obviates the need for it. This should mean that you can use Client::try_default without specifying an override config again for rustls as the ip should now trivially match the cert.

See #1184.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working rustls rustls-tls related
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants