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

Jump to bottom

x/crypto/ssh: add ServerConfig.NoClientAuthCallback #51994

Open
bradfitz opened this issue Mar 28, 2022 · 7 comments
Open

x/crypto/ssh: add ServerConfig.NoClientAuthCallback #51994

bradfitz opened this issue Mar 28, 2022 · 7 comments
Labels
Proposal Proposal-Accepted Proposal-FinalCommentPeriod
Milestone
Backlog

Comments

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Mar 28, 2022

Currently Go's SSH package doesn't permit a server to conditionally permit auth type "none" at runtime. There's a NoClientAuth bool flag to globally enable it, but you can't decide as a function of the ConnMetaData (username, IPs, etc) whether to permit it.

I propose adding a ServerConfig.NoClientAuthCallback auth hook, with a signature like the existing auth hooks:

        // NoClientAuthCallback, if non-nil, is called when a user
	// attempts to authenticate with auth method "none".
	// NoClientAuth must also be set to true for this be used, or
	// this func is unused.
	NoClientAuthCallback func(ConnMetadata) (*Permissions, error)

I sent https://go-review.googlesource.com/c/crypto/+/395314 which @rolandshoemaker approved, but this is the proposal for the API change.

/cc @maisem
@gopherbot gopherbot added this to the Proposal milestone Mar 28, 2022
@gopherbot
Copy link

@gopherbot gopherbot commented Mar 28, 2022

Change https://go.dev/cl/395314 mentions this issue: ssh: add ServerConfig.NoClientAuthCallback

@ianlancetaylor ianlancetaylor added this to Incoming in Proposals Mar 28, 2022
bradfitz added a commit to tailscale/golang-x-crypto that referenced this issue Mar 30, 2022
@bradfitz
ssh: add ServerConfig.NoClientAuthCallback
6211952 
It was possible to accept auth type "none" before, but not dynamically
at runtime as a function of the ConnMetadata like the other auth types'
callback hooks.

See golang/go#51994
and https://go-review.googlesource.com/c/crypto/+/395314

Change-Id: I83ea80901d4977d8f78523e3d1e16e0a7df5b172
(cherry picked from commit 4a431fab27b09acb1458fbb8709e12b2760e58a2)
@rsc
Copy link
Contributor

@rsc rsc commented Mar 30, 2022

/cc @FiloSottile

@rsc rsc moved this from Incoming to Active in Proposals Mar 30, 2022
@rsc
Copy link
Contributor

@rsc rsc commented Mar 30, 2022

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc
Copy link
Contributor

@rsc rsc commented Apr 6, 2022

/cc @golang/security

@rsc
Copy link
Contributor

@rsc rsc commented Apr 6, 2022

Does anyone object to adding this? It is a very small CL and Roland has already +2'ed it.

@rsc rsc moved this from Active to Likely Accept in Proposals Apr 13, 2022
@rsc
Copy link
Contributor

@rsc rsc commented Apr 13, 2022

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

@rsc rsc moved this from Likely Accept to Accepted in Proposals May 4, 2022
@rsc
Copy link
Contributor

@rsc rsc commented May 4, 2022

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

@rsc rsc changed the title proposal: x/crypto/ssh: add ServerConfig.NoClientAuthCallback x/crypto/ssh: add ServerConfig.NoClientAuthCallback May 4, 2022
@rsc rsc removed this from the Proposal milestone May 4, 2022
@rsc rsc added this to the Backlog milestone May 4, 2022
iQQBot added a commit to gitpod-io/golang-crypto that referenced this issue Jun 16, 2022
@iQQBot
ssh: add ServerConfig.NoClientAuthCallback
f7da13e 
It was possible to accept auth type "none" before, but not dynamically
at runtime as a function of the ConnMetadata like the other auth types'
callback hooks.

See golang/go#51994
and https://go-review.googlesource.com/c/crypto/+/395314

this commit is pick from golang@6211952
iQQBot pushed a commit to gitpod-io/golang-crypto that referenced this issue Jun 16, 2022
@bradfitz @iQQBot
ssh: add ServerConfig.NoClientAuthCallback
40ff35d 
It was possible to accept auth type "none" before, but not dynamically
at runtime as a function of the ConnMetadata like the other auth types'
callback hooks.

See golang/go#51994
and https://go-review.googlesource.com/c/crypto/+/395314

Change-Id: I83ea80901d4977d8f78523e3d1e16e0a7df5b172
(cherry picked from commit 4a431fab27b09acb1458fbb8709e12b2760e58a2)
iQQBot pushed a commit to gitpod-io/golang-crypto that referenced this issue Jun 16, 2022
@bradfitz @iQQBot
ssh: add ServerConfig.NoClientAuthCallback
9a7b788 
It was possible to accept auth type "none" before, but not dynamically
at runtime as a function of the ConnMetadata like the other auth types'
callback hooks.

See golang/go#51994
and https://go-review.googlesource.com/c/crypto/+/395314

Change-Id: I83ea80901d4977d8f78523e3d1e16e0a7df5b172
(cherry picked from commit 4a431fab27b09acb1458fbb8709e12b2760e58a2)
iQQBot pushed a commit to gitpod-io/golang-crypto that referenced this issue Jun 30, 2022
@bradfitz @iQQBot
ssh: add ServerConfig.NoClientAuthCallback
1e5ac8c 
It was possible to accept auth type "none" before, but not dynamically
at runtime as a function of the ConnMetadata like the other auth types'
callback hooks.

See golang/go#51994
and https://go-review.googlesource.com/c/crypto/+/395314

Change-Id: I83ea80901d4977d8f78523e3d1e16e0a7df5b172
(cherry picked from commit 4a431fab27b09acb1458fbb8709e12b2760e58a2)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Proposal Proposal-Accepted Proposal-FinalCommentPeriod
Projects
Proposals
Accepted
Milestone
Backlog
Development

No branches or pull requests
3 participants
@bradfitz @rsc @gopherbot