disable_mlock must now be explicitly included in config #29974

guygrigsby · 2025-03-20T18:00:07Z

Description

This PR eliminates a default value for disable_mlock for integrated storage. disable_mlock must now be set explicitly in a configuration file or env var if using integrated storage.

When Vault starts up with mlock enabled, Vault with integrated storage is prone to running out of memory because it doesn't allow the use of swap disk. However, while disabling mlock helps not run into OOM problems, it is a security issue because allowing for swapping to disk means that secret values can be viewed in plaintext if the swap files are inspected.

As such, Vault will no longer initialize until this value has been explicitly configured, and we will provide a clear explanation in the error message of the tradeoffs of choosing to enable or disable mlock on Integrated Storage.

We must also update documentation to include the new behavior.

TODO only if you're a HashiCorp employee

Backport Labels: If this fix needs to be backported, use the appropriate backport/ label that matches the desired release branch. Note that in the CE repo, the latest release branch will look like backport/x.x.x, but older release branches will be backport/ent/x.x.x+ent.
- LTS: If this fixes a critical security vulnerability or severity 1 bug, it will also need to be backported to the current LTS versions of Vault. To ensure this, use all available enterprise labels.
ENT Breakage: If this PR either 1) removes a public function OR 2) changes the signature
of a public function, even if that change is in a CE file, double check that
applying the patch for this PR to the ENT repo and running tests doesn't
break any tests. Sometimes ENT only tests rely on public functions in CE
files.
Jira: If this change has an associated Jira, it's referenced either
in the PR description, commit message, or branch name.
RFC: If this change has an associated RFC, please link it in the description.
ENT PR: If this change has an associated ENT PR, please link it in the
description. Also, make sure the changelog is in this PR, not in your ENT PR.

github-actions · 2025-03-20T18:11:51Z

CI Results:
All Go tests succeeded! ✅

command/server.go

 	entCheckRequestLimiter(c, config)

+	if config != nil {


command/server.go

sgmiller

I thought we wanted this to be mandatory only if using integrated storage?

github-actions · 2025-03-24T16:20:29Z

Build Results:
All builds succeeded! ✅

guygrigsby · 2025-03-24T17:20:19Z

@sgmiller I took another look at the requirements and you're right. I'll switch this back to draft on continue working on it.

…sablemlock-config

miagilepner

LGTM!

website/content/docs/configuration/index.mdx

website/content/docs/concepts/production-hardening.mdx

Use active voice. Co-authored-by: Sarah Chavis <[email protected]>

…sablemlock-config

Synchronize the `enos` directory on 1.19 with that of `main` after changes introduced in #29974 were not backported. Signed-off-by: Ryan Cragun <[email protected]>

github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Mar 20, 2025

guygrigsby force-pushed the guygrigsby/VAULT-34786/explicitly-require-disablemlock-config branch from 5818a2b to 084be9c Compare March 20, 2025 18:10

guygrigsby added the pr/no-milestone label Mar 20, 2025

guygrigsby force-pushed the guygrigsby/VAULT-34786/explicitly-require-disablemlock-config branch from e977c8f to 7aececf Compare March 21, 2025 17:09

guygrigsby commented Mar 21, 2025

View reviewed changes

command/server.go Outdated

entCheckRequestLimiter(c, config)

if config != nil {

This comment was marked as outdated.

Sign in to view

command/server.go Outdated Show resolved Hide resolved

github-advanced-security bot found potential problems Mar 21, 2025

View reviewed changes

command/server.go Fixed Show fixed Hide fixed

command/server.go Fixed Show fixed Hide fixed

guygrigsby force-pushed the guygrigsby/VAULT-34786/explicitly-require-disablemlock-config branch from 7aececf to c44c565 Compare March 21, 2025 19:02

guygrigsby added 4 commits March 24, 2025 08:16

require explicit value for disable_mlock

a684f19

set disable_mlock back to true for all docker tests

707ec8f

fix build error

6ab9674

update test config files

8780c7a

guygrigsby force-pushed the guygrigsby/VAULT-34786/explicitly-require-disablemlock-config branch from 71b840e to 8780c7a Compare March 24, 2025 14:16

guygrigsby marked this pull request as ready for review March 24, 2025 15:45

guygrigsby requested a review from a team as a code owner March 24, 2025 15:45

guygrigsby requested a review from mladlow March 24, 2025 15:45

sgmiller reviewed Mar 24, 2025

View reviewed changes

guygrigsby marked this pull request as draft March 24, 2025 17:21

change explicit mlock check to apply to integrated storage only.

c859b2a

vercel bot deployed to Preview March 24, 2025 18:27 View deployment

guygrigsby added 6 commits March 24, 2025 12:28

formatting and typo fixes

6e05112

Merge branch 'main' into guygrigsby/VAULT-34786/explicitly-require-di…

db8f66e

…sablemlock-config

added test for raft

531fabf

remove erroneous test

4a73645

remove unecessary doc line

edeee4e

Merge branch 'main' into guygrigsby/VAULT-34786/explicitly-require-di…

75c9121

…sablemlock-config

guygrigsby marked this pull request as ready for review March 25, 2025 17:51

guygrigsby requested a review from a team as a code owner March 25, 2025 17:51

vercel bot deployed to Preview April 9, 2025 17:11 View deployment

guygrigsby added 2 commits April 9, 2025 11:15

add missing envvar for docker env test

ee9b007

upate additional docker test disable_mlock values

11e3e25

ej-hashi previously approved these changes Apr 9, 2025

View reviewed changes

Merge branch 'main' into guygrigsby/VAULT-34786/explicitly-require-di…

908878b

…sablemlock-config

guygrigsby requested review from miagilepner and kubawi April 10, 2025 14:06

miagilepner previously approved these changes Apr 10, 2025

View reviewed changes

rculpepper previously approved these changes Apr 10, 2025

View reviewed changes

schavis requested changes Apr 11, 2025

View reviewed changes

website/content/docs/configuration/index.mdx Outdated Show resolved Hide resolved

website/content/docs/concepts/production-hardening.mdx Outdated Show resolved Hide resolved

Apply suggestions from code review

10626a9

Use active voice. Co-authored-by: Sarah Chavis <[email protected]>

guygrigsby dismissed stale reviews from rculpepper, miagilepner, and ej-hashi via 10626a9 April 11, 2025 17:52

Merge branch 'main' into guygrigsby/VAULT-34786/explicitly-require-di…

dc2e2b9

…sablemlock-config

guygrigsby requested a review from schavis April 11, 2025 17:53

schavis approved these changes Apr 11, 2025

View reviewed changes

guygrigsby requested a review from miagilepner April 11, 2025 18:35

ej-hashi approved these changes Apr 11, 2025

View reviewed changes

guygrigsby requested a review from rculpepper April 11, 2025 20:28

rculpepper approved these changes Apr 14, 2025

View reviewed changes

guygrigsby added 4 commits April 14, 2025 08:10

Merge branch 'main' into guygrigsby/VAULT-34786/explicitly-require-di…

5828e2d

…sablemlock-config

Merge branch 'main' into guygrigsby/VAULT-34786/explicitly-require-di…

e875cb6

…sablemlock-config

Merge branch 'main' into guygrigsby/VAULT-34786/explicitly-require-di…

6374830

…sablemlock-config

Merge branch 'main' into guygrigsby/VAULT-34786/explicitly-require-di…

0b83fd2

…sablemlock-config

miagilepner merged commit 08c5a52 into main Apr 17, 2025
95 checks passed

miagilepner deleted the guygrigsby/VAULT-34786/explicitly-require-disablemlock-config branch April 17, 2025 13:35

ryancragun added a commit that referenced this pull request Apr 22, 2025

enos(mlock): sync enos changes from main to 1.19

5cbd4c8

Synchronize the `enos` directory on 1.19 with that of `main` after changes introduced in #29974 were not backported. Signed-off-by: Ryan Cragun <[email protected]>

ryancragun mentioned this pull request Apr 22, 2025

enos(mlock): sync enos changes from main to 1.19 #30321

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

disable_mlock must now be explicitly included in config #29974

disable_mlock must now be explicitly included in config #29974

Uh oh!

guygrigsby commented Mar 20, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Mar 20, 2025 •

edited

Loading

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sgmiller left a comment

Uh oh!

github-actions bot commented Mar 24, 2025 •

edited

Loading

Uh oh!

guygrigsby commented Mar 24, 2025

Uh oh!

miagilepner left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

disable_mlock must now be explicitly included in config #29974

disable_mlock must now be explicitly included in config #29974

Uh oh!

Conversation

guygrigsby commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

TODO only if you're a HashiCorp employee

Uh oh!

github-actions bot commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sgmiller left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Mar 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

guygrigsby commented Mar 24, 2025

Uh oh!

miagilepner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

guygrigsby commented Mar 20, 2025 •

edited

Loading

github-actions bot commented Mar 20, 2025 •

edited

Loading

github-actions bot commented Mar 24, 2025 •

edited

Loading