Append call in form of `append(s)` has no effect,
it just returns `s`. Sometimes such invocation is a sign
of a programming error, so it's better to remove these.
Signed-off-by: Iskander Sharipov <quasilyte@gmail.com>
* Fix SSH zero address OTP delete
Fixed bug where SSH OTP roles could not be deleted if a zero-address role
previously existed, and there currently exist no zero-address roles.
Fixes#6382
* Eliminate zeroAddressRoles remove function
The result will still pass gofmtcheck and won't trigger additional
changes if someone isn't using goimports, but it will avoid the
piecemeal imports changes we've been seeing.
* Strip empty strings from database revocation stmts
It's technically valid to give empty strings as statements to run on
most databases. However, in the case of revocation statements, it's not
only generally inadvisable but can lead to lack of revocations when you
expect them. This strips empty strings from the array of revocation
statements.
It also makes two other changes:
* Return statements on read as empty but valid arrays rather than nulls,
so that typing information is inferred (this is more in line with the
rest of Vault these days)
* Changes field data for TypeStringSlice and TypeCommaStringSlice such
that a client-supplied value of `""` doesn't turn into `[]string{""}`
but rather `[]string{}`.
The latter and the explicit revocation statement changes are related,
and defense in depth.
This PR fix#5778.
Easy test case to reproduce the problem:
https://play.golang.org/p/CAMdrOHT7C1
Since `certStr` is empty string during first iteration `strings.Join()`
will merge empty line with first CA cert.
Extra `strings.TrimSpace` call will remove that empty line, before
certificate will be return.
* Adding support for Consul 1.4 ACL system
* Working tests
* Fixed logic gate
* Fixed logical gate that evaluate empty policy or empty list of policy names
* Ensure tests are run against appropiate Consul versions
* Running tests against official container with a 1.4.0-rc1 tag
* policies can never be nil (as even if it is empty will be an empty array)
* addressing feedback, refactoring tests
* removing cast
* converting old lease field to ttl, adding max ttl
* cleanup
* adding missing test
* testing wrong version
* adding support for local tokens
* addressing feedback
* Change ordering of user lookup vs. password hashing
This fixes a very minor information leak where someone could brute force
the existence of a username. It's not perfect as the underlying storage
plays a part but bcrypt's slowness puts that much more in the noise.
* Added role-option max_sts_ttl to cap TTL for AWS STS credentials.
* Allow for setting max_sts_ttl to 0 after it has been set already.
* Fixed message in error response for default_sts_ttl > max_sts_ttl.
This endpoint eventually goes through generateCreationBundle where we
already have the right checks.
Also add expiration to returned value to match output when using root
generation.
Fixes#5549
* Set allowed OIDs to any value when generaing a CA.
Also, allow utf-8 in addition to utf8 as the OID type specifier, and
allow `*` to specify any OID of a supported type.
* Update PKI docs
* Add ability to configure the NotBefore property of certificates in role api
* Update index.html.md
* converting field to time.Duration
* setting default back to 30s
* renaming the parameter not_before_duration to differentiate between the NotBefore datetime on the cert
* Update description
* Allow specifying role-default TTLs in AWS secret engine
* Add an acceptance test
* Add docs for AWS secret role-default TTLs
* Rename default_ttl to default_sts_ttl
* Return default_ttl as int64 instead of time.Duration
* Fix broken tests
The merge of #5383 broke the tests due to some changes in the test style
that didn't actually cause a git merge conflict. This updates the tests
to the new style.
Specifying the `allowed_organiztaional_units` parameter to a cert auth
backend role will require client certificates to contain at least one of
a list of one or more "organizational units" (OU).
Example use cases:
Certificates are issued to entities in an organization arrangement by
organizational unit (OU). The OU may be a department, team, or any other logical
grouping of resources with similar roles. The entities within the OU
should be granted the same policies.
```
$ vault write auth/cert/certs/ou-engineering \
certificate=@ca.pem \
policies=engineering \
allowed_organiztaional_units=engineering
$ vault write auth/cert/certs/ou-engineering \
certificate=@ca.pem \
policies=engineering \
allowed_organiztaional_units=engineering,support
```
* logical/aws: Harden WAL entry creation
If AWS IAM user creation failed in any way, the WAL corresponding to the
IAM user would get left around and Vault would try to roll it back.
However, because the user never existed, the rollback failed. Thus, the
WAL would essentially get "stuck" and Vault would continually attempt to
roll it back, failing every time. A similar situation could arise if the
IAM user that Vault created got deleted out of band, or if Vault deleted
it but was unable to write the lease revocation back to storage (e.g., a
storage failure).
This attempts to harden it in two ways. One is by deleting the WAL log
entry if the IAM user creation fails. However, the WAL deletion could
still fail, and this wouldn't help where the user is deleted out of
band, so second, consider the user rolled back if the user just doesn't
exist, under certain circumstances.
Fixes#5190
* Fix segfault in expiration unit tests
TestExpiration_Tidy was passing in a leaseEntry that had a nil Secret,
which then caused a segfault as the changes to revokeEntry didn't check
whether Secret was nil; this is probably unlikely to occur in real life,
but good to be extra cautious.
* Fix potential segfault
Missed the else...
* Respond to PR feedback
* auth/aws: Make identity alias configurable
This is inspired by #4178, though not quite exactly what is requested
there. Rather than just use RoleSessionName as the Identity alias, the
full ARN is uses as the Alias. This mitigates against concerns that an
AWS role with an insufficiently secured trust policy could allow an
attacker to generate arbitrary RoleSessionNames in AssumeRole calls to
impersonate anybody in the Identity store that had an alias set up.
By using the full ARN, the owner of the identity store has to explicitly
trust specific AWS roles in specific AWS accounts to generate an
appropriate RoleSessionName to map back to an identity.
Fixes#4178
* Respond to PR feedback
* Remove CreateOperation
Response to PR feedback
* Add AWS Secret Engine Root Credential Rotation
This allows the AWS Secret Engine to rotate its credentials used to
access AWS. This will only work when the AWS Secret Engine has been
provided explicit IAM credentials via the config/root endpoint, and
further, when the IAM credentials provided are the only access key on
the IAM user associated wtih the access key (because AWS allows a
maximum of 2 access keys per user).
Fixes#4385
* Add test for AWS root credential rotation
Also fix a typo in the root credential rotation code
* Add docs for AWS root rotation
* Add locks around reading and writing config/root
And wire the backend up in a bunch of places so the config can get the
lock
* Respond to PR feedback
* Fix casing in error messages
* Fix merge errors
* Fix locking bugs
* Parallelize a couple AWS acceptance tests
Starting an effort to paralleize AWS secret engine acceptance tests.
Currently they take over a minute to run, and this parallelizes the two
that explicitly call a 10-second sleep, reulting in a 10-second speedup
in test time.
* Parameterize IAM user name
Probably not needed, but future-proofing the code
* Make remainder of tests parallel
AWS_ACCOUNT_ID environment variable is no longer being used; global
mutable state is a recipe for disaster when trying to run things in
parallel, and parallelizing the tests exposed a race condition in which
they were depending on the AWS_ACCOUNT_ID environment variable to be set
before they were run.
AWS_DEFAULT_REGION is still left as an environment variable because it
is required by AWS SDKs, but its configuration is now protected by a
sync.Once to ensure it only ever gets called a single time.
* Replace generateUnique*Name with testhelpers method
* Add test file for testing path_restore in Transit backend. Fails because 'force' is not implemented yet
* initial implementation of 'force', to force restore of existing transit key atomically
* Initial implemntation of returning 529 for rate limits
- bump aws iam and sts packages to v1.14.31 to get mocking interface
- promote the iam and sts clients to the aws backend struct, for mocking in tests
- this also promotes some functions to methods on the Backend struct, so
that we can use the injected client
Generating creds requires reading config/root for credentials to contact
IAM. Here we make pathConfigRoot a method on aws/backend so we can clear
the clients on successful update of config/root path. Adds a mutex to
safely clear the clients
* refactor locking and unlocking into methods on *backend
* refactor/simply the locking
* check client after grabbing lock
Update AWS Auth backend to use TypeHeader for iam request headers
- Remove parseIamRequestHeaders function and test, no longer needed with new TypeHeader
- Update AWS auth login docs
This will cause them to be removed even if they have not expired yet,
whereas before it would simply leave them in the store until they were
expired, but remove from revocation info.
* Disallow adding CA's serial to revocation list
* Allow disabling revocation list generation. This returns an empty (but
signed) list, but does not affect tracking of revocations so turning it
back on will populate the list properly.
* Nomad: updating max token length to 256
* Initial support for supporting custom max token name length for Nomad
* simplify/correct tests
* document nomad max_token_name_length
* removed support for max token length env var. Rename field for clarity
* cleanups after removing env var support
* move RandomWithPrefix to testhelpers
* fix spelling
* Remove default 256 value. Use zero as a sentinel value and ignore it
* update docs
* Make AWS credential types more explicit
The AWS secret engine had a lot of confusing overloading with role
paramemters and how they mapped to each of the three credential types
supported. This now adds parameters to remove the overloading while
maintaining backwards compatibility.
With the change, it also becomes easier to add other feature requests.
Attaching multiple managed policies to IAM users and adding a policy
document to STS AssumedRole credentials is now also supported.
Fixes#4229Fixes#3751Fixes#2817
* Add missing write action to STS endpoint
* Allow unsetting policy_document with empty string
This allows unsetting the policy_document by passing in an empty string.
Previously, it would fail because the empty string isn't a valid JSON
document.
* Respond to some PR feedback
* Refactor and simplify role reading/upgrading
This gets rid of the duplicated role upgrade code between both role
reading and role writing by handling the upgrade all in the role
reading.
* Eliminate duplicated AWS secret test code
The testAccStepReadUser and testAccStepReadSTS were virtually identical,
so they are consolidated into a single method with the path passed in.
* Switch to use AWS ARN parser
The backwards compatibility logic was preventing updates to role
statements from taking effect. This change removes persistence of
deprecated statement fields.
1) In backends, ensure they are now using TokenPolicies
2) Don't reassign auth.Policies until after expmgr registration as we
don't need them at that point
Fixes#4829
This change makes it so that if a lease is revoked through user action,
we set the expiration time to now and update pending, just as we do with
tokens. This allows the normal retry logic to apply in these cases as
well, instead of just erroring out immediately. The idea being that once
you tell Vault to revoke something it should keep doing its darndest to
actually make that happen.
* Database updates
* Add create/update distinction for connection config
* Add create/update distinction for role config
* Add db name and revocation statements to leases to give revocation a
shot at working if the role has been deleted
Fixes#3544Fixes#4782
* Add create/update info to docs
* Add an idle timeout for the server
Because tidy operations can be long-running, this also changes all tidy
operations to behave the same operationally (kick off the process, get a
warning back, log errors to server log) and makes them all run in a
goroutine.
This could mean a sort of hard stop if Vault gets sealed because the
function won't have the read lock. This should generally be okay
(running tidy again should pick back up where it left off), but future
work could use cleanup funcs to trigger the functions to stop.
* Fix up tidy test
* Add deadline to cluster connections and an idle timeout to the cluster server, plus add readheader/read timeout to api server
* This changes the way policies are reported in audit logs.
Previously, only policies tied to tokens would be reported. This could
make it difficult to perform after-the-fact analysis based on both the
initial response entry and further requests. Now, the full set of
applicable policies from both the token and any derived policies from
Identity are reported.
To keep things consistent, token authentications now also return the
full set of policies in api.Secret.Auth responses, so this both makes it
easier for users to understand their actual full set, and it matches
what the audit logs now report.
This massively simplifies transit locking behavior by pushing some
locking down to the Policy level, and embedding either a local or global
lock in the Policy depending on whether caching is enabled or not.
* Fix panic due to metadata being nil
* added a nil check
* Added a test
* ensure metadata is never nil
* Remove unnecessary allocation
* revert back to early initialization
Taking inspiration from
https://github.com/golang/go/issues/17604#issuecomment-256384471
suggests that taking the address of a stack variable for use in atomics
works (at least, the race detector doesn't complain) but is doing it
wrong.
The only other change is a change in Leader() detecting if HA is enabled
to fast-path out. This value never changes after NewCore, so we don't
need to grab the read lock to check it.
* Update PKI to natively use time.Duration
Among other things this now means PKI will output durations in seconds
like other backends, instead of as Go strings.
* Add a warning when refusing to blow away an existing root instead of just returning success
* Fix another issue found while debugging this...
The reason it wasn't caught on tests in the first place is that the ttl
and max ttl were only being compared if in addition to a provided csr, a
role was also provided. This was because the check was in the role !=
nil block instead of outside of it. This has been fixed, which made the
problem occur in all sign-verbatim cases and the changes in this PR have
now verified the fix.
* make invalid role_id a 400 error
* remove single-use validateCredentials function
* remove single-use validateBindSecretID function
* adjust the error message for CIDR check failure
* locking updates as review feedback
Jeff Mitchell