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

List operations does not work in external plugins #3873

Closed
LeSuisse opened this issue Jan 30, 2018 · 4 comments · Fixed by #3881
Closed

List operations does not work in external plugins #3873

LeSuisse opened this issue Jan 30, 2018 · 4 comments · Fixed by #3881
Assignees
Milestone

Comments

@LeSuisse
Copy link
Contributor

LeSuisse commented Jan 30, 2018

Environment:

  • Vault Version: Vault v0.9.3 ('5acd6a21d5a69ab49d0f7c0bf540123a9b2c696d')
  • Operating System/Architecture: Linux

Vault Config File:

plugin_directory = "/path/to/my/plugin/directory"

Startup Log Output:

==> Vault server configuration:

                     Cgo: disabled
         Cluster Address: https://127.0.0.1:8201
              Listener 1: tcp (addr: "127.0.0.1:8200", cluster address: "127.0.0.1:8201", tls: "disabled")
               Log Level: trace
                   Mlock: supported: true, enabled: false
        Redirect Address: http://127.0.0.1:8200
                 Storage: inmem
                 Version: Vault v0.9.3
             Version Sha: 5acd6a21d5a69ab49d0f7c0bf540123a9b2c696d

Expected Behavior:
It is possible to do a LIST request on a path that support it.

Actual Behavior:
Any LIST request fail with an internal server error and you got these logs from Vault:

2018/01/30 23:43:53.168510 [TRACE] plugin.HandleRequest: path=kv/ status=started type=mock-plugin transport=gRPC
2018/01/30 23:43:53.169698 [TRACE] plugin.HandleRequest: path=kv/ status=finished type=mock-plugin transport=gRPC err=<nil> took=1.185434ms
2018/01/30 23:44:01.599923 [TRACE] rollback: attempting rollback: path=auth/token/
2018/01/30 23:44:01.599933 [TRACE] rollback: attempting rollback: path=identity/
2018/01/30 23:44:01.599938 [TRACE] rollback: attempting rollback: path=secret/
2018/01/30 23:44:01.599940 [TRACE] rollback: attempting rollback: path=sys/
2018/01/30 23:44:01.599958 [TRACE] rollback: attempting rollback: path=cubbyhole/
2018/01/30 23:44:01.599945 [TRACE] rollback: attempting rollback: path=mock/
2018/01/30 23:44:01.600441 [TRACE] plugin.HandleRequest: path= status=started type=mock-plugin transport=gRPC
2018/01/30 23:44:01.601123 [TRACE] plugin.HandleRequest: path= status=finished type=mock-plugin transport=gRPC err=unsupported operation took=680.425µs

The issue seems to have been introduced with Vault 0.9.2/0.9.3.

Steps to Reproduce:
The issue can be reproduced with the mock-plugin.

  1. Build the mock-plugin
  2. Add the mock-plugin to the plugin catalog
  3. Mount the mock-plugin
  4. Add a value into the kv store
  5. Do a LIST request on mock/kv
@jefferai jefferai added this to the 0.9.4 milestone Jan 30, 2018
@everesio
Copy link

I have the same issue. In github.com/hashicorp/vault/logical/response_util.go RespondErrorCommon
[]string is expected but got []interface {}

Plugin code:

func (b *GsAuthBackend) pathRoleList(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
	roles, err := req.Storage.List(ctx, "role/")
	if err != nil {
		return nil, err
	}
	resp := logical.ListResponse(roles)

	keysRaw, _ := resp.Data["keys"]
	b.Logger().Info(fmt.Sprintf("pathRoleList: type=%v keysRaw=%v ", reflect.TypeOf(keysRaw), keysRaw))
	return resp, nil
}

2018/01/31 00:20:31.264548 [INFO ] plugin.vault-gs-plugin: pathRoleList: type=[]string keysRaw=[testrole]

Vault

keys, ok := keysRaw.([]string)
if !ok {
	return http.StatusInternalServerError, nil
}

2018/01/31 00:20:31.264712 [INFO ] github.com/hashicorp/vault/logical/response_util.go RespondErrorCommon: type=[]interface {} keysRaw=[testrole]

@briankassouf
Copy link
Contributor

This is a regression with the gRPC transport type. Working on a fix for this, but in the meantime you can force your plugin to fall back to netRPC transport by unsetting the VAULT_VERSION environment variable. This will cause the plugin to fail the version check that is required to choose gRPC as the transport. In go this can be done with os.Unsetenv("VAULT_VERSION"). This should be done before plugin.Serve is called. Here's the mock plugin's main function as an example:

func main() {
	apiClientMeta := &pluginutil.APIClientMeta{}
	flags := apiClientMeta.FlagSet()
	flags.Parse(os.Args[1:]) // Ignore command, strictly parse flags

	tlsConfig := apiClientMeta.GetTLSConfig()
	tlsProviderFunc := pluginutil.VaultPluginTLSProvider(tlsConfig)

	factoryFunc := mock.FactoryType(logical.TypeLogical)

	// Fail the version check, falling back to netRPC
	os.Unsetenv("VAULT_VERSION")

	err := plugin.Serve(&plugin.ServeOpts{
		BackendFactoryFunc: factoryFunc,
		TLSProviderFunc:    tlsProviderFunc,
	})
	if err != nil {
		log.Println(err)
		os.Exit(1)
	}
}

LeSuisse added a commit to LeSuisse/vault-gpg-plugin that referenced this issue Jan 31, 2018
Workaround an issue in Vault, see hashicorp/vault#3873
@briankassouf briankassouf self-assigned this Jan 31, 2018
@briankassouf
Copy link
Contributor

Just merged the fix for this, along with some tests to ensure we are hitting this code path. Thank you for reporting this and doing some investigation.

We also bumped the minimum version required for gRPC to be selected as the transport to 0.9.4, so Go plugins compiled with the patch (91dffed) for this should fall back to netRPC on vault servers < 0.9.4. Once version 0.9.4 comes out these gRPC issues will be resolved and gRPC transport will be selected as the default.

@LeSuisse @everesio and @cypherhat (I saw you reacted to the comment above) Once your dependencies are updated it should be safe to remove the os.Unsetenv("VAULT_VERSION") call.

@cypherhat
Copy link
Contributor

I added the os.Unsetenv("VAULT_VERSION") call and removed it a few hours later. Ain't OSS grand? My plugin is happy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants