223 Commits (18831-backport1.14.10)

Author	SHA1	Message	Date
Chris S. Kim	2766043527	[1.14.x] Vault CA provider clean up previous default issuers (#18773) (#18785) ... * Vault CA provider clean up previous default issuers (#18773) (cherry picked from commit 4dfca64ded)	1 year ago
Semir Patel	3c937f3d25	[BACKPORT] 1.14.x manual backport of OSS->CE branch (#18556)	1 year ago
John Murret	64f269b2c9	ci: remove test-integrations CircleCI workflow in 1.14 (#16928) (#17050) ... * ci: remove test-integrations CircleCI workflow (#16928) * remove all CircleCI files * remove references to CircleCI * remove more references to CircleCI * pin golangci-lint to v1.51.1 instead of v1.51 * right-sizing TOTAL_runners	2 years ago
hc-github-team-consul-core	3e3ab25f9b	xds: don't attempt to load-balance sessions for local proxies (#15789) (#16004) ... Previously, we'd begin a session with the xDS concurrency limiter regardless of whether the proxy was registered in the catalog or in the server's local agent state. This caused problems for users who run `consul connect envoy` directly against a server rather than a client agent, as the server's locally registered proxies wouldn't be included in the limiter's capacity. Now, the `ConfigSource` is responsible for beginning the session and we only do so for services in the catalog. Fixes: https://github.com/hashicorp/consul/issues/15753 Co-authored-by: Dan Upton <daniel@floppy.co>	2 years ago
hc-github-team-consul-core	afcffc2385	connect: use -dev-no-store-token for test vaults to reduce source of flakes (#15691) (#15694) ... It turns out that by default the dev mode vault server will attempt to interact with the filesystem to store the provided root token. If multiple vault instances are running they'll all awkwardly share the filesystem and if timing results in one server stopping while another one is starting then the starting one will error with: Error initializing Dev mode: rename /home/circleci/.vault-token.tmp /home/circleci/.vault-token: no such file or directory This change uses `-dev-no-store-token` to bypass that source of flakes. Also the stdout/stderr from the vault process is included if the test fails. The introduction of more `t.Parallel` use in https://github.com/hashicorp/consul/pull/15669 increased the likelihood of this failure, but any of the tests with multiple vaults in use (or running multiple package tests in parallel that all use vault) were eventually going to flake on this. Co-authored-by: R.B. Boyer <4903+rboyer@users.noreply.github.com>	2 years ago
hc-github-team-consul-core	8d4bcfb06f	connect: ensure all vault connect CA tests use limited privilege tokens (#15689) ... All of the current integration tests where Vault is the Connect CA now use non-root tokens for the test. This helps us detect privilege changes in the vault model so we can keep our guides up to date. One larger change was that the RenewIntermediate function got refactored slightly so it could be used from a test, rather than the large duplicated function we were testing in a test which seemed error prone. Co-authored-by: R.B. Boyer <4903+rboyer@users.noreply.github.com>	2 years ago
hc-github-team-consul-core	9e25552415	Backport of Detect Vault 1.11+ import in secondary datacenters and update default issuer into release/1.14.x (#15683) ... * backport of commit 97fcd595d4 * backport of commit fc847e4edf * backport of commit 0207f1d54c * backport of commit 38f1824817 * backport of commit 51f8e56fe0 * backport of commit 4c7c84292d * backport of commit 5a2a1e5f13 * backport of commit e3b26c40b5 * backport of commit 0bf754af30 Co-authored-by: R.B. Boyer <rb@hashicorp.com>	2 years ago
hc-github-team-consul-core	3a03f2697e	Backport of Fix Vault managed intermediate PKI bug into release/1.14.x (#15579)	2 years ago
Chris S. Kim	84838e57f0	Detect Vault 1.11+ import, update default issuer (#15253) (#15437) ... Consul used to rely on implicit issuer selection when calling Vault endpoints to issue new CSRs. Vault 1.11+ changed that behavior, which caused Consul to check the wrong (previous) issuer when renewing its Intermediate CA. This patch allows Consul to explicitly set a default issuer when it detects that the response from Vault is 1.11+. Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> Co-authored-by: Chris S. Kim <ckim@hashicorp.com>	2 years ago
hc-github-team-consul-core	904aaf742d	Backport of connect: strip port from DNS SANs for ingress gateway leaf cert into release/1.14.x (#15354) ... This pull request was automerged via backport-assistant	2 years ago
hc-github-team-consul-core	35fb3cb433	Backport of Update go version to 1.19 into release/1.14.x (#15139) ... This pull request was automerged via backport-assistant	2 years ago
Kyle Havlovitz	d122108992	Warn instead of returning an error when intermediate mount tune permission is missing	2 years ago
freddygv	fac3ddc857	Use internal server certificate for peering TLS ... A previous commit introduced an internally-managed server certificate to use for peering-related purposes. Now the peering token has been updated to match that behavior: - The server name matches the structure of the server cert - The CA PEMs correspond to the Connect CA Note that if Conect is disabled, and by extension the Connect CA, we fall back to the previous behavior of returning the manually configured certs and local server SNI. Several tests were updated to use the gRPC TLS port since they enable Connect by default. This means that the peering token will embed the Connect CA, and the dialer will expect a TLS listener.	2 years ago
freddygv	0ea3353537	Add handling in agent cache for server leaf certs	2 years ago
Kyle Havlovitz	d67bccd210	Update intermediate pki mount/role when reconfiguring Vault provider	2 years ago
Freddy	f4dfd42e0a	Add SpiffeID for Consul server agents (#14485) ... Co-authored-by: Eric Haberkorn <erichaberkorn@gmail.com> By adding a SpiffeID for server agents, servers can now request a leaf certificate from the Connect CA. This new Spiffe ID has a key property: servers are identified by their datacenter name and trust domain. All servers that share these attributes will share a ServerURI. The aim is to use these certificates to verify the server name of ANY server in a Consul datacenter.	2 years ago
Eric Haberkorn	58901ad7df	Cluster peering failover disco chain changes (#14296)	2 years ago
R.B. Boyer	201d1458c3	xds: mesh gateways now have their own leaf certificate when involved in a peering (#13460) ... This is only configured in xDS when a service with an L7 protocol is exported. They also load any relevant trust bundles for the peered services to eventually use for L7 SPIFFE validation during mTLS termination.	2 years ago
Chris S. Kim	a02e9abcc1	Update RBAC to handle imported services (#13404) ... When converting from Consul intentions to xds RBAC rules, services imported from other peers must encode additional data like partition (from the remote cluster) and trust domain. This PR updates the PeeringTrustBundle to hold the sending side's local partition as ExportedPartition. It also updates RBAC code to encode SpiffeIDs of imported services with the ExportedPartition and TrustDomain.	3 years ago
Freddy	74ca6406ea	Configure upstream TLS context with peer root certs (#13321) ... For mTLS to work between two proxies in peered clusters with different root CAs, proxies need to configure their outbound listener to use different root certificates for validation. Up until peering was introduced proxies would only ever use one set of root certificates to validate all mesh traffic, both inbound and outbound. Now an upstream proxy may have a leaf certificate signed by a CA that's different from the dialing proxy's. This PR makes changes to proxycfg and xds so that the upstream TLS validation uses different root certificates depending on which cluster is being dialed.	3 years ago
R.B. Boyer	1a8834e1c8	peering: replicate expected SNI, SPIFFE, and service protocol to peers (#13218) ... The importing peer will need to know what SNI and SPIFFE name corresponds to each exported service. Additionally it will need to know at a high level the protocol in use (L4/L7) to generate the appropriate connection pool and local metrics. For replicated connect synthetic entities we edit the `Connect{}` part of a `NodeService` to have a new section: { "PeerMeta": { "SNI": [ "web.default.default.owt.external.183150d5-1033-3672-c426-c29205a576b8.consul" ], "SpiffeID": [ "spiffe://183150d5-1033-3672-c426-c29205a576b8.consul/ns/default/dc/dc1/svc/web" ], "Protocol": "tcp" } } This data is then replicated and saved as-is at the importing side. Both SNI and SpiffeID are slices for now until I can be sure we don't need them for how mesh gateways will ultimately work.	3 years ago
Mark Anderson	c6ff4ba7d8	Support vault namespaces in connect CA (#12904) ... * Support vault namespaces in connect CA Follow on to some missed items from #12655 From an internal ticket "Support standard "Vault namespace in the path" semantics for Connect Vault CA Provider" Vault allows the namespace to be specified as a prefix in the path of a PKI definition, but our usage of the Vault API includes calls that don't support a namespaced key. In particular the sys.* family of calls simply appends the key, instead of prefixing the namespace in front of the path. Unfortunately it is difficult to reliably parse a path with a namespace; only vault knows what namespaces are present, and the '/' separator can be inside a key name, as well as separating path elements. This is in use in the wild; for example 'dc1/intermediate-key' is a relatively common naming schema. Instead we add two new fields: RootPKINamespace and IntermediatePKINamespace, which are the absolute namespace paths 'prefixed' in front of the respective PKI Paths. Signed-off-by: Mark Anderson <manderson@hashicorp.com>	3 years ago
Chris S. Kim	9791bad136	peering: Make Upstream peer-aware (#12900) ... Adds DestinationPeer field to Upstream. Adds Peer field to UpstreamID and its string conversion functions.	3 years ago
R.B. Boyer	4274e67b47	chore: upgrade mockery to v2 and regenerate (#12836)	3 years ago
John Murret	a1117261df	set vault namespaces on vault client prior to logging in with the vault auth method	3 years ago
Dan Upton	325c1c0dd7	ConnectCA.Sign gRPC Endpoint (#12787) ... Introduces a gRPC endpoint for signing Connect leaf certificates. It's also the first of the public gRPC endpoints to perform leader-forwarding, so establishes the pattern of forwarding over the multiplexed internal RPC port.	3 years ago
Mark Anderson	98a2e282be	Fixup acl.EnterpriseMeta ... Signed-off-by: Mark Anderson <manderson@hashicorp.com>	3 years ago
Mark Anderson	018edc222e	Avoid using sys/mounts to enable namespaces (#12655) ... * Avoid doing list of /sys/mounts From an internal ticket "Support standard "Vault namespace in the path" semantics for Connect Vault CA Provider" Vault allows the namespace to be specified as a prefix in the path of a PKI definition, but this doesn't currently work for ```IntermediatePKIPath``` specifications, because we attempt to list all of the paths to check if ours is already defined. This doesn't really work in a namespaced world. This changes the IntermediatePKIPath code to follow the same pattern as the root key, where we directly get the key rather than listing. This code is difficult to write automated tests for because it relies on features of Vault Enterprise, which isn't currently part of our test framework, so it was tested manually. Signed-off-by: Mark Anderson <manderson@hashicorp.com> * add changelog Signed-off-by: Mark Anderson <manderson@hashicorp.com>	3 years ago
Connor	922619dfc3	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	3 years ago
Dhia Ayachi	72a997242b	split `pbcommon` to `pbcommon` and `pbcommongogo` (#12587) ... * mogify needed pbcommon structs * mogify needed pbconnect structs * fix compilation errors and make config_translate_test pass * add missing file * remove redundant oss func declaration * fix EnterpriseMeta to copy the right data for enterprise * rename pbcommon package to pbcommongogo * regenerate proto and mog files * add missing mog files * add pbcommon package * pbcommon no mog * fix enterprise meta code generation * fix enterprise meta code generation (pbcommongogo) * fix mog generation for gogo * use `protoc-go-inject-tag` to inject tags * rename proto package * pbcommon no mog * use `protoc-go-inject-tag` to inject tags * add non gogo proto to make file * fix proto get	3 years ago
Daniel Nephin	1f00ede559	ca: require that tests that use Vault are named correctly ... Previously we were using two different criteria to decide where to run a test. The main `go-test` job would skip Vault tests based on the presence of the `vault` binary, but the `test-connect-ca-providers` job would run tests based on the name. This led to a scenario where a test may never run in CI. To fix this problem I added a name check to the function we use to skip the test. This should ensure that any test that requires vault is named correctly to be run as part of the `test-connect-ca-providers` job. At the same time I relaxed the regex we use. I verified this runs the same tests using `go test --list Vault`. I made this change because a bunch of tests in `agent/connect/ca` used `Vault` in the name, without the underscores. Instead of changing a bunch of test names, this seemed easier. With this approach, the worst case is that we run a few extra tests in the `test-connect-ca-providers` job, which doesn't seem like a problem.	3 years ago
Daniel Nephin	6b679aa9d4	Update TODOs to reference an issue with more details ... And remove a no longer needed TODO	3 years ago
Daniel Nephin	5e8ea2a039	ca: add a test for secondary with external CA	3 years ago
Daniel Nephin	42ec34d101	ca: examine the full chain in newCARoot ... make TestNewCARoot much more strict compare the full result instead of only a few fields. add a test case with 2 and 3 certificates in the pem	3 years ago
Daniel Nephin	71f3ae04e2	ca: small docs improvements	3 years ago
Daniel Nephin	86994812ed	ca: cleanup validateSetIntermediate	3 years ago
Daniel Nephin	c1c1580bf8	ca: only return the leaf cert from Sign in vault provider ... The interface is documented as 'Sign will only return the leaf', and the other providers only return the leaf. It seems like this was added during the initial implementation, so is likely just something we missed. It doesn't break anything , but it does cause confusing cert chains in the API response which could break something in the future.	3 years ago
FFMMM	78264a8030	Vendor in rpc mono repo for net/rpc fork, go-msgpack, msgpackrpc. (#12311) ... This commit syncs ENT changes to the OSS repo. Original commit details in ENT: ``` commit 569d25f7f4578981c3801e6e067295668210f748 Author: FFMMM <FFMMM@users.noreply.github.com> Date: Thu Feb 10 10:23:33 2022 -0800 Vendor fork net rpc (#1538) * replace net/rpc w consul-net-rpc/net/rpc Signed-off-by: FFMMM <FFMMM@users.noreply.github.com> * replace msgpackrpc and go-msgpack with fork from mono repo Signed-off-by: FFMMM <FFMMM@users.noreply.github.com> * gofmt all files touched Signed-off-by: FFMMM <FFMMM@users.noreply.github.com> ``` Signed-off-by: FFMMM <FFMMM@users.noreply.github.com>	3 years ago
Daniel Nephin	51b0f82d0e	Make test more readable ... And fix typo	3 years ago
Daniel Nephin	608597c7b6	ca: relax and move private key type/bit validation for vault ... This commit makes two changes to the validation. Previously we would call this validation in GenerateRoot, which happens both on initialization (when a follower becomes leader), and when a configuration is updated. We only want to do this validation during config update so the logic was moved to the UpdateConfiguration function. Previously we would compare the config values against the actual cert. This caused problems when the cert was created manually in Vault (not created by Consul). Now we compare the new config against the previous config. Using a already created CA cert should never error now. Adding the key bit and types to the config should only error when the previous values were not the defaults.	3 years ago
Daniel Nephin	7839b2d7e0	ca: add a test that uses an intermediate CA as the primary CA ... This test found a bug in the secondary. We were appending the root cert to the PEM, but that cert was already appended. This was failing validation in Vault here: https://github.com/hashicorp/vault/blob/sdk/v0.3.0/sdk/helper/certutil/types.go#L329 Previously this worked because self signed certs have the same SubjectKeyID and AuthorityKeyID. So having the same self-signed cert repeated doesn't fail that check. However with an intermediate that is not self-signed, those values are different, and so we fail the check. A test I added in a previous commit should show that this continues to work with self-signed root certs as well.	3 years ago
Daniel Nephin	9b7468f99e	ca/provider: remove ActiveRoot from Provider	3 years ago
Daniel Nephin	c2b9c81a55	ca: update MockProvider for new interface	3 years ago
Daniel Nephin	f05bad4a1d	ca: update GenerateRoot godoc	3 years ago
R.B. Boyer	b60d89e7ef	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	3 years ago
R.B. Boyer	31f6f55bbe	test: normalize require.New and assert.New syntax	3 years ago
Daniel Nephin	4116a143e0	fix misleading errors on vault shutdown	3 years ago
Daniel Nephin	214dcf8d0d	ca: use the real FSM operation in tests ... Previously we had a couple copies that reproduced the FSM operation. These copies introduce risk that the test does not accurately match production. This PR removes the test versions of the FSM operation, and exports the real production FSM operation so that it can be used in tests. The consul provider tests did need to change because of this. Previously we would return a hardcoded value of 2, but in production this value is always incremented.	3 years ago
Daniel Nephin	81afb208ac	Merge pull request #11677 from hashicorp/dnephin/freeport-interface ... sdk: use t.Cleanup in freeport and remove unnecessary calls	3 years ago
R.B. Boyer	db91cbf484	auto-config: ensure the feature works properly with partitions (#11699)	3 years ago