Last active
October 4, 2024 00:26
-
-
Save cognifloyd/438137dfac880862da6fd1cee201d65b to your computer and use it in GitHub Desktop.
openbao login mfa plugin implementation notes
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
--- | |
notes: | |
- protobuf (not json) is used for the config stored in the barrier, not for IPC | |
- memdb only indexes key attributes, but full object (interace{}) is stored: | |
- loginMFAConfigTableSchema: id, namespace_id, type, name | |
- loginEnforcementTableSchema: id, namespace, nameAndNamespace | |
- mfa method - auth method: | |
# stays in core | |
TOTP: - | |
# extract these to external plugins | |
Okta: okta | |
Duo: - | |
PingID: - | |
- mfa plugin interface: | |
- new request types: | |
- HasMFAConfig(configId string) bool | |
- ValidateMFA(mfaFactors *MFAFactor, mConfig *mfa.Config, finalUsername, reqConnectionRemoteAddress string) | |
# configId = mConfig.GetExternalConfig().BackendConfigId | |
- plugin defined config management endpoints: LIST, GET, UPDATE, DELETE |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
--- | |
helper/identity/mfa/types.proto: # only used for storage in the barrier, not for IPC | |
message Config: | |
...: | |
oneof config: | |
- ExternalConfig external_config = 100; # add this with high number | |
message ExternalConfig: | |
- bool use_passcode = 1; | |
- string backend = 2; # okta, duo, ... or maybe mfa plugin uuid or accessor? | |
- string backend_config_id = 3; # to query the mfa backend for this config | |
# other bits to identify the plugin that implements this and has the full config | |
# Is there a way for protobuf to define a runtime field ignored for un/marshall? | |
IdentityStore: | |
# vault/identity_store.go | |
mfaPaths: | |
- noop / not supported by openbao response for removed mfa/method/{okta,duo,pingid}* | |
- Pattern: "mfa/method/external" + genericOptionalUUIDRegex("method_id") | |
Fields: | |
method_name: | |
method_id: | |
username_format: | |
use_passcode: | |
backend: | |
backend_config_id: | |
Operations: | |
logical.ReadOperation: &framework.PathOperation{Callback: i.handleMFAMethodReadExternal} | |
logical.UpdateOperation: &framework.PathOperation{Callback: i.handleMFAMethodUpdateExternal} | |
logical.DeleteOperation: &framework.PathOperation{Callback: i.handleMFAMethodDeleteExternal} | |
- Pattern: "mfa/method/external/?$" | |
Operations: | |
logical.ListOperation: &framework.PathOperation{Callback: i.handleMFAMethodListExternal} | |
# vault/login_mfa.go | |
handleMFAMethodListExternal: # LIST sys/mfa/method/external/? | |
- return i.handleMFAMethodList(..., mfaMethodTypeExternal) | |
handleMFAMethodReadExternal: # GET sys/mfa/method/external/:UUID | |
- return i.handleMFAMethodReadCommon(..., mfaMethodTypeExternal) | |
handleMFAMethodUpdateExternal: # UPDATE sys/mfa/method/external/:UUID | |
- return i.handleMFAMethodUpdateCommon(..., mfaMethodTypeExternal) | |
handleMFAMethodDeleteExternal: # DELETE sys/mfa/method/external/:UUID | |
- return i.handleMFAMethodUpdateCommon(..., mfaMethodTypeExternal) | |
handleMFAMethodUpdateCommon: # only used for UPDATE sys/mfa/method/external/:UUID | |
- | | |
switch methodType | |
... | |
case mfaMethodTypeExternal | |
i.mfaBackend.parseExternalConfig(mConfig, d) | |
- i.mfaBackend.putMFAConfigByID(..., mConfig) | |
- i.mfaBackend.MemDBUpsertMFAConfig(..., mConfig) | |
MFABackend: | |
parseExternalConfig: # (b *MFABackend) receiver, not a plain func like the other parse*Config funcs | |
- externalConfig = &mfa.ExternalConfig{} # need to add this to protobuf | |
- externalConfig.UsePasscode = d.Get("use_passcode").(bool) | |
- externalConfig.Backend = d.Get("backend").(string) | |
- externalConfig.BackendConfigID = d.Get("backend_config_id").(string) | |
#- externalConfig.Config = ??? # load the config from the external plugin? | |
# If so, this ends up in memdb, but not persisted in barrier. | |
# If not, at least validate backend_config_id exists w/ backend | |
- mConfig.Config = &mfa.Config_ExternalConfig{ExternalConfig: externalConfig} | |
LoginMFABackend: | |
mfaConfigToMap: | |
- | | |
switch mConfig.Config.(type) | |
case *mfa.Config_ExternalConfig | |
externalConfig := mConfig.GetExternalConfig() | |
respData["use_passcode"] = externalConfig.UsePasscode | |
respData["backend"] = externalConfig.Backend | |
respData["backend_config_id"] = externalConfig.BackendConfigID | |
#respData["config"] = ??? # dump the config from the external plugin? | |
# If so, this gets included in LIST sys/mfa/method[/external]/? and READ sys/mfa/method[/external]/:UUID | |
Core: | |
validateLoginMFAInternal: | |
- drop parseMfaFactors in favor of passing raw logical.MFACreds map to the MFA backend | |
- | | |
var finalUsername string | |
switch mConfig.Type | |
case mfaMethodTypeDuo, mfaMethodTypeOkta, mfaMethodTypePingID, mfaMethodTypeExternal # add External | |
... # finalUsername calc is the same | |
- | | |
switch mConfig.Type | |
case mfaMethodTypeExternal # add External | |
return c.validateExternal(ctx, mfaCreds, mConfig, finalUsername, reqConnectionRemoteAddress) | |
validate*: | |
- use logical.MFACreds instead of mfaFactors | |
- mfaFactors = parseMfaFactors(mfaCreds) | |
- ... | |
validateExternal: | |
- externalConfig := mConfig.GetExternalConfig() | |
- backend = externalConfig.Backend | |
- actual_backend = lookupMFABackend(backend) | |
- actual_backend.validate(..., externalConfig) # request validation from external backend |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
--- | |
# New Plugin Type | |
plugin_types: | |
Add consts: | |
consts: | |
- PluginTypeMFA = "mfa" | |
files: | |
- sdk/helper/consts/plugin_types.go | |
- api/plugin_types.go | |
Add sdk package using ONE OF (OR): | |
- sdk/mfa/mfaplugin: new GRPC interface # like sdk/database/dbplugin | |
- extend: | |
sdk.framework.Backend struct: | |
Paths: config CRUD | |
HasMFAConfigFunc: # new | |
ParseMFAFactorsFunc: # new | |
ValidateMFAFunc: # new | |
Backend.HandleRequest: | |
- | | |
switch req.Operation | |
case ... | |
... | |
case logical.HasMFAConfigOperation | |
return b.handleHasMFAConfig(ctx, req) | |
case logical.ValidateMFAOperation | |
return b.handleValidateMFA(ctx, req) | |
Backend.handleHasMFAConfig: # new (ctx, req) -> (bool, error) | |
- configId = req.Data["backend_config_id"].(string) | |
- | | |
if b.HasMFAConfigFunc != nil | |
return b.HasMFAConfigFunc(ctx, configId) | |
return nil, fmt.Errorf("mfa is unsupported by this backend") | |
Backend.handleValidateMFA: # new | |
- mfaCreds = req.MFACreds | |
- | | |
mfaFactor := &MFAFactor{} | |
if b.ParseMFAFactorsFunc != nil | |
err = b.ParseMFAFactorsFunc(mfaCreds, mfaFactor) | |
- configId = req.Data["backend_config_id"].(string) | |
- usesPasscode = req.Data["uses_passcode"].(bool) | |
- username = req.Data["username"].(string) | |
- reqConnectionRemoteAddress = req.Data["req_connection_remote_address"] | |
#- reqConnectionRemoteAddress = req.Connection.RemoteAddress | |
- | | |
if b.ValidateMFAFunc != nil | |
return b.ValidateMFAFunc(ctx, mfaFactor, configId, usesPasscode, username, reqConnectionRemoteAddress) | |
return nil, fmt.Errorf("mfa is unsupported by this backend") | |
sdk/logical/request.go: | |
- add const: | |
HasMFAConfigOperation = "has-mfa-config" | |
ValidateMFAOperation = "validate-mfa" | |
- | | |
func HasMFAConfigRequest(configId string) *Request { | |
return &Request{ | |
Operation: HasMFAConfigOperation, | |
Data: map[string]interface{}{ | |
"backend_config_id": configId, | |
}, | |
} | |
} | |
- | | |
func ValidateMFARequest(data map[string]interface{}, mfaCreds MFACreds) *Request { | |
# or (..., req *Request) | |
#req = req.Clone() | |
#req.Operation = ValidateMFAOperation | |
#req.Data = data # basically mConfig | |
return &Request{ | |
Operation: ValidateMFAOperation, | |
Data: data, # basically mConfig | |
MFACreds: mfaCreds, | |
} | |
} | |
#sdk.logical.Backend interface: # maybe not used? | |
# # if used: | |
# TypeMFA BackendType = 3 | |
# BackendType = "mfa" | |
vault/acl.go: | |
# maybe use permissions.MFAMethods to determine this? | |
- add logical.HasMFAConfigOperation | |
- add logical.ValidateMFAOperation |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
\o hey @cognifloyd!
Generally, I think I'm confused about the high level flow, though I will admit I'm not the most familiar with MFA right now. :-)
If I were to start over from a higher level, I think it'd be nice if the plugin owns its own configuration internally and handles the mapping. They can do memdb if they want, or they can use transactions with dual entries or something similar. In particular, we need fast lookup of
name->id
or id-based query, but that's about it. We could probably just run a separate instance of each MFA plugin per namespace and that simplifies storage a ton (to avoid the NamespaceID correlation that also occurs today).(I think you're doing something similar, but that config seems to spill over into the global scope, so I think you're intending to index it into the global memdb... I'd avoid that, tbh and only leave it for legacy usage until we deprecate the existing MFA in the future.)
In particular, if we turn MFAs into a top-level mounts under
identity/mfa/method/:path
(which can be namespaced in the future because I don't think we want cross-namespace MFAs and only want parent->child simple inheritance... ...and disallowing the current mount names)... I think we can play with the definition ofmfa_name
andmfa_id
for the new external mounts (but not for the existing mounts).In particular, I think we could prefix both with
:path
or:accessor
components, which lets us uniquely identify the MFA mount without needing global uniqueness.This then simplifies the definition substantially: MFA plugins can expose custom routes to manage itself, which roughly conforms to what we have today for plugins, which are also implicitly namespaced. Then we can define a simpler extension to
Backend
for MFA plugins which extends the Auth/Secret mount for MFA verification:Because we prefixed
methodID
with:path
or:accessor
, we can correctly route MFA requests to the correct plugin... We can mount multiple plugins of different types (which, I think, cannot be done currently in this proposal asExternal
is the type)... and we can reuse a lot of the heavy lifting of existing plugin architecture.Thoughts?