Plugins system for authentication

[francoismichel]

This PR is a first attempt for providing a pluggable authentication system.
The end goal is to allow compile-time plugins to implement new authentication behaviours (e.g. SAML, passkeys, ...).

[EthanHeilman]

This code looks great, looking forward to using the plugin to add OpenPubkey. I did a small code review to help myself learn the code.

[EthanHeilman]

Does it make sense to debug log the err pubkeyBytes, err := os.ReadFile(fmt.Sprintf("%s.pub", m.Filename())) in case the error isn't just "file not found"? Maybe someone messed up the permissions and made it write-only or the system ran out of file descriptors?

In the case of ssh.ParseAuthorizedKey this means we found a matching .pub file and the file is somehow wrong. That probably means the user did something wrong in creating it. Might save a user sometime if you print a warning to the console.

[francoismichel]

Agree, thanks ! I think I'll put it at Debug level though since it is happening in an optional mechanism searching for the .pub counterpart of the key, which may simply be a different file for whatever reason.

[EthanHeilman]

foundAgentKey is always false, if !foundAgentKey { is always true.

[francoismichel]

Nice catch, it's an artefact from the non-plugin behaviour.
In fact we can just remove foundAgentKey.

[EthanHeilman]

Logging an error here, or returning an ok value, or catching the returned "" could be valuable as it looks like the server supports "RS256", "EdDSA", "ES256" but the client over supports "RS256", "EdDSA". Alternatively adding "ES256" to the client Alg()

I haven't tested this but reading the code it seems like if "ES256" is used on the client Alg() will return "", which will cause the server to throw "unsupported signature algorithm" in server_plugin.go. This could be confusing because it is the client which doesn't support "ES256".

ssh3/auth/plugins/pubkey_authentication/server/server_plugin.go

Lines 40 to 44 in 92aeb1b

	switch unvalidatedToken.Method.Alg() {
	case "RS256", "EdDSA", "ES256":
	return v.pubkey, nil
	}
	return nil, fmt.Errorf("unsupported signature algorithm '%s' for %T", unvalidatedToken.Method.Alg(), v)

[francoismichel]

Yeah, I don't like that part that much, it's just a bit forced due to implementing the jwt.SigningMethod interface and therefore I can't change the return type by adding an ok value.

What I can do is adding a constructor for the agentSigningMethod verifying the key type and returning an error if it's not supported.

[francoismichel]

Here's the current plugins interface. For people reading it, let me know your thoughts on the interface !
What's scaring me the most is people starting to develop plugins and then having to change the interface afterwards,
so it's best putting as much thought in it as possible before merging that.

Plugins interface

Developers can develop server-side and client-side authentication plugins.
The plugins have to be inserted at compile time, for instance by adding an import in cmd/ssh3/main.go or cmd/ssh3-server/main.go.
Plugins can register themselves by implementing an init() function in which they call the plugins.RegisterServerAuthPlugin() or plugins.RegisterClientAuthPlugin() function to register the authentication plugin.

Server-side plugins provide a ServerAuthPlugin function that takes a username and an ssh3 identity line in the authorized_identities/authorized_keys file and generates a RequestIdentityVerifier if the identity line correctly represents the authentication method defined by the plugin.
The RequestIdentityVerifier interface received an HTTP request and returns whether or not the request contains all the needed authentication material required to authenticate the user.

Client-side plugins provide a ClientAuthPlugin structure. This structure defines its own SSH3 options that can then be put in the SSH config (and optionally as a CLI arg). This structure should also hold a GetClientAuthMethodsFunc that provides all the suitable authentication methods to be tried against the server in the form of a slice of ClientAuthMethod. Every ClientAuthMethod will have the opportunity to prepare an HTTP request with authentication material to startup an SSH3 conversation. For instance, for pubkey authentication using the private key files on the filesystem, the GetClientAuthMethodsFunc can return a slice containing one ClientAuthMethod for each private key file it wants to try.

/////////////////////////////////////
//       Server auth plugins	   //
/////////////////////////////////////

// In ssh3, authorized_keys are replaced by authorized_identities where a use can specify classical
// public keys as well as other authentication and authorization methods such as OAUTH2 and SAML 2.0
type RequestIdentityVerifier interface {
	Verify(request *http.Request, base64ConversationID string) bool
}

// parses an AuthorizedIdentity line (`identityStr`). Returns a new Identity and a nil error if the
// line was successfully parsed. Returns a nil identity and a nil error if the line format is unknown
// to the plugin. Returns a non-nil error if any other error that is worth to be logged occurs.
//
// plugins are currently a single function so that they are completely stateless
type ServerAuthPlugin func(username string, identityStr string) (RequestIdentityVerifier, error)

/////////////////////////////////////
//       Client auth plugins	   //
/////////////////////////////////////


// returns all the suitable authentication methods to be tried against the server in the form
// of a slice of ClientAuthMethod. Every ClientAuthMethod will have the opportunity to prepare
// an HTTP request with authentication material to startup an SSH3 conversation. For instance,
// for pubkey authentication using the private key files on the filesystem, the
// GetClientAuthMethodsFunc can return a slice containing one ClientAuthMethod for
// each private key file it wants to try.
// if no SSH agent socket if found, sshAgent is nil
type GetClientAuthMethodsFunc func(request *http.Request, sshAgent agent.ExtendedAgent, clientConfig *client_config.Config, roundTripper *http3.RoundTripper) ([]ClientAuthMethod, error)

type ClientAuthMethod interface {
	// PrepareRequestForAuth updated the provided request with the needed headers
	// for authentication.
	// The method must not alter the request method (must always be CONNECT) nor the
	// Host/:origin, User-Agent or :path headers.
	// The agent is the connected SSH agent if it exists, nil otherwise
	// The provided roundTripper can be used to perform requests with the server to prepare
	// the authentication process.
	// username is the username to authenticate
	// conversation is the Conversation we want to establish
	PrepareRequestForAuth(request *http.Request, sshAgent agent.ExtendedAgent, roundTripper *http3.RoundTripper, username string, conversation *ssh3.Conversation) error
}

type ClientAuthPlugin struct {
	// A plugin can define one or more new SSH3 config options.
	// A new option is defined by providing a dedicated option parser.
	// The key in PluginOptions must be a unique name for each option
	// and must not conflict with any existing option
	// (good practice: "<your_repo_name>[-<option_name>]")
	PluginOptions map[client_config.OptionName]client_config.OptionParser

	PluginFunc GetClientAuthMethodsFunc
}

[EthanHeilman]

@francoismichel Whats the right way for a plugin to only be used if a certain CLI option is provided? For instance if I wanted to do -opk to flag openpubkey the way -oidc flags that a browser window should be opened for openid connect?

[francoismichel]

You'll have to devine a CLIOptionParser.
You can take inspiration from how the current plugins work.
For instance, the privkey_auth plugin defines a PrivkeyOptionParser.
The OptionParser and CLIOptionParser interfaces are missing from my post above so I'll add that.

But in a nutshell, you'll have to implement a CLIOptionParser:

// OptionParser allows parsing an client config option from a string.
type OptionParser interface {
	// returns the option config keyword
	// This keyword is used when parsing the SSH config.
	OptionConfigName() string

	// returns the Option represented by this list of config values
	// the values are all retrieved from the config.
	// values contain several entries if the keyword (see `OptionConfigName`) is
	// present several times in the config.
	Parse(values []string) (Option, error)
}

// CLIOptionParser defines a parser that can be hooked in the CLI flags
type CLIOptionParser interface {
	OptionParser
	FlagName() string
	Usage() string
	// returns whether it should be considered as a boolean flag (parsed using flag.Bool(), with no flag value)
	// or as a flag with a value. If IsBoolFlag returns true, the Parse() method will be called with "yes" or "no"
	// depending of the boolean value of the arg
	IsBoolFlag() bool
}

And then, you can register your parser in your plugin's init() function like it is done for privkey_auth:

func init() {
	plugin := auth.ClientAuthPlugin{
		PluginOptions: map[config.OptionName]config.OptionParser{PRIVKEY_OPTION_NAME: &PrivkeyOptionParser{}},
		PluginFunc:    privkeyPluginFunc,
	}
	plugins.RegisterClientAuthPlugin("privkey_auth", plugin)
}

Also, all ideas/suggestions to improve the current design are welcome.

[EthanHeilman]

@francoismichel Thanks, that's helpful.

I have been playing around with CLIOptionParser for the SSH3 OpenPubkey integration. The issue is that I want the ability to enable/disable the OpenPubkey plugin via a CLI flag. As you suggest, borrowing from privkey_auth, it seems like the best way current way to do this is to have the PluginFunc return an empty auth methods list if the OpenPubkey CLI enable flag isn't set.

Also, all ideas/suggestions to improve the current design are welcome.

What about having a more explicit way to enable/disable a plugin enforced as part of the plugin interface. I could see this being useful in situations where a user wants to skip a particular plugin entirely. Perhaps, the plugin isn't fedramp compliance or maybe that plugins PluginFunc is crashing.

func Dial(ctx context.Context, config *client_config.Config, qconn quic.EarlyConnection,
	roundTripper *http3.RoundTripper,
	sshAgent agent.ExtendedAgent) 
  ...
  // Use plugin interface to force plugin implementers to specify their plugins:
  // - Enable flag --use-oidc
  // - Disable flag --no-use-oidc
  // and if the plugin is enabled by default
  plugins := internal.GetClientAuthPlugins()
  enabledPlugins := []auth.ClientAuthPlugin{}
  for _, plugin := range plugins {
    if IsPluginDisabledByFlag(config, plugin.DisableFlag()) {
      continue
    } else if IsPluginEnabledByFlag(config, plugin.EnableFlag()) ||  Plugin.EnabledByDefault()  {
        enabledPlugins = append(enabledPlugins, plugin)
        continue
    }
  }
  // Only call the enabled plugins
  for _, plugin := range enabledPlugins {
    authMethods, err := plugin.PluginFunc(req, sshAgent, config, roundTripper)
    if err != nil {
	    return nil, err
    }
    for _, authMethod := range authMethods {
      err = authMethod.PrepareRequestForAuth(req, sshAgent, roundTripper, config.Username(), conv)
    ...

[francoismichel]

You mean, forcing plugins implementers to define CLI flags to entirely disable their plugins ?
What about a more general option like --disable-plugin <plugin_name> that could be repeated ? That way, implementers would not have to define such flags for their plugins themselves (and this could also avoid weird names).

Your opinion ?

[EthanHeilman]

You mean, forcing plugins implementers to define CLI flags to entirely disable their plugins ? What about a more general option like --disable-plugin <plugin_name> that could be repeated ?

I like that idea.

Maybe this as well so that someone can cherry pick the plugins they want to use
--require-plugin <plugin_name>

[francoismichel]

Merged ! We can look at the --disable and --require switches in another PR. :-)