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

Support Devfile registries with self-signed or untrusted certificates by allowing to skip TLS checks #6635

Open
8 tasks
rm3l opened this issue Mar 6, 2023 · 5 comments
Labels
area/registry Issues or PRs related to Devfile registries kind/user-story An issue of user-story kind lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. priority/Medium Nice to have issue. Getting it done before priority changes would be great.

Comments

@rm3l
Copy link
Member

rm3l commented Mar 6, 2023

/kind user-story

User Story

As an odo user, I want odo to allow me to use a registry with a self-signed or even invalid TLS certificate, So that I can purposely use odo with my registry, regardless of the security issues.
This can be the case for example for local registries or even a registry behind some reverse proxy doing TLS termination using a self-signed certificate.

Acceptance Criteria

  • Remove --token flag from odo preference add registry
  • Ignore secure field in preferences file
  • Add --skip-tls-verify flag to odo preference add registry
  • Add new SkipTLSVerify field to preferences file
  • Add new skipTLSVerify field to odo preference view JSON and human-readable output
  • Remove Secure column from odo preference view human-readable output
  • Handle mapping of skipTLSVerify field from DevfileRegistriesList and ClusterDevfileRegistriesList custom resources
  • Pass skipTLSVerify when calling the registry

Unable to force-use Devfile registries with self-signed or untrusted certificates + confusing Secure property

What versions of software are you using?

Operating System:
Fedora 37

Output of odo version:
odo v3.7.0 (26c90d7)

How did you run odo exactly?

Let's say that I have a registry exposed using a self-signed or untrusted certificate, and I intentionally want to use it. It might be a local non-production registry for example.

$ odo preference add registry my-local-devfile-registry https://my-local-devfile-registry.172.17.0.1.nip.io
$ odo registry --devfile-registry my-local-devfile-registry --details --devfile go

Actual behavior

$ odo registry --devfile-registry my-local-devfile-registry --details --devfile go
 ⚠  Registry my-local-devfile-registry is not set up properly with error: 
Get "https://my-local-devfile-registry.172.17.0.1.nip.io": 
x509: certificate is valid for ingress.local, not https://my-local-devfile-registry.172.17.0.1.nip.io, 
please check the registry URL, and credential and remove add the registry again 
(refer to `odo preference add registry --help`)

 ✗  no deployable components found

Expected behavior

I think it is okay to enforce TLS checks by default, but users should be allowed to bypass those checks if needed, just like it is doable with curl --insecure or wget --no-check-certificate.
They might want to use a local registry or even a registry behind some reverse proxy doing TLS termination using a self-signed certificate.

I thought that would be the purpose of the Secure property on Registries, but it looks like this is set to True only if users pass a token when adding their registries. The token value seems to be stored and deleted, but never read, so not sure how this token is being used to interact with the corresponding Devfile registry.

Looking at the code, the Registry Library provides a SkipTLSVerify field, but it is always set to false by odo:

SkipTLSVerify: false,

So maybe we should provide a way for users to explicitly skip TLS checks when registering a registry.

Any logs, error output, etc?

Also, in #6622 (PR for #5128), we translated the skipTLSVerify field in the {Cluster,}DevfileRegistriesList Custom Resource into a Secure property, but it would probably make more sense to use a dedicated property for this.

@openshift-ci openshift-ci bot added the kind/bug Categorizes issue or PR as related to a bug. label Mar 6, 2023
@rm3l rm3l added the area/registry Issues or PRs related to Devfile registries label Mar 6, 2023
@github-actions github-actions bot added the needs-triage Indicates an issue or PR lacks a `triage/*` and requires one. label Mar 6, 2023
@rm3l rm3l added this to odo Project Mar 6, 2023
@rm3l rm3l changed the title Unable to force-use (skip TLS checks) Devfile registries with invalid certificates + confusing Secure property Unable to force-use Devfile registries with self-signed or untrusted certificates + confusing Secure property Mar 6, 2023
@kadel
Copy link
Member

kadel commented Mar 10, 2023

I thought that would be the purpose of the Secure property on Registries, but it looks like this is set to True only if users pass a token when adding their registries. The token value seems to be stored and deleted, but never read, so not sure how this token is being used to interact with the corresponding Devfile registry.

I think that Secure property is reminder from old "github" based implementation of devfile registries. This is why it is not used anymore. We should remove it.

+1 for new skipTLSVerify property.

@github-actions
Copy link
Contributor

A friendly reminder that this issue had no activity for 90 days. Stale issues will be closed after an additional 30 days of inactivity.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 10, 2023
@rm3l
Copy link
Member Author

rm3l commented Jun 12, 2023

/remove-lifecycle stale

@openshift-ci openshift-ci bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 12, 2023
@rm3l rm3l removed the needs-triage Indicates an issue or PR lacks a `triage/*` and requires one. label Jul 6, 2023
@rm3l rm3l changed the title Unable to force-use Devfile registries with self-signed or untrusted certificates + confusing Secure property Add new skipTLSVerify setting per registry and remove Secure property (was: Unable to force-use Devfile registries with self-signed or untrusted certificates + confusing Secure property) Aug 29, 2023
@rm3l rm3l added kind/user-story An issue of user-story kind and removed kind/bug Categorizes issue or PR as related to a bug. labels Aug 29, 2023
@rm3l rm3l changed the title Add new skipTLSVerify setting per registry and remove Secure property (was: Unable to force-use Devfile registries with self-signed or untrusted certificates + confusing Secure property) Add new skipTLSVerify property per Devfile Registry and remove Secure property Aug 29, 2023
Copy link
Contributor

A friendly reminder that this issue had no activity for 90 days. Stale issues will be closed after an additional 30 days of inactivity.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 28, 2023
@rm3l
Copy link
Member Author

rm3l commented Nov 28, 2023

/remove-lifecycle stale
/lifecycle frozen
/priority Medium
/retitle Support Devfile registries with self-signed or untrusted certificates by allowing to skip TLS checks

@openshift-ci openshift-ci bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 28, 2023
@openshift-ci openshift-ci bot changed the title Add new skipTLSVerify property per Devfile Registry and remove Secure property Support Devfile registries with self-signed or untrusted certificates by allowing to skip TLS checks Nov 28, 2023
@openshift-ci openshift-ci bot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. priority/Medium Nice to have issue. Getting it done before priority changes would be great. labels Nov 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/registry Issues or PRs related to Devfile registries kind/user-story An issue of user-story kind lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. priority/Medium Nice to have issue. Getting it done before priority changes would be great.
Projects
Status: No status
Development

No branches or pull requests

2 participants