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

[Proposal] design document for authentication rework #3417

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

apostasie
Copy link
Contributor

@apostasie apostasie commented Sep 6, 2024

Our current implementation of login suffers from structural issues manifest in a variety of ways (see #3072), and is vastly incomplete when it comes to supporting hosts.toml properly.

The core issue is that it predates these mechanisms, and has been architected with different requirements in mind that need to be re-evaluated today, especially in light of upcoming features like oauth device-flow.

First and foremost though, it lacks a lot of clarity about what is supposed to happen when, in a context where a lot of legacy features have complex interactions and effects:

  • the insecure-registry flag
  • the "default" behaviors for localhost
  • the use of the docker credentials store to retrieve credentials

... to a point that even seasoned contributors are confused as to whether we should allow a scheme to be passed in front of a registry url when we do login, or what exactly should happen in term of downgrading protocols or implied port and scheme.

Besides the bugs, reconciling these legacy features, the requirement to use docker credentials, and being able to actually use hosts.toml does require significant changes to our login and dockerconfigresolver code - aka: a full rewrite.

Some of these details are outlined in #3265 - though this is not the place for a proper reference - and some of the groundwork for this rewrite have been recently merged.

This document offers solutions, and describes precisely what happens when, and how things interact together. It is not a replacement for the hosts.toml slapstick spec, but rather a description of what we want to achieve with the upcoming rewrite.

While there is a lot of important details in there that should not be overlooked, the key parts of the proposal are the introduction of a new nerdctl login flag allowing to authenticate with endpoints, an experimental scheme for storing endpoint credentials, and clarification on the role and use of the --insecure-registry flag.

Given the extent of the changes to come - that will be very hard to break down into small pieces - I felt like it was better to start here and making sure we are aligned on the vision.

Cheers.

  • A.

@apostasie
Copy link
Contributor Author

Oh project checks... I just hate you.

@apostasie apostasie force-pushed the design-auth branch 2 times, most recently from 86b0ad3 to a8f336a Compare September 6, 2024 16:40
@apostasie apostasie force-pushed the design-auth branch 2 times, most recently from fd39c86 to 0d49eab Compare September 10, 2024 21:00
@AkihiroSuda AkihiroSuda added documentation Improvements or additions to documentation area/login authentification/ login labels Sep 10, 2024
Docker will not recognize this schema, hence will not wrongly send these
credentials when trying to log in into a known _endpoint_ as a registry.

The schema is: `nerdctl-experimental://namespace.example:123/?endpoint=myserver.example:456`
Copy link
Member

Choose a reason for hiding this comment

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

Where is this expected to be stored?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Inside the normal docker config location.

Copy link
Member

Choose a reason for hiding this comment

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

Does Docker allow it?

Copy link
Contributor Author

@apostasie apostasie Sep 18, 2024

Choose a reason for hiding this comment

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

Yes, it is possible to save and retrieve it.
The docker cli will just never use it (and we want it that way)

Our current implementation of login suffers from structural issues manifest in a variety of ways
(see containerd#3072), and is vastly incomplete when it comes to
supporting hosts.toml properly.
The core issue is that it predates these mechanisms, and has been architected with different requirements in mind
that need to be re-evaluated today, especially in light of upcoming features like oauth device-flow.

First and foremost though, it lacks a lot of clarity about what is supposed to happen when, in a context
where a lot of legacy features have complex interactions are effects:
- the insecure-registry flag
- the "default" behaviors for localhost
- the use of the docker credentials store to retrieve credentials
... to a point that even seasoned contributors are confused as to whether we should allow a scheme to be passed
in front of a registry url when we do login, or what exactly should happen in term of downgrading protocols or
implied port and schemes.

Besides the bugs, reconciling these legacy features, the requirement to use docker credentials,
and being able to actually use hosts.toml does require significant changes to our login and dockerconfigresolver
code.

Some of these details are outlined in containerd#3265 - though this is not the
place for a proper reference.

This document offers solutions, and describes precisely what happens when and how things work together.
It is not a replacement for the hosts.toml slapstick spec, but rather a description of what we want to achieve
with the upcoming rewrite.

While there is a lot of important details in there that should not be overlooked, the key parts of the proposal
are the introduction of a new flag allowing to log in into endpoints, an experimental scheme for storing
endpoint credentials, and clarification on the role and use of --insecure-registry flag.

Signed-off-by: apostasie <[email protected]>
@apostasie
Copy link
Contributor Author

@fahedouch would love your opinion here overall.

The take-away really is:

  • spelling out loud stuff that is tribal knowledge right now, and making it clear what we do wrt scheme, port, localhost behavior
  • proposing a way to deal with authentication when hosts.toml are involved (that is the new nerdctl flag and docker credentials scheme)

@apostasie
Copy link
Contributor Author

@AkihiroSuda once I get the testing/CI work done, is it fine with you that I move to implementation of this?

It might be simpler for people to discuss with actual code?

@apostasie apostasie mentioned this pull request Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/login authentification/ login documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants