Commit Graph

116 Commits

Author SHA1 Message Date
Kyle Havlovitz 58172c260b xds: respect chain protocol on default discovery chain 2022-02-17 11:47:20 -08:00
freddygv 7fba7456ec Fix race of upstreams with same passthrough ip
Due to timing, a transparent proxy could have two upstreams to dial
directly with the same address.

For example:
- The orders service can dial upstreams shipping and payment directly.
- An instance of shipping at address 10.0.0.1 is deregistered.
- Payments is scaled up and scheduled to have address 10.0.0.1.
- The orders service receives the event for the new payments instance
before seeing the deregistration for the shipping instance. At this
point two upstreams have the same passthrough address and Envoy will
reject the listener configuration.

To disambiguate this commit considers the Raft index when storing
passthrough addresses. In the example above, 10.0.0.1 would only be
associated with the newer payments service instance.
2022-02-10 17:01:57 -07:00
freddygv d5a2eb677f Ensure passthrough addresses get cleaned up
Transparent proxies can set up filter chains that allow direct
connections to upstream service instances. Services that can be dialed
directly are stored in the PassthroughUpstreams map of the proxycfg
snapshot.

Previously these addresses were not being cleaned up based on new
service health data. The list of addresses associated with an upstream
service would only ever grow.

As services scale up and down, eventually they will have instances
assigned to an IP that was previously assigned to a different service.
When IP addresses are duplicated across filter chain match rules the
listener config will be rejected by Envoy.

This commit updates the proxycfg snapshot management so that passthrough
addresses can get cleaned up when no longer associated with a given
upstream.

There is still the possibility of a race condition here where due to
timing an address is shared between multiple passthrough upstreams.
That concern is mitigated by #12195, but will be further addressed
in a follow-up.
2022-02-10 17:01:57 -07:00
R.B. Boyer baf886c6f3
proxycfg: introduce explicit UpstreamID in lieu of bare string (#12125)
The gist here is that now we use a value-type struct proxycfg.UpstreamID
as the map key in ConfigSnapshot maps where we used to use "upstream
id-ish" strings. These are internal only and used just for bidirectional
trips through the agent cache keyspace (like the discovery chain target
struct).

For the few places where the upstream id needs to be projected into xDS,
that's what (proxycfg.UpstreamID).EnvoyID() is for. This lets us ALWAYS
inject the partition and namespace into these things without making
stuff like the golden testdata diverge.
2022-01-20 10:12:04 -06:00
Kyle Havlovitz 2ba76486d0 Add virtual IP generation for term gateway backed services 2022-01-12 12:08:49 -08:00
Mike Morris 277c41d336
ingress: allow setting TLS min version and cipher suites in ingress gateway config entries (#11576)
* xds: refactor ingress listener SDS configuration

* xds: update resolveListenerSDS call args in listeners_test

* ingress: add TLS min, max and cipher suites to GatewayTLSConfig

* xds: implement envoyTLSVersions and envoyTLSCipherSuites

* xds: merge TLS config

* xds: configure TLS parameters with ingress TLS context from leaf

* xds: nil check in resolveListenerTLSConfig validation

* xds: nil check in makeTLSParameters* functions

* changelog: add entry for TLS params on ingress config entries

* xds: remove indirection for TLS params in TLSConfig structs

* xds: return tlsContext, nil instead of ambiguous err

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

* xds: switch zero checks to types.TLSVersionUnspecified

* ingress: add validation for ingress config entry TLS params

* ingress: validate listener TLS config

* xds: add basic ingress with TLS params tests

* xds: add ingress listeners mixed TLS min version defaults precedence test

* xds: add more explicit tests for ingress listeners inheriting gateway defaults

* xds: add test for single TLS listener on gateway without TLS defaults

* xds: regen golden files for TLSVersionInvalid zero value, add TLSVersionAuto listener test

* types/tls: change TLSVersion to string

* types/tls: update TLSCipherSuite to string type

* types/tls: implement validation functions for TLSVersion and TLSCipherSuites, make some maps private

* api: add TLS params to GatewayTLSConfig, add tests

* api: add TLSMinVersion to ingress gateway config entry test JSON

* xds: switch to Envoy TLS cipher suite encoding from types package

* xds: fixup validation for TLSv1_3 min version with cipher suites

* add some kitchen sink tests and add a missing struct tag

* xds: check if mergedCfg.TLSVersion is in TLSVersionsWithConfigurableCipherSuites

* xds: update connectTLSEnabled comment

* xds: remove unsued resolveGatewayServiceTLSConfig function

 * xds: add makeCommonTLSContextFromLeafWithoutParams

* types/tls: add LessThan comparator function for concrete values

* types/tls: change tlsVersions validation map from string to TLSVersion keys

* types/tls: remove unused envoyTLSCipherSuites

* types/tls: enable chacha20 cipher suites for Consul agent

* types/tls: remove insecure cipher suites from allowed config

TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 and TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 are both explicitly listed as insecure and disabled in the Go source.

Refs https://cs.opensource.google/go/go/+/refs/tags/go1.17.3:src/crypto/tls/cipher_suites.go;l=329-330

* types/tls: add ValidateConsulAgentCipherSuites function, make direct lookup map private

* types/tls: return all unmatched cipher suites in validation errors

* xds: check that Envoy API value matching TLS version is found when building TlsParameters

* types/tls: check that value is found in map before appending to slice in MarshalEnvoyTLSCipherSuiteStrings

* types/tls: cast to string rather than fmt.Printf in TLSCihperSuite.String()

* xds: add TLSVersionUnspecified to list of configurable cipher suites

* structs: update note about config entry warning

* xds: remove TLS min version cipher suite unconfigurable test placeholder

* types/tls: update tests to remove assumption about private map values

Co-authored-by: R.B. Boyer <rb@hashicorp.com>
2022-01-11 11:46:42 -05:00
freddygv b704d4e2dd Validate chains are associated with upstreams
Previously we could get into a state where discovery chain entries were
not cleaned up after the associated watch was cancelled. These changes
add handling for that case where stray chain references are encountered.
2021-12-13 18:56:13 -07:00
freddygv 97b4068137 Update listener generation to account for consul VIP 2021-12-03 17:27:56 -07:00
Freddy eb2b40b22d
Update filter chain creation for sidecar/ingress listeners (#11245)
The duo of `makeUpstreamFilterChainForDiscoveryChain` and `makeListenerForDiscoveryChain` were really hard to reason about, and led to concealing a bug in their branching logic. There were several issues here:

- They tried to accomplish too much: determining filter name, cluster name, and whether RDS should be used. 
- They embedded logic to handle significantly different kinds of upstream listeners (passthrough, prepared query, typical services, and catch-all)
- They needed to coalesce different data sources (Upstream and CompiledDiscoveryChain)

Rather than handling all of those tasks inside of these functions, this PR pulls out the RDS/clusterName/filterName logic.

This refactor also fixed a bug with the handling of [UpstreamDefaults](https://www.consul.io/docs/connect/config-entries/service-defaults#defaults). These defaults get stored as UpstreamConfig in the proxy snapshot with a DestinationName of "*", since they apply to all upstreams. However, this wildcard destination name must not be used when creating the name of the associated upstream cluster. The coalescing logic in the original functions here was in some situations creating clusters with a `*.` prefix, which is not a valid destination.
2021-11-09 14:43:51 -07:00
freddygv 05f91bd2b8 Update comments 2021-10-27 12:36:44 -06:00
freddygv 0391a65772 Replace default partition check 2021-10-27 11:15:25 -06:00
freddygv ee45ac9dc5 PR comments 2021-10-27 11:15:25 -06:00
freddygv 8b5a9369eb Account for partitions in xds gen for mesh gw
This commit avoids skipping gateways in remote partitions of the local
DC when generating listeners/clusters/endpoints.
2021-10-27 11:15:25 -06:00
freddygv d1d513b1b3 Account for partition in SNI for gateways 2021-10-27 11:15:25 -06:00
freddygv 4f0432be5e Update xds pkg to account for GatewayKey 2021-10-27 09:03:56 -06:00
Paul Banks 5c8702b182 Add support for enabling connect-based ingress TLS per listener. 2021-10-19 20:58:28 +01:00
Paul Banks fe4f69613c Refactor Ingress-specific lister code to separate file 2021-09-23 10:13:19 +01:00
Paul Banks f4f0793a10 Minor PR typo and cleanup fixes 2021-09-23 10:13:19 +01:00
Paul Banks 15969327c0 Remove unused argument to fix lint error 2021-09-23 10:09:11 +01:00
Paul Banks 9422e4ebc7 Handle namespaces in route names correctly; add tests for enterprise 2021-09-23 10:09:11 +01:00
Paul Banks 8a4254a894 Update xDS Listeners with SDS support 2021-09-23 10:08:02 +01:00
Chris S. Kim d222f170a7
connect: Allow upstream listener escape hatch for prepared queries (#11109) 2021-09-22 15:27:10 -04:00
Dhia Ayachi 96d7842118
partition dicovery chains (#10983)
* partition dicovery chains

* fix default partition for OSS
2021-09-07 16:29:32 -04:00
Dhia Ayachi eb19271fd7
add partition to SNI when partition is non default (#10917) 2021-09-01 10:35:39 -04:00
Dhia Ayachi f766b6dff7
oss portion of ent #1069 (#10883) 2021-08-20 12:57:45 -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
Freddy 0d05cbb105
Update agent/xds/listeners.go
Co-authored-by: R.B. Boyer <4903+rboyer@users.noreply.github.com>
2021-06-15 14:09:26 -06:00
freddygv e07542211a Remove unused param 2021-06-15 11:19:45 -06:00
freddygv 450ac44126 Update ingress gateway stats labeling
In the absence of stats_tags to handle this pattern, when we pass
"ingress_upstream.$port" as the stat_prefix, Envoy splits up that prefix
and makes the port a part of the metric name.

For example:
- stat_prefix: ingress_upstream.8080

This leads to metric names like envoy_http_8080_no_route. Changing the
stat_prefix to ingress_upstream_80880 yields the expected metric names
such as envoy_http_no_route.

Note that we don't encode the destination's name/ns/dc in this
stat_prefix because for HTTP services ingress gateways use a single
filter chain. Only cluster metrics are available on a per-upstream
basis.
2021-06-15 08:52:18 -06:00
freddygv 5d7188e290 Update terminating gateway stats labeling
This change makes it so that the stat prefix for terminating gateways
matches that of connect proxies. By using the structure of
"upstream.svc.ns.dc" we can extract labels for the destination service,
namespace, and datacenter.
2021-06-15 08:52:18 -06:00
R.B. Boyer 8d5f81b460
xds: ensure that dependent xDS resources are reconfigured during primary type warming (#10381)
Updates to a cluster will clear the associated endpoints, and updates to
a listener will clear the associated routes. Update the incremental xDS
logic to account for this implicit cleanup so that we can finish warming
the clusters and listeners.

Fixes #10379
2021-06-14 17:20:27 -05:00
Freddy f399fd2add
Rename CatalogDestinationsOnly (#10397)
CatalogDestinationsOnly is a passthrough that would enable dialing
addresses outside of Consul's catalog. However, when this flag is set to
true only _connect_ endpoints for services can be dialed.

This flag is being renamed to signal that non-Connect endpoints can't be
dialed by transparent proxies when the value is set to true.
2021-06-14 14:15:09 -06:00
Freddy 61ae2995b7
Add flag for transparent proxies to dial individual instances (#10329) 2021-06-09 14:34:17 -06:00
Freddy 62facc1a04
Revert "Avoid adding original_dst filter when not needed" (#10365) 2021-06-08 13:18:41 -06:00
Freddy 7cfd7e9ec1
Avoid adding original_dst filter when not needed (#10302) 2021-05-26 15:04:45 -06:00
Mark Anderson 5f04b6abe2 Convert mode to string representation
Signed-off-by: Mark Anderson <manderson@hashicorp.com>
2021-05-04 12:41:43 -07:00
Mark Anderson 626b27a874 Continue working through proxy and agent
Rework/listeners, rename makeListener

Refactor, tests pass

Signed-off-by: Mark Anderson <manderson@hashicorp.com>
2021-05-04 12:41:43 -07:00
Freddy ec38cf3206
Fixup discovery chain handling in transparent mode (#10168)
Co-authored-by: R.B. Boyer <4903+rboyer@users.noreply.github.com>

Previously we would associate the address of a discovery chain target
with the discovery chain's filter chain. This was broken for a few reasons:

- If the upstream is a virtual service, the client proxy has no way of
dialing it because virtual services are not targets of their discovery
chains. The targets are distinct services. This is addressed by watching
the endpoints of all upstream services, not just their discovery chain
targets.

- If multiple discovery chains resolve to the same target, that would
lead to multiple filter chains attempting to match on the target's
virtual IP. This is addressed by only matching on the upstream's virtual
IP.

NOTE: this implementation requires an intention to the redirecting
virtual service and not just to the final destination. This is how
we can know that the virtual service is an upstream to watch.

A later PR will look into traversing discovery chains when computing
upstreams so that intentions are only required to the discovery chain
targets.
2021-05-04 08:45:19 -06:00
Freddy 5427a1465c
Only consider virtual IPs for transparent proxies (#10162)
Initially we were loading every potential upstream address into Envoy
and then routing traffic to the logical upstream service. The downside
of this behavior is that traffic meant to go to a specific instance
would be load balanced across ALL instances.

Traffic to specific instance IPs should be forwarded to the original
destination and if it's a destination in the mesh then we should ensure
the appropriate certificates are used.

This PR makes transparent proxying a Kubernetes-only feature for now
since support for other environments requires generating virtual IPs,
and Consul does not do that at the moment.
2021-05-03 14:15:22 -06:00
R.B. Boyer 97e57aedfb
connect: update supported envoy versions to 1.18.2, 1.17.2, 1.16.3, and 1.15.4 (#10101)
The only thing that needed fixing up pertained to this section of the 1.18.x release notes:

> grpc_stats: the default value for stats_for_all_methods is switched from true to false, in order to avoid possible memory exhaustion due to an untrusted downstream sending a large number of unique method names. The previous default value was deprecated in version 1.14.0. This only changes the behavior when the value is not set. The previous behavior can be used by setting the value to true. This behavior change by be overridden by setting runtime feature envoy.deprecated_features.grpc_stats_filter_enable_stats_for_all_methods_by_default.

For now to maintain status-quo I'm explicitly setting `stats_for_all_methods=true` in all versions to avoid relying upon the default.

Additionally the naming of the emitted metrics for these gRPC requests changed slightly so the integration test assertions for `case-grpc` needed adjusting.
2021-04-29 15:22:03 -05:00
R.B. Boyer 91bee6246f
Support Incremental xDS mode (#9855)
This adds support for the Incremental xDS protocol when using xDS v3. This is best reviewed commit-by-commit and will not be squashed when merged.

Union of all commit messages follows to give an overarching summary:

xds: exclusively support incremental xDS when using xDS v3

Attempts to use SoTW via v3 will fail, much like attempts to use incremental via v2 will fail.
Work around a strange older envoy behavior involving empty CDS responses over incremental xDS.
xds: various cleanups and refactors that don't strictly concern the addition of incremental xDS support

Dissolve the connectionInfo struct in favor of per-connection ResourceGenerators instead.
Do a better job of ensuring the xds code uses a well configured logger that accurately describes the connected client.
xds: pull out checkStreamACLs method in advance of a later commit

xds: rewrite SoTW xDS protocol tests to use protobufs rather than hand-rolled json strings

In the test we very lightly reuse some of the more boring protobuf construction helper code that is also technically under test. The important thing of the protocol tests is testing the protocol. The actual inputs and outputs are largely already handled by the xds golden output tests now so these protocol tests don't have to do double-duty.

This also updates the SoTW protocol test to exclusively use xDS v2 which is the only variant of SoTW that will be supported in Consul 1.10.

xds: default xds.Server.AuthCheckFrequency at use-time instead of construction-time
2021-04-29 13:54:05 -05:00
Freddy 401f3010e0
Rename "cluster" config entry to "mesh" (#10127)
This config entry is being renamed primarily because in k8s the name
cluster could be confusing given that the config entry applies across
federated datacenters.

Additionally, this config entry will only apply to Consul as a service
mesh, so the more generic "cluster" name is not needed.
2021-04-28 16:13:29 -06:00
Freddy 57b998e027
Merge pull request #9987 from hashicorp/remove-kube-dns-hack 2021-04-14 10:00:53 -06:00
freddygv 911d7dcaa8 Remove todo that was todone 2021-04-13 10:19:59 -06:00
freddygv eeccba945d Replace TransparentProxy bool with ProxyMode
This PR replaces the original boolean used to configure transparent
proxy mode. It was replaced with a string mode that can be set to:

- "": Empty string is the default for when the setting should be
defaulted from other configuration like config entries.
- "direct": Direct mode is how applications originally opted into the
mesh. Proxy listeners need to be dialed directly.
- "transparent": Transparent mode enables configuring Envoy as a
transparent proxy. Traffic must be captured and redirected to the
inbound and outbound listeners.

This PR also adds a struct for transparent proxy specific configuration.
Initially this is not stored as a pointer. Will revisit that decision
before GA.
2021-04-12 09:35:14 -06:00
Iryna Shustava ff2e70f4ce
cli: Add new `consul connect redirect-traffic` command for applying traffic redirection rules when Transparent Proxy is enabled. (#9910)
* Add new consul connect redirect-traffic command for applying traffic redirection rules when Transparent Proxy is enabled.
* Add new iptables package for applying traffic redirection rules with iptables.
2021-04-09 11:48:10 -07:00
freddygv b97d3422a7 Stable sort cidr ranges to match on 2021-04-08 11:27:57 -06:00
freddygv 69822fa5ae Remove kube-dns resolution since clusterip will be a tagged addr 2021-04-07 14:15:21 -06:00
R.B. Boyer 82245585c6
connect: add toggle to globally disable wildcard outbound network access when transparent proxy is enabled (#9973)
This adds a new config entry kind "cluster" with a single special name "cluster" where this can be controlled.
2021-04-06 13:19:59 -05:00
freddygv 6c43195e2a Merge master and fix upstream config protocol defaulting 2021-03-17 21:13:40 -06:00