Skip to content

Commit 58b88b8

Browse files
fairclothjmfairclothjm
authored
Revert "AutoMTLS for secrets/auth plugins (#15671)" (#16377)
This reverts commit 39bcd5c.
1 parent a5f6b1c commit 58b88b8

14 files changed

+428
-729
lines changed

‎api/plugin_helpers.go

+1-5
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,6 @@ import (
1717
)
1818

1919
var (
20-
// PluginAutoMTLSEnv ensures AutoMTLS is used. This overrides setting a
21-
// TLSProviderFunc for a plugin.
22-
PluginAutoMTLSEnv = "VAULT_PLUGIN_AUTOMTLS"
23-
2420
// PluginMetadataModeEnv is an ENV name used to disable TLS communication
2521
// to bootstrap mounting plugins.
2622
PluginMetadataModeEnv = "VAULT_PLUGIN_METADATA_MODE"
@@ -124,7 +120,7 @@ func VaultPluginTLSProvider(apiTLSConfig *TLSConfig) func() (*tls.Config, error)
124120
// VaultPluginTLSProviderContext is run inside a plugin and retrieves the response
125121
// wrapped TLS certificate from vault. It returns a configured TLS Config.
126122
func VaultPluginTLSProviderContext(ctx context.Context, apiTLSConfig *TLSConfig) func() (*tls.Config, error) {
127-
if os.Getenv(PluginAutoMTLSEnv) == "true" || os.Getenv(PluginMetadataModeEnv) == "true" {
123+
if os.Getenv(PluginMetadataModeEnv) == "true" {
128124
return nil
129125
}
130126

‎builtin/plugin/backend.go

+6-23
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99

1010
log "github.com/hashicorp/go-hclog"
1111

12-
"github.com/hashicorp/go-multierror"
1312
uuid "github.com/hashicorp/go-uuid"
1413
"github.com/hashicorp/vault/sdk/framework"
1514
"github.com/hashicorp/vault/sdk/helper/consts"
@@ -52,32 +51,17 @@ func Backend(ctx context.Context, conf *logical.BackendConfig) (*PluginBackend,
5251

5352
sys := conf.System
5453

55-
merr := &multierror.Error{}
56-
// NewBackend with isMetadataMode set to false
57-
raw, err := bplugin.NewBackend(ctx, name, pluginType, sys, conf, false, true)
54+
// NewBackend with isMetadataMode set to true
55+
raw, err := bplugin.NewBackend(ctx, name, pluginType, sys, conf, true)
5856
if err != nil {
59-
merr = multierror.Append(merr, err)
60-
// NewBackend with isMetadataMode set to true
61-
raw, err = bplugin.NewBackend(ctx, name, pluginType, sys, conf, true, false)
62-
if err != nil {
63-
merr = multierror.Append(merr, err)
64-
return nil, merr
65-
}
66-
} else {
67-
b.Backend = raw
68-
b.config = conf
69-
b.loaded = true
70-
b.autoMTLSSupported = true
71-
72-
return &b, nil
57+
return nil, err
7358
}
74-
75-
// Setup the backend so we can inspect the SpecialPaths and Type
7659
err = raw.Setup(ctx, conf)
7760
if err != nil {
7861
raw.Cleanup(ctx)
7962
return nil, err
8063
}
64+
// Get SpecialPaths and BackendType
8165
paths := raw.SpecialPaths()
8266
btype := raw.Type()
8367

@@ -101,8 +85,7 @@ type PluginBackend struct {
10185
Backend logical.Backend
10286
sync.RWMutex
10387

104-
autoMTLSSupported bool
105-
config *logical.BackendConfig
88+
config *logical.BackendConfig
10689

10790
// Used to detect if we already reloaded
10891
canary string
@@ -122,7 +105,7 @@ func (b *PluginBackend) startBackend(ctx context.Context, storage logical.Storag
122105
// Ensure proper cleanup of the backend (i.e. call client.Kill())
123106
b.Backend.Cleanup(ctx)
124107

125-
nb, err := bplugin.NewBackend(ctx, pluginName, pluginType, b.config.System, b.config, false, b.autoMTLSSupported)
108+
nb, err := bplugin.NewBackend(ctx, pluginName, pluginType, b.config.System, b.config, false)
126109
if err != nil {
127110
return err
128111
}

‎builtin/plugin/backend_lazyLoad_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ func testLazyLoad(t *testing.T, methodWrapper func() error) *PluginBackend {
5959
}
6060

6161
// this is a dummy plugin that hasn't really been loaded yet
62-
orig, err := plugin.NewBackend(ctx, "test-plugin", consts.PluginTypeSecrets, sysView, config, true, false)
62+
orig, err := plugin.NewBackend(ctx, "test-plugin", consts.PluginTypeSecrets, sysView, config, true)
6363
if err != nil {
6464
t.Fatal(err)
6565
}

‎changelog/15671.txt

-3
This file was deleted.

‎sdk/helper/pluginutil/env.go

-4
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,6 @@ import (
88
)
99

1010
var (
11-
// PluginAutoMTLSEnv is used to ensure AutoMTLS is used. This will override
12-
// setting a TLSProviderFunc for a plugin.
13-
PluginAutoMTLSEnv = "VAULT_PLUGIN_AUTOMTLS"
14-
1511
// PluginMlockEnabled is the ENV name used to pass the configuration for
1612
// enabling mlock
1713
PluginMlockEnabled = "VAULT_PLUGIN_MLOCK_ENABLED"

‎sdk/helper/pluginutil/multiplexing.go

-2
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@ import (
99
status "google.golang.org/grpc/status"
1010
)
1111

12-
const MultiplexingCtxKey string = "multiplex_id"
13-
1412
type PluginMultiplexingServerImpl struct {
1513
UnimplementedPluginMultiplexingServer
1614

‎sdk/helper/pluginutil/run_config.go

+5-7
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ type PluginClientConfig struct {
2222
IsMetadataMode bool
2323
AutoMTLS bool
2424
MLock bool
25-
Wrapper RunnerUtil
2625
}
2726

2827
type runConfig struct {
@@ -34,6 +33,8 @@ type runConfig struct {
3433
// Initialized with what's in PluginRunner.Env, but can be added to
3534
env []string
3635

36+
wrapper RunnerUtil
37+
3738
PluginClientConfig
3839
}
3940

@@ -42,7 +43,7 @@ func (rc runConfig) makeConfig(ctx context.Context) (*plugin.ClientConfig, error
4243
cmd.Env = append(cmd.Env, rc.env...)
4344

4445
// Add the mlock setting to the ENV of the plugin
45-
if rc.MLock || (rc.Wrapper != nil && rc.Wrapper.MlockEnabled()) {
46+
if rc.MLock || (rc.wrapper != nil && rc.wrapper.MlockEnabled()) {
4647
cmd.Env = append(cmd.Env, fmt.Sprintf("%s=%s", PluginMlockEnabled, "true"))
4748
}
4849
cmd.Env = append(cmd.Env, fmt.Sprintf("%s=%s", PluginVaultVersionEnv, version.GetVersion().Version))
@@ -53,9 +54,6 @@ func (rc runConfig) makeConfig(ctx context.Context) (*plugin.ClientConfig, error
5354
metadataEnv := fmt.Sprintf("%s=%t", PluginMetadataModeEnv, rc.IsMetadataMode)
5455
cmd.Env = append(cmd.Env, metadataEnv)
5556

56-
automtlsEnv := fmt.Sprintf("%s=%t", PluginAutoMTLSEnv, rc.AutoMTLS)
57-
cmd.Env = append(cmd.Env, automtlsEnv)
58-
5957
var clientTLSConfig *tls.Config
6058
if !rc.AutoMTLS && !rc.IsMetadataMode {
6159
// Get a CA TLS Certificate
@@ -72,7 +70,7 @@ func (rc runConfig) makeConfig(ctx context.Context) (*plugin.ClientConfig, error
7270

7371
// Use CA to sign a server cert and wrap the values in a response wrapped
7472
// token.
75-
wrapToken, err := wrapServerConfig(ctx, rc.Wrapper, certBytes, key)
73+
wrapToken, err := wrapServerConfig(ctx, rc.wrapper, certBytes, key)
7674
if err != nil {
7775
return nil, err
7876
}
@@ -122,7 +120,7 @@ func Env(env ...string) RunOpt {
122120

123121
func Runner(wrapper RunnerUtil) RunOpt {
124122
return func(rc *runConfig) {
125-
rc.Wrapper = wrapper
123+
rc.wrapper = wrapper
126124
}
127125
}
128126

‎sdk/helper/pluginutil/run_config_test.go

+5-7
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"fmt"
66
"os/exec"
7+
"reflect"
78
"testing"
89
"time"
910

@@ -13,7 +14,6 @@ import (
1314
"github.com/hashicorp/go-plugin"
1415
"github.com/hashicorp/vault/sdk/helper/wrapping"
1516
"github.com/stretchr/testify/mock"
16-
"github.com/stretchr/testify/require"
1717
)
1818

1919
func TestMakeConfig(t *testing.T) {
@@ -78,7 +78,6 @@ func TestMakeConfig(t *testing.T) {
7878
"initial=true",
7979
fmt.Sprintf("%s=%s", PluginVaultVersionEnv, version.GetVersion().Version),
8080
fmt.Sprintf("%s=%t", PluginMetadataModeEnv, true),
81-
fmt.Sprintf("%s=%t", PluginAutoMTLSEnv, false),
8281
},
8382
),
8483
SecureConfig: &plugin.SecureConfig{
@@ -144,7 +143,6 @@ func TestMakeConfig(t *testing.T) {
144143
fmt.Sprintf("%s=%t", PluginMlockEnabled, true),
145144
fmt.Sprintf("%s=%s", PluginVaultVersionEnv, version.GetVersion().Version),
146145
fmt.Sprintf("%s=%t", PluginMetadataModeEnv, false),
147-
fmt.Sprintf("%s=%t", PluginAutoMTLSEnv, false),
148146
fmt.Sprintf("%s=%s", PluginUnwrapTokenEnv, "testtoken"),
149147
},
150148
),
@@ -207,7 +205,6 @@ func TestMakeConfig(t *testing.T) {
207205
"initial=true",
208206
fmt.Sprintf("%s=%s", PluginVaultVersionEnv, version.GetVersion().Version),
209207
fmt.Sprintf("%s=%t", PluginMetadataModeEnv, true),
210-
fmt.Sprintf("%s=%t", PluginAutoMTLSEnv, true),
211208
},
212209
),
213210
SecureConfig: &plugin.SecureConfig{
@@ -269,7 +266,6 @@ func TestMakeConfig(t *testing.T) {
269266
"initial=true",
270267
fmt.Sprintf("%s=%s", PluginVaultVersionEnv, version.GetVersion().Version),
271268
fmt.Sprintf("%s=%t", PluginMetadataModeEnv, false),
272-
fmt.Sprintf("%s=%t", PluginAutoMTLSEnv, true),
273269
},
274270
),
275271
SecureConfig: &plugin.SecureConfig{
@@ -294,7 +290,7 @@ func TestMakeConfig(t *testing.T) {
294290
Return(test.responseWrapInfo, test.responseWrapInfoErr)
295291
mockWrapper.On("MlockEnabled").
296292
Return(test.mlockEnabled)
297-
test.rc.Wrapper = mockWrapper
293+
test.rc.wrapper = mockWrapper
298294
defer mockWrapper.AssertNumberOfCalls(t, "ResponseWrapData", test.responseWrapInfoTimes)
299295
defer mockWrapper.AssertNumberOfCalls(t, "MlockEnabled", test.mlockEnabledTimes)
300296

@@ -322,7 +318,9 @@ func TestMakeConfig(t *testing.T) {
322318
}
323319
config.TLSConfig = nil
324320

325-
require.Equal(t, config, test.expectedConfig)
321+
if !reflect.DeepEqual(config, test.expectedConfig) {
322+
t.Fatalf("Actual config: %#v\nExpected config: %#v", config, test.expectedConfig)
323+
}
326324
})
327325
}
328326
}

‎sdk/helper/pluginutil/runner.go

+2
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ type PluginClient interface {
3838
plugin.ClientProtocol
3939
}
4040

41+
const MultiplexingCtxKey string = "multiplex_id"
42+
4143
// PluginRunner defines the metadata needed to run a plugin securely with
4244
// go-plugin.
4345
type PluginRunner struct {

‎sdk/plugin/backend.go

+9-11
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,9 @@ var (
2020
// GRPCBackendPlugin is the plugin.Plugin implementation that only supports GRPC
2121
// transport
2222
type GRPCBackendPlugin struct {
23-
Factory logical.Factory
24-
MetadataMode bool
25-
AutoMTLSSupported bool
26-
Logger log.Logger
23+
Factory logical.Factory
24+
MetadataMode bool
25+
Logger log.Logger
2726

2827
// Embeding this will disable the netRPC protocol
2928
plugin.NetRPCUnsupportedPlugin
@@ -42,13 +41,12 @@ func (b GRPCBackendPlugin) GRPCServer(broker *plugin.GRPCBroker, s *grpc.Server)
4241

4342
func (b *GRPCBackendPlugin) GRPCClient(ctx context.Context, broker *plugin.GRPCBroker, c *grpc.ClientConn) (interface{}, error) {
4443
ret := &backendGRPCPluginClient{
45-
client: pb.NewBackendClient(c),
46-
clientConn: c,
47-
broker: broker,
48-
cleanupCh: make(chan struct{}),
49-
doneCtx: ctx,
50-
// Only run in metadata mode if mode is true and autoMTLS is not supported
51-
metadataMode: b.MetadataMode && !b.AutoMTLSSupported,
44+
client: pb.NewBackendClient(c),
45+
clientConn: c,
46+
broker: broker,
47+
cleanupCh: make(chan struct{}),
48+
doneCtx: ctx,
49+
metadataMode: b.MetadataMode,
5250
}
5351

5452
// Create the value and set the type

‎sdk/plugin/plugin.go

+19-41
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"sync"
88

99
"github.com/hashicorp/errwrap"
10+
log "github.com/hashicorp/go-hclog"
1011
plugin "github.com/hashicorp/go-plugin"
1112
"github.com/hashicorp/vault/sdk/helper/consts"
1213
"github.com/hashicorp/vault/sdk/helper/pluginutil"
@@ -34,7 +35,7 @@ func (b *BackendPluginClient) Cleanup(ctx context.Context) {
3435
// external plugins, or a concrete implementation of the backend if it is a builtin backend.
3536
// The backend is returned as a logical.Backend interface. The isMetadataMode param determines whether
3637
// the plugin should run in metadata mode.
37-
func NewBackend(ctx context.Context, pluginName string, pluginType consts.PluginType, sys pluginutil.LookRunnerUtil, conf *logical.BackendConfig, isMetadataMode bool, autoMTLS bool) (logical.Backend, error) {
38+
func NewBackend(ctx context.Context, pluginName string, pluginType consts.PluginType, sys pluginutil.LookRunnerUtil, conf *logical.BackendConfig, isMetadataMode bool) (logical.Backend, error) {
3839
// Look for plugin in the plugin catalog
3940
pluginRunner, err := sys.LookupPlugin(ctx, pluginName, pluginType)
4041
if err != nil {
@@ -58,16 +59,8 @@ func NewBackend(ctx context.Context, pluginName string, pluginType consts.Plugin
5859
}
5960
}
6061
} else {
61-
config := pluginutil.PluginClientConfig{
62-
Name: pluginName,
63-
PluginType: pluginType,
64-
Logger: conf.Logger.Named(pluginName),
65-
IsMetadataMode: isMetadataMode,
66-
AutoMTLS: autoMTLS,
67-
Wrapper: sys,
68-
}
6962
// create a backendPluginClient instance
70-
backend, err = NewPluginClient(ctx, pluginRunner, config)
63+
backend, err = NewPluginClient(ctx, sys, pluginRunner, conf.Logger, isMetadataMode)
7164
if err != nil {
7265
return nil, err
7366
}
@@ -76,49 +69,34 @@ func NewBackend(ctx context.Context, pluginName string, pluginType consts.Plugin
7669
return backend, nil
7770
}
7871

79-
// pluginSet returns the go-plugin PluginSet that we can dispense. This ensures
80-
// that plugins that don't support AutoMTLS are run on the appropriate version.
81-
func pluginSet(autoMTLS, metadataMode bool) map[int]plugin.PluginSet {
82-
if autoMTLS {
83-
return map[int]plugin.PluginSet{
84-
5: {
85-
"backend": &GRPCBackendPlugin{
86-
MetadataMode: false,
87-
AutoMTLSSupported: true,
88-
},
89-
},
90-
}
91-
}
92-
return map[int]plugin.PluginSet{
72+
func NewPluginClient(ctx context.Context, sys pluginutil.RunnerUtil, pluginRunner *pluginutil.PluginRunner, logger log.Logger, isMetadataMode bool) (logical.Backend, error) {
73+
// pluginMap is the map of plugins we can dispense.
74+
pluginSet := map[int]plugin.PluginSet{
9375
// Version 3 used to supports both protocols. We want to keep it around
9476
// since it's possible old plugins built against this version will still
9577
// work with gRPC. There is currently no difference between version 3
9678
// and version 4.
9779
3: {
9880
"backend": &GRPCBackendPlugin{
99-
MetadataMode: metadataMode,
81+
MetadataMode: isMetadataMode,
10082
},
10183
},
10284
4: {
10385
"backend": &GRPCBackendPlugin{
104-
MetadataMode: metadataMode,
86+
MetadataMode: isMetadataMode,
10587
},
10688
},
10789
}
108-
}
10990

110-
func NewPluginClient(ctx context.Context, pluginRunner *pluginutil.PluginRunner, config pluginutil.PluginClientConfig) (logical.Backend, error) {
111-
ps := pluginSet(config.AutoMTLS, config.IsMetadataMode)
112-
113-
client, err := pluginRunner.RunConfig(ctx,
114-
pluginutil.Runner(config.Wrapper),
115-
pluginutil.PluginSets(ps),
116-
pluginutil.HandshakeConfig(handshakeConfig),
117-
pluginutil.Env(),
118-
pluginutil.Logger(config.Logger),
119-
pluginutil.MetadataMode(config.IsMetadataMode),
120-
pluginutil.AutoMTLS(config.AutoMTLS),
121-
)
91+
namedLogger := logger.Named(pluginRunner.Name)
92+
93+
var client *plugin.Client
94+
var err error
95+
if isMetadataMode {
96+
client, err = pluginRunner.RunMetadataMode(ctx, sys, pluginSet, handshakeConfig, []string{}, namedLogger)
97+
} else {
98+
client, err = pluginRunner.Run(ctx, sys, pluginSet, handshakeConfig, []string{}, namedLogger)
99+
}
122100
if err != nil {
123101
return nil, err
124102
}
@@ -148,9 +126,9 @@ func NewPluginClient(ctx context.Context, pluginRunner *pluginutil.PluginRunner,
148126
}
149127

150128
// Wrap the backend in a tracing middleware
151-
if config.Logger.IsTrace() {
129+
if namedLogger.IsTrace() {
152130
backend = &backendTracingMiddleware{
153-
logger: config.Logger.With("transport", transport),
131+
logger: namedLogger.With("transport", transport),
154132
next: backend,
155133
}
156134
}

0 commit comments

Comments
 (0)