Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Custom plugins mounted over built in plugins can lead to a bricked installation #11687

Closed
corybolar opened this issue May 22, 2021 · 2 comments · Fixed by #11690
Closed

Custom plugins mounted over built in plugins can lead to a bricked installation #11687

corybolar opened this issue May 22, 2021 · 2 comments · Fixed by #11690

Comments

@corybolar
Copy link
Contributor

Describe the bug
If a custom plugin is mounted over a built in plugin then removed or otherwise altered, vault will refuse to unseal.

To Reproduce
Assuming a working plugin at $PLUGINDIR and a binary at $PLUGINBIN. In this example vault-plugin-auth-jwt

PLUGINDIR=/vault/plugins
PLUGINBIN=vault-plugin-auth-jwt
# Register
vault plugin register -command $PLUGINBIN -sha256 $(openssl dgst -sha256 $PLUGINDIR/$PLUGINBIN | cut -d '=' -f2 | tr -d ' ') oidc
# Verify its our plugin and not builtin
vault read /sys/plugins/catalog/auth/oidc
# Mount it
vault write auth/oidc/config oidc_discovery_url="..." oidc_client_id="..." oidc_client_secret="..." default_role="default"

# stop server however applicable (docker, service, CTRL-C)
mv $PLUGINDIR/$PLUGINBIN $PLUGINDIR/$PLUGINBIN.old

# start vault server however applicable
vault operator unseal ...

Logs:

[ERROR] core: failed to create credential entry: path=oidc/ error="error verifying checksum: open /vault/plugins/vault-plugin-auth-jwt: no such file or directory"
[INFO]  core: pre-seal teardown starting
[INFO]  core: pre-seal teardown complete
[ERROR] core: post-unseal setup failed: error="failed to setup auth table"

Seemingly no recovery from this point. Vault cannot be unsealed to remove the plugin or in the case of a sha256 mismatch, reregister it. Very similar to #3602

Expected behavior
Vault starts albeit in a degraded state but from which it can be repaired by removing or reregistering the plugin.

Environment:

  • Vault Server Version (retrieve with vault status): 1.7.1
  • Vault CLI Version (retrieve with vault version): v1.7.1
  • Server Operating System/Architecture: docker amd64 https://hub.docker.com/_/vault

Vault server configuration file(s):

storage "file" {
  path = "/mnt/vault-data"
}

listener "tcp" {
  address         = "0.0.0.0:80"
  tls_disable     = "true"
}

plugin_directory = "/vault/plugins"
disable_mlock   = true
api_addr = "http://example.com:8080"

disable_clustering  = true

log_level           = "Trace"

default_lease_ttl   = "1h"
max_lease_ttl       = "1h"

plugin_directory    = "/vault/plugins"
ui                  = true

Additional context
Having now reviewed the relevant code sections, this might be caused by the logic of:

if !c.builtinRegistry.Contains(entry.Type, consts.PluginTypeCredential) {

As I read this, the intended purpose is to continue on with mounting if the plugin is not a builtin. Unfortunately the lookup here only matches the plugin type to the builtin registry rather than actually checking to see if it's a built in or not. That information is available, just not being used here. A possible solution would be to query the plugin catalog instead. This builds but I have not yet tested it resolves my issue as a complete build of vault with the ui takes a lifetime on my machine.

diff --git a/vault/auth.go b/vault/auth.go
index 5b5394716..39c4b19bc 100644
--- a/vault/auth.go
+++ b/vault/auth.go
@@ -685,7 +685,7 @@ func (c *Core) setupCredentials(ctx context.Context) error {
                backend, err = c.newCredentialBackend(ctx, entry, sysView, view)
                if err != nil {
                        c.logger.Error("failed to create credential entry", "path", entry.Path, "error", err)
-                       if !c.builtinRegistry.Contains(entry.Type, consts.PluginTypeCredential) {
+                       if plug, plugerr := c.pluginCatalog.Get(ctx, entry.Type, consts.PluginTypeCredential); plugerr == nil && !plug.Builtin {
                                // If we encounter an error instantiating the backend due to an error,
                                // skip backend initialization but register the entry to the mount table
                                // to preserve storage and path.

The need to mount this particular custom plugin or top of the existing builtin is to retain the ui configuration and authentication listing visibility of the original oidc plugin. Mounting this with an alternate path and preserving those features does not seem possible.

@corybolar
Copy link
Contributor Author

A more self-contained reproducer is below.

#!/bin/bash
set -xeo pipefail

DIMAGE="vault:1.7.1"
PLUGINDIR="$(pwd)/plugins"
PLUGINBIN="vault-plugin-auth-jwt"
export VAULT_ADDR="http://127.0.0.1:8200"

cat <<- EOF > ./config.hcl
storage "file" {
  path = "/vault/file"
}

listener "tcp" {
  address = "0.0.0.0:8200"
  tls_disable = "true"
}

disable_mlock   = true
api_addr = "http://127.0.0.1:8200"

disable_clustering  = true

log_level           = "Trace"

default_lease_ttl   = "1h"
max_lease_ttl       = "1h"

plugin_directory    = "/vault/plugins"
ui                  = false
EOF

stop_cont () { docker stop vault-dev; docker rm vault-dev; }
rename_plugin () { mv "$PLUGINDIR/$PLUGINBIN.old" "$PLUGINDIR/$PLUGINBIN" || true; }
cleanup () { stop_cont; }

docker run -d --name vault-dev \
    -v "$PLUGINDIR:/vault/plugins" \
    -v "$(pwd)/config.hcl:/vault/config/config.hcl" \
    -p "8200:8200" \
    "$DIMAGE" server
trap cleanup EXIT SIGTERM SIGKILL
while [[ "$(curl -s -o /dev/null -w '%{http_code}' http://127.0.0.1:8200/v1/sys/seal-status)" != "200" ]]; do sleep 1; done
docker logs -f vault-dev &

KEYS=$(vault operator init -key-shares 1 -key-threshold 1 -format json)
VAULT_TOKEN=$(echo "$KEYS" | jq -r '.root_token')
VAULT_UNSEAL_KEY=$(echo "$KEYS" | jq -r '.unseal_keys_b64[]')
vault operator unseal "$VAULT_UNSEAL_KEY"
export VAULT_TOKEN

vault plugin register -command "$PLUGINBIN" -sha256 $(openssl dgst -sha256 "$PLUGINDIR/$PLUGINBIN" | cut -d '=' -f2 | tr -d ' ') oidc
vault read /sys/plugins/catalog/auth/oidc
vault auth enable oidc

docker stop vault-dev
docker wait vault-dev
mv "$PLUGINDIR/$PLUGINBIN" "$PLUGINDIR/$PLUGINBIN.old"
cleanup () { stop_cont; rename_plugin; }

docker start vault-dev
while [[ "$(curl -s -o /dev/null -w '%{http_code}' http://127.0.0.1:8200/v1/sys/seal-status)" != "200" ]]; do sleep 1; done
docker logs -f vault-dev &
vault operator unseal "$VAULT_UNSEAL_KEY"

@corybolar
Copy link
Contributor Author

corybolar commented May 22, 2021

I can confirm the fix outlined in my original post resolves this issue. Logs from the fix:

2021-05-22T14:12:11.298Z [ERROR] core: failed to create credential entry: path=oidc/ error="error verifying checksum: open /vault/plugins/vault-plugin-auth-jwt: no such file or directory"
2021-05-22T14:12:11.298Z [WARN]  core: skipping plugin-based credential entry: path=oidc/
2021-05-22T14:12:11.298Z [INFO]  core: successfully enabled credential backend: type=oidc path=oidc/

corybolar added a commit to corybolar/vault that referenced this issue May 22, 2021
Checking if a plugin is a builtin by comparing it's type to those in the builtin
registry allows for a custom plugin loaded with the same name to be considered a
builtin during error handling of the mounting process.  This can cause the vault
installation to brick itself because it cannot be unsealed to register a new
sha256 or file path for a previously loaded custom plugin.  Improve this logic
by checking the plugin catalog rather than the builtin registry.

Fixes hashicorp#11687
sgmiller pushed a commit that referenced this issue May 27, 2021
Checking if a plugin is a builtin by comparing it's type to those in the builtin
registry allows for a custom plugin loaded with the same name to be considered a
builtin during error handling of the mounting process.  This can cause the vault
installation to brick itself because it cannot be unsealed to register a new
sha256 or file path for a previously loaded custom plugin.  Improve this logic
by checking the plugin catalog rather than the builtin registry.

Fixes #11687
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant