Commit Graph

42 Commits

Author SHA1 Message Date
Connor f8fc317731
Fix leaked Vault LifetimeRenewers (#12607)
* Fix leaked Vault LifetimeRenewers

When the Vault CA Provider is reconfigured we do not stop the
LifetimeRenewers which can cause them to leak until the Consul processes
recycles. On Configure execute stopWatcher if it exists and is not nil
before starting a new renewal

* Add jitter before restarting the LifetimeWatcher

If we fail to login to Vault or our token is no longer valid we can
overwhelm a Vault instance with many requests very quickly by restarting
the LifetimeWatcher. Before restarting the LifetimeWatcher provide a
backoff time of 1 second or less.

* Use a retry.Waiter instead of RandomStagger

* changelog

* gofmt'd

* Swap out bool for atomic.Unit32 in test

* Provide some extra clarification in comment and changelog
2022-03-28 09:58:16 -05:00
Daniel Nephin fa8ff28a63 ca/provider: remove ActiveRoot from Provider 2022-01-27 13:07:37 -05:00
R.B. Boyer 05c7373a28 bulk rewrite using this script
set -euo pipefail

    unset CDPATH

    cd "$(dirname "$0")"

    for f in $(git grep '\brequire := require\.New(' | cut -d':' -f1 | sort -u); do
        echo "=== require: $f ==="
        sed -i '/require := require.New(t)/d' $f
        # require.XXX(blah) but not require.XXX(tblah) or require.XXX(rblah)
        sed -i 's/\brequire\.\([a-zA-Z0-9_]*\)(\([^tr]\)/require.\1(t,\2/g' $f
        # require.XXX(tblah) but not require.XXX(t, blah)
        sed -i 's/\brequire\.\([a-zA-Z0-9_]*\)(\(t[^,]\)/require.\1(t,\2/g' $f
        # require.XXX(rblah) but not require.XXX(r, blah)
        sed -i 's/\brequire\.\([a-zA-Z0-9_]*\)(\(r[^,]\)/require.\1(t,\2/g' $f
        gofmt -s -w $f
    done

    for f in $(git grep '\bassert := assert\.New(' | cut -d':' -f1 | sort -u); do
        echo "=== assert: $f ==="
        sed -i '/assert := assert.New(t)/d' $f
        # assert.XXX(blah) but not assert.XXX(tblah) or assert.XXX(rblah)
        sed -i 's/\bassert\.\([a-zA-Z0-9_]*\)(\([^tr]\)/assert.\1(t,\2/g' $f
        # assert.XXX(tblah) but not assert.XXX(t, blah)
        sed -i 's/\bassert\.\([a-zA-Z0-9_]*\)(\(t[^,]\)/assert.\1(t,\2/g' $f
        # assert.XXX(rblah) but not assert.XXX(r, blah)
        sed -i 's/\bassert\.\([a-zA-Z0-9_]*\)(\(r[^,]\)/assert.\1(t,\2/g' $f
        gofmt -s -w $f
    done
2022-01-20 10:46:23 -06:00
Iryna Shustava bd3fb0d0e9
connect: Support auth methods for the vault connect CA provider (#11573)
* Support vault auth methods for the Vault connect CA provider
* Rotate the token (re-authenticate to vault using auth method) when the token can no longer be renewed
2021-11-18 13:15:28 -07:00
Connor b3af482e09
Support Vault Namespaces explicitly in CA config (#11477)
* Support Vault Namespaces explicitly in CA config

If there is a Namespace entry included in the Vault CA configuration,
set it as the Vault Namespace on the Vault client

Currently the only way to support Vault namespaces in the Consul CA
config is by doing one of the following:
1) Set the VAULT_NAMESPACE environment variable which will be picked up
by the Vault API client
2) Prefix all Vault paths with the namespace

Neither of these are super pleasant. The first requires direct access
and modification to the Consul runtime environment. It's possible and
expected, not super pleasant.

The second requires more indepth knowledge of Vault and how it uses
Namespaces and could be confusing for anyone without that context. It
also infers that it is not supported

* Add changelog

* Remove fmt.Fprint calls

* Make comment clearer

* Add next consul version to website docs

* Add new test for default configuration

* go mod tidy

* Add skip if vault not present

* Tweak changelog text
2021-11-05 11:42:28 -05:00
FFMMM 27227c0fd2
add root_cert_ttl option for consul connect, vault ca providers (#11428)
* add root_cert_ttl option for consul connect, vault ca providers

Signed-off-by: FFMMM <FFMMM@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: Chris S. Kim <ckim@hashicorp.com>

* add changelog, pr feedback

Signed-off-by: FFMMM <FFMMM@users.noreply.github.com>

* Update .changelog/11428.txt, more docs

Co-authored-by: Daniel Nephin <dnephin@hashicorp.com>

* Update website/content/docs/agent/options.mdx

Co-authored-by: Kyle Havlovitz <kylehav@gmail.com>

Co-authored-by: Chris S. Kim <ckim@hashicorp.com>
Co-authored-by: Daniel Nephin <dnephin@hashicorp.com>
Co-authored-by: Kyle Havlovitz <kylehav@gmail.com>
2021-11-02 11:02:10 -07:00
Daniel Nephin d4bb9fd97a ca: move provider creation into CAManager
This further decouples the CAManager from Server. It reduces the interface between them and
removes the need for the SetLogger method on providers.
2021-07-12 09:32:33 -04:00
Dhia Ayachi b57cf27e8f
Format certificates properly (rfc7468) with a trailing new line (#10411)
* trim carriage return from certificates when inserting rootCA in the inMemDB

* format rootCA properly when returning the CA on the connect CA endpoint

* Fix linter warnings

* Fix providers to trim certs before returning it

* trim newlines on write when possible

* add changelog

* make sure all provider return a trailing newline after the root and intermediate certs

* Fix endpoint to return trailing new line

* Fix failing test with vault provider

* make test more robust

* make sure all provider return a trailing newline after the leaf certs

* Check for suffix before removing newline and use function

* Add comment to consul provider

* Update change log

Co-authored-by: R.B. Boyer <4903+rboyer@users.noreply.github.com>

* fix typo

* simplify code callflow

Co-authored-by: R.B. Boyer <4903+rboyer@users.noreply.github.com>

* extract requireNewLine as shared func

* remove dependency to testify in testing file

* remove extra newline in vault provider

* Add cert newline fix to envoy xds

* remove new line from mock provider

* Remove adding a new line from provider and fix it when the cert is read

* Add a comment to explain the fix

* Add missing for leaf certs

* fix missing new line

* fix missing new line in leaf certs

* remove extra new line in test

* updage changelog

Co-authored-by: Daniel Nephin <dnephin@hashicorp.com>

* fix in vault provider and when reading cache (RPC call)

* fix AWS provider

* fix failing test in the provider

* remove comments and empty lines

* add check for empty cert in test

* fix linter warnings

* add new line for leaf and private key

* use string concat instead of Sprintf

* fix new lines for leaf signing

* preallocate slice and remove append

* Add new line to `SignIntermediate` and `CrossSignCA`

Co-authored-by: R.B. Boyer <4903+rboyer@users.noreply.github.com>
Co-authored-by: Daniel Nephin <dnephin@hashicorp.com>
2021-06-30 20:48:29 -04:00
Matt Keeler 2d2ce1fb0c
Ensure that CA initialization does not block leader election.
After fixing that bug I uncovered a couple more:

Fix an issue where we might try to cross sign a cert when we never had a valid root.
Fix a potential issue where reconfiguring the CA could cause either the Vault or AWS PCA CA providers to delete resources that are still required by the new incarnation of the CA.
2021-01-19 15:27:48 -05:00
Kyle Havlovitz 968fd8660d Update CI for leader renew CA test using Vault 2020-10-09 05:48:15 -07:00
Kyle Havlovitz 2956313f2d connect: Enable renewing the intermediate cert in the primary DC 2020-09-30 12:31:21 -07:00
Kyle Havlovitz 1595add842 Clean up Vault renew tests and shutdown 2020-09-11 08:41:05 -07:00
Kyle Havlovitz 1c57b72a9f Add a test for token renewal 2020-09-09 16:36:37 -07:00
Hans Hasselberg 681124eaff
Setup intermediate_pki_path on secondary when using vault (#8001)
Make sure to mount vault backend for intermediate_pki_path on secondary
dc.
2020-06-05 21:36:22 +02:00
Daniel Nephin 8b6e07d960 ci: Run all connect/ca tests from the integration suite
To reduce the chance of some tests not being run because it does not
match the regex passed to '-run'.

Also document why some tests are allowed to be skipped on CI.
2020-03-24 15:22:01 -04:00
Daniel Nephin dc983db333 ci: Do not skip tests because of missing binaries on CI
If the CI environment is not correct for running tests the tests
should fail, so that we don't accidentally stop running some tests
because of a change to our CI environment.

Also removed a duplicate delcaration from init. I believe one was
overriding the other as they are both in the same package.
2020-03-24 14:34:13 -04:00
R.B. Boyer 791f7baa6b
test: ensure we don't ask vault to sign a leaf that outlives its CA when acting as a secondary (#7100) 2020-01-21 14:55:21 -06:00
Paul Banks 9e17aa3b41
Change CA Configure struct to pass Datacenter through (#6775)
* Change CA Configure struct to pass Datacenter through

* Remove connect/ca/plugin as we don't have immediate plans to use it.

We still intend to one day but there are likely to be several changes to the CA provider interface before we do so it's better to rebuild from history when we do that work properly.

* Rename PrimaryDC; fix endpoint in secondary DCs
2019-11-18 14:22:19 +00:00
Paul Banks ca96d5fa72
connect: Allow CA Providers to store small amount of state (#6751)
* pass logger through to provider

* test for proper operation of NeedsLogger

* remove public testServer function

* Ooops actually set the logger in all the places we need it - CA config set wasn't and causing segfault

* Fix all the other places in tests where we set the logger

* Allow CA Providers to persist some state

* Update CA provider plugin interface

* Fix plugin stubs to match provider changes

* Update agent/connect/ca/provider.go

Co-Authored-By: R.B. Boyer <rb@hashicorp.com>

* Cleanup review comments
2019-11-11 20:57:16 +00:00
Todd Radel 19a3892f71 connect: Implement NeedsLogger interface for CA providers (#6556)
* add NeedsLogger to Provider interface

* implements NeedsLogger in default provider

* pass logger through to provider

* test for proper operation of NeedsLogger

* remove public testServer function

* Switch test to actually assert on logging output rather than reflection.

--amend

* Ooops actually set the logger in all the places we need it - CA config set wasn't and causing segfault

* Fix all the other places in tests where we set the logger

* Add TODO comment
2019-11-11 20:30:01 +00:00
Todd Radel e100fda218 Make all Connect Cert Common Names valid FQDNs (#6423) 2019-11-11 17:11:54 +00:00
Paul Banks 5f405c3277
Fix support for RSA CA keys in Connect. (#6638)
* Allow RSA CA certs for consul and vault providers to correctly sign EC leaf certs.

* Ensure key type ad bits are populated from CA cert and clean up tests

* Add integration test and fix error when initializing secondary CA with RSA key.

* Add more tests, fix review feedback

* Update docs with key type config and output

* Apply suggestions from code review

Co-Authored-By: R.B. Boyer <rb@hashicorp.com>
2019-11-01 13:20:26 +00:00
R.B. Boyer 1d54909333
connect: intermediate CA certs generated with the vault provider lack URI SANs (#6491)
This only affects vault versions >=1.1.1 because the prior code
accidentally relied upon a bug that was fixed in
https://github.com/hashicorp/vault/pull/6505

The existing tests should have caught this, but they were using a
vendored copy of vault version 0.10.3. This fixes the tests by running
an actual copy of vault instead of an in-process copy. This has the
added benefit of changing the dependency on vault to just vault/api.

Also update VaultProvider to use similar SetIntermediate validation code
as the ConsulProvider implementation.
2019-09-23 12:04:40 -05:00
Alvin Huang e4e9381851
revert commits on master (#6413) 2019-08-27 17:45:58 -04:00
tradel 82544b64e5 add subject names to issued certs 2019-08-27 14:15:10 -07:00
Christian Muehlhaeuser 2602f6907e Simplified code in various places (#6176)
All these changes should have no side-effects or change behavior:

- Use bytes.Buffer's String() instead of a conversion
- Use time.Since and time.Until where fitting
- Drop unnecessary returns and assignment
2019-07-20 09:37:19 -04:00
R.B. Boyer 23f824fb60 test: fix concurrent map access when setting up test vault 2019-03-01 14:30:19 -06:00
Hans Hasselberg 092907077d
connect: add tls config for vault connect ca provider (#5125)
* add tlsconfig for vault connect ca provider.
* add options to the docs
* add tests for new configuration
2019-01-08 17:09:22 +01:00
Kyle Havlovitz 6301f763df connect/ca: tighten up the intermediate signing verification 2018-09-14 16:08:54 -07:00
Kyle Havlovitz ba1d7201a0 connect/ca: add intermediate functions to Vault ca provider 2018-09-13 13:38:32 -07:00
Kyle Havlovitz 9b8f8975c6
Merge pull request #4644 from hashicorp/ca-refactor
connect/ca: rework initialization/root generation in providers
2018-09-13 13:08:34 -07:00
Paul Banks 09e4c2995b
Fix CA pruning when CA config uses string durations. (#4669)
* Fix CA pruning when CA config uses string durations.

The tl;dr here is:

 - Configuring LeafCertTTL with a string like "72h" is how we do it by default and should be supported
 - Most of our tests managed to escape this by defining them as time.Duration directly
 - Out actual default value is a string
 - Since this is stored in a map[string]interface{} config, when it is written to Raft it goes through a msgpack encode/decode cycle (even though it's written from server not over RPC).
 - msgpack decode leaves the string as a `[]uint8`
 - Some of our parsers required string and failed
 - So after 1 hour, a default configured server would throw an error about pruning old CAs
 - If a new CA was configured that set LeafCertTTL as a time.Duration, things might be OK after that, but if a new CA was just configured from config file, intialization would cause same issue but always fail still so would never prune the old CA.
 - Mostly this is just a janky error that got passed tests due to many levels of complicated encoding/decoding.

tl;dr of the tl;dr: Yay for type safety. Map[string]interface{} combined with msgpack always goes wrong but we somehow get bitten every time in a new way :D

We already fixed this once! The main CA config had the same problem so @kyhavlov already wrote the mapstructure DecodeHook that fixes it. It wasn't used in several places it needed to be and one of those is notw in `structs` which caused a dependency cycle so I've moved them.

This adds a whole new test thta explicitly tests the case that broke here. It also adds tests that would have failed in other places before (Consul and Vaul provider parsing functions). I'm not sure if they would ever be affected as it is now as we've not seen things broken with them but it seems better to explicitly test that and support it to not be bitten a third time!

* Typo fix

* Fix bad Uint8 usage
2018-09-13 15:43:00 +01:00
Kyle Havlovitz 8fc2c77fdf
connect/ca: some cleanup and reorganizing of the new methods 2018-09-11 16:43:04 -07:00
Kyle Havlovitz e184a18e4b
connect/ca: add Configure/GenerateRoot to provider interface 2018-09-06 19:18:59 -07:00
Kyle Havlovitz 68d7a9fbd3
connect/ca: simplify passing of leaf cert TTL 2018-07-25 17:51:45 -07:00
Kyle Havlovitz 45ec8849f3
connect/ca: add configurable leaf cert TTL 2018-07-16 13:33:37 -07:00
Kyle Havlovitz f3089a6647 connect/ca: undo the interface changes and use sign-self-issued in Vault 2018-06-25 12:25:42 -07:00
Kyle Havlovitz f79e3e3fa5 connect/ca: add leaf verify check to cross-signing tests 2018-06-25 12:25:41 -07:00
Kyle Havlovitz 675555c4ff connect/ca: update Vault provider to add cross-signing methods 2018-06-25 12:25:41 -07:00
Kyle Havlovitz a97c44c1ba connect/ca: add URI SAN support to the Vault provider 2018-06-25 12:25:41 -07:00
Kyle Havlovitz 7b0845ccde connect/ca: fix vault provider URI SANs and test 2018-06-25 12:25:41 -07:00
Kyle Havlovitz a98b85b25c connect/ca: add the Vault CA provider 2018-06-25 12:25:41 -07:00