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

Add Ability to Disable Replication Status Endpoints in Listener Configuration #23547

Merged
merged 21 commits into from
Oct 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
83ae451
CI: Pre-emptively delete logs dir after cache restore in test-collect…
kubawi Oct 11, 2023
f144d1b
Fix OktaNumberChallenge (#23565)
Monkeychip Oct 11, 2023
163c03f
exclude changelog in verifying doc/ui PRs (#23601)
hghaf099 Oct 11, 2023
88fb88e
Audit: eventlogger sink node reopen on SIGHUP (#23598)
Oct 11, 2023
bc64648
Capture errors emitted from all nodes during proccessing of audit pip…
Oct 11, 2023
525bf2f
Update security-scan.yml
mcollao-hc Oct 11, 2023
813c786
Listeners: Redaction only for TCP (#23592)
Oct 11, 2023
2f8e59c
fix panic when unlocking unlocked user (#23611)
davidadeleon Oct 11, 2023
30f19b3
VAULT-18307: update rotation period for aws static roles on update (#…
kpcraig Oct 11, 2023
88183ab
add disable_replication_status_endpoints tcp listener config parameter
Oct 3, 2023
1111cbd
add wrapping handler for disabled replication status endpoints setting
Oct 5, 2023
143d47c
adapt disable_replication_status_endpoints configuration parsing code…
Oct 5, 2023
ca60b22
refactor configuration parsing code to facilitate testing
Oct 6, 2023
294d962
fix a panic when parsing configuration
Oct 6, 2023
47c3f5b
update refactored configuration parsing code
Oct 6, 2023
a33a93c
fix merge corruption
Oct 6, 2023
2b9879b
add changelog file
Oct 10, 2023
2319e88
document new TCP listener configuration parameter
Oct 10, 2023
c420c66
make sure disable_replication_status_endpoints only has effect on TCP…
Oct 10, 2023
a9feb41
use active voice for explanation of disable_replication_status_endpoints
Oct 10, 2023
3f5f03c
fix minor merge issue
Oct 11, 2023
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
5 changes: 3 additions & 2 deletions .github/scripts/verify_changes.sh
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,9 @@ else
fi

# git diff with ... shows the differences between base_commit and head_commit starting at the last common commit
changed_dir=$(git diff $base_commit...$head_commit --name-only | awk -F"/" '{ print $1}' | uniq)
change_count=$(git diff $base_commit...$head_commit --name-only | awk -F"/" '{ print $1}' | uniq | wc -l)
# excluding the changelog directory
changed_dir=$(git diff $base_commit...$head_commit --name-only | awk -F"/" '{ print $1}' | uniq | sed '/changelog/d')
change_count=$(git diff $base_commit...$head_commit --name-only | awk -F"/" '{ print $1}' | uniq | sed '/changelog/d' | wc -l)

# There are 4 main conditions to check:
#
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/security-scan.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ jobs:
repository: hashicorp/security-scanner
token: ${{ secrets.HASHIBOT_PRODSEC_GITHUB_TOKEN }}
path: security-scanner
ref: 52d94588851f38a416f11c1e727131b3c8b0dd4d
ref: 6c530f211c0a1b1da078a9c80341ebe2dca4200c

- name: Install dependencies
shell: bash
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/test-go.yml
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,7 @@ jobs:
name: test-results
path: test-results/go-test
- run: |
rm -rf test-results/go-test/logs
ls -lhR test-results/go-test
find test-results/go-test -mindepth 1 -mtime +3 -delete

Expand Down
44 changes: 28 additions & 16 deletions builtin/audit/file/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -391,27 +391,39 @@ func (b *Backend) open() error {
}

func (b *Backend) Reload(_ context.Context) error {
switch b.path {
case stdout, discard:
// When there are nodes created in the map, use the eventlogger behavior.
if len(b.nodeMap) > 0 {
for _, n := range b.nodeMap {
if n.Type() == eventlogger.NodeTypeSink {
return n.Reopen()
}
}

return nil
}
} else {
// old non-eventlogger behavior
switch b.path {
case stdout, discard:
return nil
}

b.fileLock.Lock()
defer b.fileLock.Unlock()
b.fileLock.Lock()
defer b.fileLock.Unlock()

if b.f == nil {
return b.open()
}
if b.f == nil {
return b.open()
}

err := b.f.Close()
// Set to nil here so that even if we error out, on the next access open()
// will be tried
b.f = nil
if err != nil {
return err
}
err := b.f.Close()
// Set to nil here so that even if we error out, on the next access open()
// will be tried
b.f = nil
if err != nil {
return err
}

return b.open()
return b.open()
}
}

func (b *Backend) Invalidate(_ context.Context) {
Expand Down
14 changes: 14 additions & 0 deletions builtin/logical/aws/path_static_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,20 @@ func (b *backend) pathStaticRolesWrite(ctx context.Context, req *logical.Request
if err != nil {
return nil, fmt.Errorf("failed to add item into the rotation queue for role %q: %w", config.Name, err)
}
} else {
// creds already exist, so all we need to do is update the rotation
// what here stays the same and what changes? Can we change the name?
i, err := b.credRotationQueue.PopByKey(config.Name)
if err != nil {
return nil, fmt.Errorf("expected an item with name %q, but got an error: %w", config.Name, err)
}
i.Value = config
// update the next rotation to occur at now + the new rotation period
i.Priority = time.Now().Add(config.RotationPeriod).Unix()
err = b.credRotationQueue.Push(i)
if err != nil {
return nil, fmt.Errorf("failed to add updated item into the rotation queue for role %q: %w", config.Name, err)
}
}

return &logical.Response{
Expand Down
65 changes: 59 additions & 6 deletions builtin/logical/aws/path_static_roles_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,12 +124,21 @@ func TestStaticRolesWrite(t *testing.T) {
bgCTX := context.Background()

cases := []struct {
name string
opts []awsutil.MockIAMOption
name string
// objects to return from mock IAM.
// You'll need a GetUserOutput (to validate the existence of the user being written,
// the keys the user has already been assigned,
// and the new key vault requests.
opts []awsutil.MockIAMOption // objects to return from the mock IAM
// the name, username if updating, and rotation_period of the user. This is the inbound request the cod would get.
data map[string]interface{}
expectedError bool
findUser bool
isUpdate bool
// if data is sent the name "johnny", then we'll match an existing user with rotation period 24 hours.
isUpdate bool
newPriority int64 // update time of new item in queue, skip if isUpdate false. There is a wiggle room of 5 seconds
// so the deltas between the old and the new update time should be larger than that to ensure the difference
// can be detected.
}{
{
name: "happy path",
Expand Down Expand Up @@ -168,7 +177,7 @@ func TestStaticRolesWrite(t *testing.T) {
expectedError: true,
},
{
name: "update existing user",
name: "update existing user, decreased rotation duration",
opts: []awsutil.MockIAMOption{
awsutil.WithGetUserOutput(&iam.GetUserOutput{User: &iam.User{UserName: aws.String("john-doe"), UserId: aws.String("unique-id")}}),
awsutil.WithListAccessKeysOutput(&iam.ListAccessKeysOutput{
Expand All @@ -187,8 +196,33 @@ func TestStaticRolesWrite(t *testing.T) {
"name": "johnny",
"rotation_period": "19m",
},
findUser: true,
isUpdate: true,
findUser: true,
isUpdate: true,
newPriority: time.Now().Add(19 * time.Minute).Unix(),
},
{
name: "update existing user, increased rotation duration",
opts: []awsutil.MockIAMOption{
awsutil.WithGetUserOutput(&iam.GetUserOutput{User: &iam.User{UserName: aws.String("john-doe"), UserId: aws.String("unique-id")}}),
awsutil.WithListAccessKeysOutput(&iam.ListAccessKeysOutput{
AccessKeyMetadata: []*iam.AccessKeyMetadata{},
IsTruncated: aws.Bool(false),
}),
awsutil.WithCreateAccessKeyOutput(&iam.CreateAccessKeyOutput{
AccessKey: &iam.AccessKey{
AccessKeyId: aws.String("abcdefghijklmnopqrstuvwxyz"),
SecretAccessKey: aws.String("zyxwvutsrqponmlkjihgfedcba"),
UserName: aws.String("john-doe"),
},
}),
},
data: map[string]interface{}{
"name": "johnny",
"rotation_period": "40h",
},
findUser: true,
isUpdate: true,
newPriority: time.Now().Add(40 * time.Hour).Unix(),
},
}

Expand Down Expand Up @@ -269,6 +303,11 @@ func TestStaticRolesWrite(t *testing.T) {
expectedData = staticRole
}

var actualItem *queue.Item
if c.isUpdate {
actualItem, _ = b.credRotationQueue.PopByKey(expectedData.Name)
}

if u, ok := fieldData.GetOk("username"); ok {
expectedData.Username = u.(string)
}
Expand All @@ -289,6 +328,20 @@ func TestStaticRolesWrite(t *testing.T) {
if en, an := expectedData.Name, actualData.Name; en != an {
t.Fatalf("mismatched role name, expected %q, but got %q", en, an)
}

// one-off to avoid importing/casting
abs := func(x int64) int64 {
if x < 0 {
return -x
}
return x
}

if c.isUpdate {
if ep, ap := c.newPriority, actualItem.Priority; abs(ep-ap) > 5 { // 5 second wiggle room for how long the test takes
t.Fatalf("mismatched updated priority, expected %d but got %d", ep, ap)
}
}
})
}
}
Expand Down
3 changes: 3 additions & 0 deletions changelog/23528.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
secrets/aws: update credential rotation deadline when static role rotation period is updated
```
3 changes: 3 additions & 0 deletions changelog/23547.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:feature
config/listener: allow per-listener configuration setting to disable replication status endpoints.
Copy link
Contributor

Choose a reason for hiding this comment

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

@marcboudreau next time please use the correct new feature formatting for new features in the changelog - this also applies to #23534 and #23740. Probably there should be only a single changelog entry introducing the feature as a whole.

```
3 changes: 3 additions & 0 deletions changelog/23565.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
ui: Fix regression that broke the oktaNumberChallenge on the ui.
```
3 changes: 3 additions & 0 deletions changelog/23598.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
audit: Fix bug reopening 'file' audit devices on SIGHUP.
```
4 changes: 2 additions & 2 deletions command/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -886,9 +886,9 @@ func (c *ServerCommand) InitListeners(config *server.Config, disableClustering b
}

if reloadFunc != nil {
relSlice := (*c.reloadFuncs)["listener|"+lnConfig.Type]
relSlice := (*c.reloadFuncs)[fmt.Sprintf("listener|%s", lnConfig.Type)]
relSlice = append(relSlice, reloadFunc)
(*c.reloadFuncs)["listener|"+lnConfig.Type] = relSlice
(*c.reloadFuncs)[fmt.Sprintf("listener|%s", lnConfig.Type)] = relSlice
}

if !disableClustering && lnConfig.Type == "tcp" {
Expand Down
35 changes: 26 additions & 9 deletions command/server/config_test_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -793,7 +793,7 @@ func testConfig_Sanitized(t *testing.T) {
"address": "127.0.0.1:443",
"chroot_namespace": "admin/",
},
"type": "tcp",
"type": configutil.TCP,
},
},
"log_format": "",
Expand Down Expand Up @@ -890,6 +890,15 @@ listener "tcp" {
redact_addresses = true
redact_cluster_name = true
redact_version = true
}
listener "unix" {
address = "/var/run/vault.sock"
socket_mode = "644"
socket_user = "1000"
socket_group = "1000"
redact_addresses = true
redact_cluster_name = true
redact_version = true
}`))

config := Config{
Expand All @@ -903,16 +912,14 @@ listener "tcp" {
config.Listeners = listeners
// Track which types of listener were found.
for _, l := range config.Listeners {
config.found(l.Type, l.Type)
config.found(l.Type.String(), l.Type.String())
}

if len(config.Listeners) == 0 {
t.Fatalf("expected at least one listener in the config")
}
listener := config.Listeners[0]
if listener.Type != "tcp" {
t.Fatalf("expected tcp listener in the config")
}
require.Len(t, config.Listeners, 2)
tcpListener := config.Listeners[0]
require.Equal(t, configutil.TCP, tcpListener.Type)
unixListner := config.Listeners[1]
require.Equal(t, configutil.Unix, unixListner.Type)

expected := &Config{
SharedConfig: &configutil.SharedConfig{
Expand Down Expand Up @@ -946,6 +953,16 @@ listener "tcp" {
RedactClusterName: true,
RedactVersion: true,
},
{
Type: "unix",
Address: "/var/run/vault.sock",
SocketMode: "644",
SocketUser: "1000",
SocketGroup: "1000",
RedactAddresses: false,
RedactClusterName: false,
RedactVersion: false,
},
},
},
}
Expand Down
2 changes: 1 addition & 1 deletion command/server/listener.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (
type ListenerFactory func(*configutil.Listener, io.Writer, cli.Ui) (net.Listener, map[string]string, reloadutil.ReloadFunc, error)

// BuiltinListeners is the list of built-in listener types.
var BuiltinListeners = map[string]ListenerFactory{
var BuiltinListeners = map[configutil.ListenerType]ListenerFactory{
"tcp": tcpListenerFactory,
"unix": unixListenerFactory,
}
Expand Down
29 changes: 18 additions & 11 deletions http/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,20 +237,27 @@ func handler(props *vault.HandlerProperties) http.Handler {
additionalRoutes(mux, core)
}

// Wrap the handler in another handler to trigger all help paths.
helpWrappedHandler := wrapHelpHandler(mux, core)
corsWrappedHandler := wrapCORSHandler(helpWrappedHandler, core)
quotaWrappedHandler := rateLimitQuotaWrapping(corsWrappedHandler, core)
genericWrappedHandler := genericWrapping(core, quotaWrappedHandler, props)

// Wrap the handler with PrintablePathCheckHandler to check for non-printable
// characters in the request path.
printablePathCheckHandler := genericWrappedHandler
// Build up a chain of wrapping handlers.
wrappedHandler := wrapHelpHandler(mux, core)
wrappedHandler = wrapCORSHandler(wrappedHandler, core)
wrappedHandler = rateLimitQuotaWrapping(wrappedHandler, core)
wrappedHandler = genericWrapping(core, wrappedHandler, props)

// Add an extra wrapping handler if the DisablePrintableCheck listener
// setting isn't true that checks for non-printable characters in the
// request path.
if !props.DisablePrintableCheck {
printablePathCheckHandler = cleanhttp.PrintablePathCheckHandler(genericWrappedHandler, nil)
wrappedHandler = cleanhttp.PrintablePathCheckHandler(wrappedHandler, nil)
}

return printablePathCheckHandler
// Add an extra wrapping handler if the DisableReplicationStatusEndpoints
// setting is true that will create a new request with a context that has
// a value indicating that the replication status endpoints are disabled.
if props.ListenerConfig != nil && props.ListenerConfig.DisableReplicationStatusEndpoints {
wrappedHandler = disableReplicationStatusEndpointWrapping(wrappedHandler)
}

return wrappedHandler
}

type copyResponseWriter struct {
Expand Down
8 changes: 8 additions & 0 deletions http/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,14 @@ func rateLimitQuotaWrapping(handler http.Handler, core *vault.Core) http.Handler
})
}

func disableReplicationStatusEndpointWrapping(h http.Handler) http.Handler {

Choose a reason for hiding this comment

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

Just needs a go doc comment here 😄

return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
request := r.WithContext(context.WithValue(r.Context(), "disable_replication_status_endpoints", true))

h.ServeHTTP(w, request)
})
}

func parseRemoteIPAddress(r *http.Request) string {
ip, _, err := net.SplitHostPort(r.RemoteAddr)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion internalshared/configutil/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ func ParseConfig(d string) (*SharedConfig, error) {

// Track which types of listener were found.
for _, l := range result.Listeners {
result.found(l.Type, l.Type)
result.found(l.Type.String(), l.Type.String())
}
}

Expand Down
Loading
Loading