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.

Sign up for GitHub

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

Add SDK helpers and Core stubs for plugins to communicate with Enterprise Rotation Manager #29273

Merged
merged 13 commits into from
Jan 7, 2025
Merged

Add SDK helpers and Core stubs for plugins to communicate with Enterprise Rotation Manager #29273

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
78efc39
add relevant sdk helpers/methods for plugins to communicate with RM
vinay-gopalan Jan 2, 2025
86301b4
Update sdk/logical/rotation_job.go
robmonte Jan 6, 2025
dbe1b10
Update sdk/helper/automatedrotationutil/fields.go
robmonte Jan 6, 2025
baba2e2
create top-level sdk/rotation package
vinay-gopalan Jan 6, 2025
b32e721
fix upstream conflict in comment
vinay-gopalan Jan 6, 2025
08647d1
add dergesitration flow
vinay-gopalan Jan 6, 2025
cbba982
add changelog
vinay-gopalan Jan 6, 2025
1d93da9
fix aws config tests
vinay-gopalan Jan 6, 2025
71c0eb9
update to use rotation_period and add helper to check registration
vinay-gopalan Jan 7, 2025
d4b97e1
Merge branch 'main' into VAULT-33156/rotation-manager-ce-stubs
vinay-gopalan Jan 7, 2025
0f9ac2a
run make proto with protoc-gen-go v1.36.0
vinay-gopalan Jan 7, 2025
7399636
add note on sysview methods
vinay-gopalan Jan 7, 2025
0800a06
Merge branch 'main' into VAULT-33156/rotation-manager-ce-stubs
vinay-gopalan Jan 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 87 additions & 0 deletions builtin/logical/aws/backend.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
"sync"
"time"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/iam"
"github.com/aws/aws-sdk-go/service/iam/iamiface"
"github.com/aws/aws-sdk-go/service/sts/stsiface"
"github.com/hashicorp/vault/sdk/framework"
Expand Down Expand Up @@ -74,6 +76,91 @@ func Backend(_ *logical.BackendConfig) *backend {
}
return nil
},
RotateCredential: func(ctx context.Context, req *logical.Request) error {
// the following code is a modified version of the rotate-root method
client, err := b.clientIAM(ctx, req.Storage)
if err != nil {
return err
}
if client == nil {
return fmt.Errorf("nil IAM client")
}

b.clientMutex.Lock()
defer b.clientMutex.Unlock()

rawRootConfig, err := req.Storage.Get(ctx, "config/root")
if err != nil {
return err
}
if rawRootConfig == nil {
return fmt.Errorf("no configuration found for config/root")
}
var config rootConfig
if err := rawRootConfig.DecodeJSON(&config); err != nil {
return fmt.Errorf("error reading root configuration: %w", err)
}

if config.AccessKey == "" || config.SecretKey == "" {
return fmt.Errorf("cannot call config/rotate-root when either access_key or secret_key is empty")
}

var getUserInput iam.GetUserInput // empty input means get current user
getUserRes, err := client.GetUserWithContext(ctx, &getUserInput)
if err != nil {
return fmt.Errorf("error calling GetUser: %w", err)
}
if getUserRes == nil {
return fmt.Errorf("nil response from GetUser")
}
if getUserRes.User == nil {
return fmt.Errorf("nil user returned from GetUser")
}
if getUserRes.User.UserName == nil {
return fmt.Errorf("nil UserName returned from GetUser")
}

createAccessKeyInput := iam.CreateAccessKeyInput{
UserName: getUserRes.User.UserName,
}
createAccessKeyRes, err := client.CreateAccessKeyWithContext(ctx, &createAccessKeyInput)
if err != nil {
return fmt.Errorf("error calling CreateAccessKey: %w", err)
}
if createAccessKeyRes.AccessKey == nil {
return fmt.Errorf("nil response from CreateAccessKey")
}
if createAccessKeyRes.AccessKey.AccessKeyId == nil || createAccessKeyRes.AccessKey.SecretAccessKey == nil {
return fmt.Errorf("nil AccessKeyId or SecretAccessKey returned from CreateAccessKey")
}

oldAccessKey := config.AccessKey

config.AccessKey = *createAccessKeyRes.AccessKey.AccessKeyId
config.SecretKey = *createAccessKeyRes.AccessKey.SecretAccessKey

newEntry, err := logical.StorageEntryJSON("config/root", config)
if err != nil {
return fmt.Errorf("error generating new config/root JSON: %w", err)
}
if err := req.Storage.Put(ctx, newEntry); err != nil {
return fmt.Errorf("error saving new config/root: %w", err)
}

b.iamClient = nil
b.stsClient = nil

deleteAccessKeyInput := iam.DeleteAccessKeyInput{
AccessKeyId: aws.String(oldAccessKey),
UserName: getUserRes.User.UserName,
}
_, err = client.DeleteAccessKeyWithContext(ctx, &deleteAccessKeyInput)
if err != nil {
return fmt.Errorf("error deleting old access key: %w", err)
}

return nil
},
BackendType: logical.TypeLogical,
}

Expand Down
111 changes: 98 additions & 13 deletions builtin/logical/aws/path_config_root.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,18 @@ import (

"github.com/aws/aws-sdk-go/aws"
"github.com/hashicorp/vault/sdk/framework"
"github.com/hashicorp/vault/sdk/helper/automatedrotationutil"
"github.com/hashicorp/vault/sdk/helper/pluginidentityutil"
"github.com/hashicorp/vault/sdk/helper/pluginutil"
"github.com/hashicorp/vault/sdk/logical"
"github.com/hashicorp/vault/sdk/rotation"
)

// A single default template that supports both the different credential types (IAM/STS) that are capped at differing length limits (64 chars/32 chars respectively)
const defaultUserNameTemplate = `{{ if (eq .Type "STS") }}{{ printf "vault-%s-%s" (unix_time) (random 20) | truncate 32 }}{{ else }}{{ printf "vault-%s-%s-%s" (printf "%s-%s" (.DisplayName) (.PolicyName) | truncate 42) (unix_time) (random 20) | truncate 64 }}{{ end }}`
const (
defaultUserNameTemplate = `{{ if (eq .Type "STS") }}{{ printf "vault-%s-%s" (unix_time) (random 20) | truncate 32 }}{{ else }}{{ printf "vault-%s-%s-%s" (printf "%s-%s" (.DisplayName) (.PolicyName) | truncate 42) (unix_time) (random 20) | truncate 64 }}{{ end }}`
rootRotationJobName = "aws-root-creds"
)

func pathConfigRoot(b *backend) *framework.Path {
p := &framework.Path{
Expand Down Expand Up @@ -95,6 +100,7 @@ func pathConfigRoot(b *backend) *framework.Path {
HelpDescription: pathConfigRootHelpDesc,
}
pluginidentityutil.AddPluginIdentityTokenFields(p.Fields)
automatedrotationutil.AddAutomatedRotationFields(p.Fields)

return p
}
Expand All @@ -103,20 +109,14 @@ func (b *backend) pathConfigRootRead(ctx context.Context, req *logical.Request,
b.clientMutex.RLock()
defer b.clientMutex.RUnlock()

entry, err := req.Storage.Get(ctx, "config/root")
config, exists, err := getConfigFromStorage(ctx, req)
if err != nil {
return nil, err
}
if entry == nil {
if !exists {
return nil, nil
}

var config rootConfig

if err := entry.DecodeJSON(&config); err != nil {
return nil, err
}

configData := map[string]interface{}{
"access_key": config.AccessKey,
"region": config.Region,
Expand All @@ -131,6 +131,8 @@ func (b *backend) pathConfigRootRead(ctx context.Context, req *logical.Request,
}

config.PopulatePluginIdentityTokenData(configData)
config.PopulateAutomatedRotationData(configData)

return &logical.Response{
Data: configData,
}, nil
Expand Down Expand Up @@ -158,6 +160,12 @@ func (b *backend) pathConfigRootWrite(ctx context.Context, req *logical.Request,
b.clientMutex.Lock()
defer b.clientMutex.Unlock()

// check for existing config
previousCfg, previousCfgExists, err := getConfigFromStorage(ctx, req)
if err != nil {
return nil, err
}

rc := rootConfig{
AccessKey: data.Get("access_key").(string),
SecretKey: data.Get("secret_key").(string),
Expand All @@ -174,6 +182,9 @@ func (b *backend) pathConfigRootWrite(ctx context.Context, req *logical.Request,
if err := rc.ParsePluginIdentityTokenFields(data); err != nil {
return logical.ErrorResponse(err.Error()), nil
}
if err := rc.ParseAutomatedRotationFields(data); err != nil {
return logical.ErrorResponse(err.Error()), nil
}

if rc.IdentityTokenAudience != "" && rc.AccessKey != "" {
return logical.ErrorResponse("only one of 'access_key' or 'identity_token_audience' can be set"), nil
Expand All @@ -195,12 +206,54 @@ func (b *backend) pathConfigRootWrite(ctx context.Context, req *logical.Request,
}
}

entry, err := logical.StorageEntryJSON("config/root", rc)
if err != nil {
return nil, err
// Save the initial config only if it does not already exist
if !previousCfgExists {
if err := putConfigToStorage(ctx, req, &rc); err != nil {
return nil, err
}
}

if err := req.Storage.Put(ctx, entry); err != nil {
// Now that the root config is set up, register the rotation job if it required
vinay-gopalan marked this conversation as resolved.
Show resolved Hide resolved
if rc.ShouldRegisterRotationJob() {
cfgReq := &rotation.RotationJobConfigureRequest{
Name: rootRotationJobName,
MountPoint: req.MountPoint,
ReqPath: req.Path,
RotationSchedule: rc.RotationSchedule,
RotationWindow: rc.RotationWindow,
RotationPeriod: rc.RotationPeriod,
}

rotationJob, err := rotation.ConfigureRotationJob(cfgReq)
if err != nil {
return logical.ErrorResponse("error configuring rotation job: %s", err), nil
}

b.Logger().Debug("Registering rotation job", "mount", req.MountPoint+req.Path)
rotationID, err := b.System().RegisterRotationJob(ctx, rotationJob)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How and when does a rotation job get deregistered?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call out, not sure if we have yet discussed the idea of deregistering rotation. Is it something we should allow? I wonder if there's any differences between allowing automated rotation to be disabled versus a pause mechanism meant only for emergencies?

Copy link
Contributor Author

@vinay-gopalan vinay-gopalan Jan 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

De-registration / off-boarding should be allowed IMO, if only as a disabling escape hatch. Added a method on the SystemView to Deregister a rotation job. This will be triggered if a boolean disable_automated_rotation (subject to change) is set to true on the config. Example of this in AWS is linked here

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about the disable and delete actions. Without deregistration, how do we stop rotations when a mount is disabled or a static role is deleted that have been registered with the Rotation Manager?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see, that's a good question! So this won't be visible in this PR, but the Enterprise Rotation Manager implementation does store the request path that a credential comes from. The RM also has access to core.Router, and I believe we should be able to use MatchingMountEntry to determine if a given mount exists.

This will be performed on the Enterprise Rotation Manager code (before it routes the request to the plugin backend, it can confirm that a matching mount entry exists for handling the request). I can make sure to test that flow, and call it out in the Enterprise PR once it is opened (after this PR is merged in so that CI in Enterprise is happy)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it can confirm that a matching mount entry exists for handling the request

I think we will want to deregister a job when the backend/role is disabled or deleted, right?

if err != nil {
return logical.ErrorResponse("error registering rotation job: %s", err), nil
}

rc.RotationID = rotationID
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the backend is reloaded, will this value be overwritten with a new value? Should we check if the mount's root rotation job has already been registered before registering?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, don't think we accounted for this yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good question. Like Robert said, we haven't worked this out yet, but one thought I had was the following:

We can have the RM keep track of each Rotation ID in the form of a list. The structure of Rotation IDs encodes the Request Path as well. Before registering a root credential, we can check the incoming Rotation Job's ReqPath, and if that Path has already been stored in our Rotation IDs list, we can avoid registering and log a warning — stating the user needs to delete a rotation job before re-registering another for the same mount.

Can sketch out some ideas to enable tracking IDs in the case where the plugin crashes/reloads, and will put that up as part of the Enterprise PR 👍🏼

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The structure of Rotation IDs encodes the Request Path as well

Sorry to keep coming back to this, but if we are already doing this in Vault Core, why are we requiring plugins to store this mapping as well?

}

// Disable Automated Rotation and Deregister credentials if required
if rc.DisableAutomatedRotation {
// Ensure de-registering only occurs on updates and if
// a credential has actually been registered
if previousCfgExists && previousCfg.RotationID != "" {
err := b.System().DeregisterRotationJob(ctx, previousCfg.RotationID)
if err != nil {
return logical.ErrorResponse("error de-registering rotation job: %s", err), nil
}

rc.RotationID = ""
}
}

// update config entry with rotation ID
if err := putConfigToStorage(ctx, req, &rc); err != nil {
return nil, err
}

Expand All @@ -212,8 +265,40 @@ func (b *backend) pathConfigRootWrite(ctx context.Context, req *logical.Request,
return nil, nil
}

func getConfigFromStorage(ctx context.Context, req *logical.Request) (*rootConfig, bool, error) {
entry, err := req.Storage.Get(ctx, "config/root")
if err != nil {
return nil, false, err
}
if entry == nil {
return nil, false, nil
}

var config rootConfig

if err := entry.DecodeJSON(&config); err != nil {
return nil, false, err
}

return &config, true, nil
}

func putConfigToStorage(ctx context.Context, req *logical.Request, rc *rootConfig) error {
entry, err := logical.StorageEntryJSON("config/root", rc)
if err != nil {
return err
}

if err := req.Storage.Put(ctx, entry); err != nil {
return err
}

return nil
}

type rootConfig struct {
pluginidentityutil.PluginIdentityTokenParams
automatedrotationutil.AutomatedRotationParams

AccessKey string `json:"access_key"`
SecretKey string `json:"secret_key"`
Expand Down
Loading
Loading