Commit Graph

3439 Commits

Author SHA1 Message Date
Freddy cf4821885d
Avoid panic on concurrent writes to cached service config map (#10647)
If multiple instances of a service are co-located on the same node then
their proxies will all share a cache entry for their resolved service
configuration. This is because the cache key contains the name of the
watched service but does not take into account the ID of the watching
proxies.

This means that there will be multiple agent service manager watches
that can wake up on the same cache update. These watchers then
concurrently modify the value in the cache when merging the resolved
config into the local proxy definitions.

To avoid this concurrent map write we will only delete the key from
opaque config in the local proxy definition after the merge, rather
than from the cached value before the merge.
2021-07-20 10:09:29 -06:00
hc-github-team-consul-core 139717d3f8 auto-updated agent/uiserver/bindata_assetfs.go from commit 1eb7a83ee 2021-07-20 15:15:10 +00:00
Blake Covarrubias a0cd3dd88e
Add DNS recursor strategy option (#10611)
This change adds a new `dns_config.recursor_strategy` option which
controls how Consul queries DNS resolvers listed in the `recursors`
config option. The supported options are `sequential` (default), and
`random`.

Closes #8807

Co-authored-by: Blake Covarrubias <blake@covarrubi.as>
Co-authored-by: Priyanka Sengupta <psengupta@flatiron.com>
2021-07-19 15:22:51 -07:00
Daniel Nephin 499250cbf1
Merge pull request #10396 from hashicorp/dnephin/fix-more-data-races
Fix some data races
2021-07-16 18:21:58 -04:00
Daniel Nephin 1c8ac9cd4b
Merge pull request #10009 from hashicorp/dnephin/trim-dns-response-with-edns
dns: properly trim response when EDNS is used
2021-07-16 18:09:25 -04:00
Daniel Nephin a77575e93e acl: use SetHash consistently in testPolicyForID
A previous commit used SetHash on two of the cases to fix a data race. This commit applies
that change to all cases. Using SetHash in this test helper should ensure that the
test helper behaves closer to production.
2021-07-16 17:59:56 -04:00
Daniel Nephin 4bf58d8e6a dns: improve naming of error to match DNS terminology
Co-authored-by: Blake Covarrubias <blake@covarrubi.as>
2021-07-16 12:40:24 -04:00
Dhia Ayachi f0cd1441a9 fix truncate when NS is set
Also: fix test to catch the issue
2021-07-16 12:40:11 -04:00
Evan Culver 0527dcff57
acls: Show `AuthMethodNamespace` when reading/listing ACL token meta (#10598) 2021-07-15 10:38:52 -07:00
Daniel Nephin bb675139c1
Merge pull request #10567 from hashicorp/dnephin/config-unexport-build
config: unexport the remaining builder methods
2021-07-15 12:05:19 -04:00
Freddy 12b7e07d5c
Merge pull request #10621 from hashicorp/vuln/validate-sans 2021-07-15 09:43:55 -06:00
Daniel Nephin bb7fb21004
Fix godoc comment
Co-authored-by: Freddy <freddygv@users.noreply.github.com>
2021-07-15 11:22:46 -04:00
R.B. Boyer 20feb42d3a
xds: ensure single L7 deny intention with default deny policy does not result in allow action (CVE-2021-36213) (#10619) 2021-07-15 10:09:00 -05:00
hc-github-team-consul-core 58807668bd auto-updated agent/uiserver/bindata_assetfs.go from commit 0762da3a6 2021-07-15 11:23:49 +00:00
freddygv b4c5c58c9b Add TODOs about partition handling 2021-07-14 22:21:55 -06:00
freddygv 5a82656510 Update golden files 2021-07-14 22:21:55 -06:00
freddygv 47da00d3c7 Validate SANs for passthrough clusters and failovers 2021-07-14 22:21:55 -06:00
freddygv 5454147c09 Update golden files to account for SAN validation 2021-07-14 22:21:55 -06:00
freddygv a6d3fe90b1 Validate Subject Alternative Name for upstreams
These changes ensure that the identity of services dialed is
cryptographically verified.

For all upstreams we validate against SPIFFE IDs in the format used by
Consul's service mesh:

spiffe://<trust-domain>/ns/<namespace>/dc/<datacenter>/svc/<service>
2021-07-14 22:20:27 -06:00
Daniel Nephin fa47c04065 Fix a data race in TestACLResolver_Client
By setting the hash when we create the policy.

```
WARNING: DATA RACE
Read at 0x00c0028b4b10 by goroutine 1182:
  github.com/hashicorp/consul/agent/structs.(*ACLPolicy).SetHash()
      /home/daniel/pers/code/consul/agent/structs/acl.go:701 +0x40d
  github.com/hashicorp/consul/agent/structs.ACLPolicies.resolveWithCache()
      /home/daniel/pers/code/consul/agent/structs/acl.go:779 +0xfe
  github.com/hashicorp/consul/agent/structs.ACLPolicies.Compile()
      /home/daniel/pers/code/consul/agent/structs/acl.go:809 +0xf1
  github.com/hashicorp/consul/agent/consul.(*ACLResolver).ResolveTokenToIdentityAndAuthorizer()
      /home/daniel/pers/code/consul/agent/consul/acl.go:1226 +0x6ef
  github.com/hashicorp/consul/agent/consul.resolveTokenAsync()
      /home/daniel/pers/code/consul/agent/consul/acl_test.go:66 +0x5c

Previous write at 0x00c0028b4b10 by goroutine 1509:
  github.com/hashicorp/consul/agent/structs.(*ACLPolicy).SetHash()
      /home/daniel/pers/code/consul/agent/structs/acl.go:730 +0x3a8
  github.com/hashicorp/consul/agent/structs.ACLPolicies.resolveWithCache()
      /home/daniel/pers/code/consul/agent/structs/acl.go:779 +0xfe
  github.com/hashicorp/consul/agent/structs.ACLPolicies.Compile()
      /home/daniel/pers/code/consul/agent/structs/acl.go:809 +0xf1
  github.com/hashicorp/consul/agent/consul.(*ACLResolver).ResolveTokenToIdentityAndAuthorizer()
      /home/daniel/pers/code/consul/agent/consul/acl.go:1226 +0x6ef
  github.com/hashicorp/consul/agent/consul.resolveTokenAsync()
      /home/daniel/pers/code/consul/agent/consul/acl_test.go:66 +0x5c

Goroutine 1182 (running) created at:
  github.com/hashicorp/consul/agent/consul.TestACLResolver_Client.func4()
      /home/daniel/pers/code/consul/agent/consul/acl_test.go:1669 +0x459
  testing.tRunner()
      /usr/lib/go/src/testing/testing.go:1193 +0x202

Goroutine 1509 (running) created at:
  github.com/hashicorp/consul/agent/consul.TestACLResolver_Client.func4()
      /home/daniel/pers/code/consul/agent/consul/acl_test.go:1668 +0x415
  testing.tRunner()
      /usr/lib/go/src/testing/testing.go:1193 +0x202
```
2021-07-14 18:58:16 -04:00
Daniel Nephin a0ca381037 agent: remove deprecated call in a test 2021-07-14 18:58:16 -04:00
Daniel Nephin 678014de1d agent: fix a data race in a test
The test was modifying a pointer to a struct that had been passed to
another goroutine. Instead create a new struct to modify.

```
WARNING: DATA RACE
Write at 0x00c01407c3c0 by goroutine 832:
  github.com/hashicorp/consul/agent.TestServiceManager_PersistService_API()
      /home/daniel/pers/code/consul/agent/service_manager_test.go:446 +0x1d86
  testing.tRunner()
      /usr/lib/go/src/testing/testing.go:1193 +0x202

Previous read at 0x00c01407c3c0 by goroutine 938:
  reflect.typedmemmove()
      /usr/lib/go/src/runtime/mbarrier.go:177 +0x0
  reflect.Value.Set()
      /usr/lib/go/src/reflect/value.go:1569 +0x13b
  github.com/mitchellh/copystructure.(*walker).Primitive()
      /home/daniel/go/pkg/mod/github.com/mitchellh/copystructure@v1.0.0/copystructure.go:289 +0x190
  github.com/mitchellh/reflectwalk.walkPrimitive()
      /home/daniel/go/pkg/mod/github.com/mitchellh/reflectwalk@v1.0.1/reflectwalk.go:252 +0x31b
  github.com/mitchellh/reflectwalk.walk()
      /home/daniel/go/pkg/mod/github.com/mitchellh/reflectwalk@v1.0.1/reflectwalk.go:179 +0x24d
  github.com/mitchellh/reflectwalk.walkStruct()
      /home/daniel/go/pkg/mod/github.com/mitchellh/reflectwalk@v1.0.1/reflectwalk.go:386 +0x4ec
  github.com/mitchellh/reflectwalk.walk()
      /home/daniel/go/pkg/mod/github.com/mitchellh/reflectwalk@v1.0.1/reflectwalk.go:188 +0x656
  github.com/mitchellh/reflectwalk.walkStruct()
      /home/daniel/go/pkg/mod/github.com/mitchellh/reflectwalk@v1.0.1/reflectwalk.go:386 +0x4ec
  github.com/mitchellh/reflectwalk.walk()
      /home/daniel/go/pkg/mod/github.com/mitchellh/reflectwalk@v1.0.1/reflectwalk.go:188 +0x656
  github.com/mitchellh/reflectwalk.Walk()
      /home/daniel/go/pkg/mod/github.com/mitchellh/reflectwalk@v1.0.1/reflectwalk.go:92 +0x164
  github.com/mitchellh/copystructure.Config.Copy()
      /home/daniel/go/pkg/mod/github.com/mitchellh/copystructure@v1.0.0/copystructure.go:69 +0xe7
  github.com/mitchellh/copystructure.Copy()
      /home/daniel/go/pkg/mod/github.com/mitchellh/copystructure@v1.0.0/copystructure.go:13 +0x84
  github.com/hashicorp/consul/agent.mergeServiceConfig()
      /home/daniel/pers/code/consul/agent/service_manager.go:362 +0x56
  github.com/hashicorp/consul/agent.(*serviceConfigWatch).handleUpdate()
      /home/daniel/pers/code/consul/agent/service_manager.go:279 +0x250
  github.com/hashicorp/consul/agent.(*serviceConfigWatch).runWatch()
      /home/daniel/pers/code/consul/agent/service_manager.go:246 +0x2d4

Goroutine 832 (running) created at:
  testing.(*T).Run()
      /usr/lib/go/src/testing/testing.go:1238 +0x5d7
  testing.runTests.func1()
      /usr/lib/go/src/testing/testing.go:1511 +0xa6
  testing.tRunner()
      /usr/lib/go/src/testing/testing.go:1193 +0x202
  testing.runTests()
      /usr/lib/go/src/testing/testing.go:1509 +0x612
  testing.(*M).Run()
      /usr/lib/go/src/testing/testing.go:1417 +0x3b3
  main.main()
      _testmain.go:1181 +0x236

Goroutine 938 (running) created at:
  github.com/hashicorp/consul/agent.(*serviceConfigWatch).start()
      /home/daniel/pers/code/consul/agent/service_manager.go:223 +0x4e4
  github.com/hashicorp/consul/agent.(*ServiceManager).AddService()
      /home/daniel/pers/code/consul/agent/service_manager.go:98 +0x344
  github.com/hashicorp/consul/agent.(*Agent).addServiceLocked()
      /home/daniel/pers/code/consul/agent/agent.go:1942 +0x2e4
  github.com/hashicorp/consul/agent.(*Agent).AddService()
      /home/daniel/pers/code/consul/agent/agent.go:1929 +0x337
  github.com/hashicorp/consul/agent.TestServiceManager_PersistService_API()
      /home/daniel/pers/code/consul/agent/service_manager_test.go:400 +0x17c4
  testing.tRunner()
      /usr/lib/go/src/testing/testing.go:1193 +0x202

```
2021-07-14 18:58:16 -04:00
Daniel Nephin 0acfc2c65b agent: fix a data race in DNS tests
The dnsConfig pulled from the atomic.Value is a pointer, so modifying it in place
creates a data race. Use the exported ReloadConfig interface instead.
2021-07-14 18:58:16 -04:00
Daniel Nephin 970f5d78ec agent: fix two data race in agent tests
The LogOutput io.Writer used by TestAgent must allow concurrent reads and writes, and a
bytes.Buffer does not allow this. The bytes.Buffer must be wrapped with a lock to make this safe.
2021-07-14 18:58:16 -04:00
Daniel Nephin baa2b8628e consul: fix data race in leader CA tests
Some global variables are patched to shorter values in these tests. But the goroutines that read
them can outlive the test because nothing waited for them to exit.

This commit adds a Wait() method to the routine manager, so that tests can wait for the goroutines
to exit. This prevents the data race because the 'reset to original value' can happen
after all other goroutines have stopped.
2021-07-14 18:58:15 -04:00
Daniel Nephin 204bf2b345 dns: correct rcode for qtype not supported
A previous commit started using QueryRefuced, but that is not correct. QueryRefuced refers to
the OpCode, not the query type.

Instead use errNoAnswer because we have no records for that query type.
2021-07-14 17:48:50 -04:00
Dhia Ayachi ad2065f2aa Check response len do not exceed max Buffer size 2021-07-14 17:15:34 -04:00
Dhia Ayachi f8f2756967 add missing test for truncate 2021-07-14 17:15:34 -04:00
Daniel Nephin d116bda958 dns: remove network parameter from two funcs
Now that trimDNSResponse is handled by the caller we don't need to pass this value
around. We can remove it from both the serviceLookup struct, and two functions.
2021-07-14 17:15:34 -04:00
Daniel Nephin 42f7963252 dns: trim response immediately before the write
Previously the response was being trimmed before adding the EDNS values, which could cause it to exceed
the max size.
2021-07-14 17:15:34 -04:00
Daniel Nephin 436a02af31 dns: handle errors from dispatch 2021-07-14 17:15:34 -04:00
Daniel Nephin 9267b09c32 dns: error response from dispatch
So that dispatch can communicate status back to the caller.
2021-07-14 17:15:34 -04:00
Daniel Nephin 68d6f1315f dns: refactor dispatch to use an explicit return in each case
In preparation for changing the return value, so that SOA, eDNS trimming and 'not found'
errors can be handled in a single place.
2021-07-14 17:15:34 -04:00
Daniel Nephin b96c8195a5 dns: small refactor to setEDNS to return early
Using a guard clause instead of a long nested if.

The diff is best viewed with whitespace turned off.
2021-07-14 17:15:34 -04:00
Daniel Nephin 4beff900d1 dns: remove unused method
It was added in 5934f803bf but it was never used.
2021-07-14 17:15:34 -04:00
Daniel Nephin f31aa12cf1 dns: remove unnecessary function wrapping
The dispatch function was called from a single place and did nothing but add a default value.
Removing it makes code easier to trace by removing an unnecessary hop.
2021-07-14 17:15:33 -04:00
Kyle Havlovitz 77a2f38677 http: add partition query param parsing 2021-07-14 12:07:38 -07:00
hc-github-team-consul-core 1169df0878 auto-updated agent/uiserver/bindata_assetfs.go from commit 3e80e637b 2021-07-14 18:00:42 +00:00
Daniel Nephin 74fb650b6b
Merge pull request #10588 from hashicorp/dnephin/config-fix-ports-grpc
config: rename `ports.grpc` to `ports.xds`
2021-07-13 13:11:38 -04:00
Daniel Nephin b5cd2050b4 fix backwards compat for envoy command
The compatv2 integration tests were failing because they use an older CLI version with a newer
HTTP API. This commit restores the GRPCPort field to the DebugConfig output to allow older
CIs to continue to fetch the port.
2021-07-13 12:31:49 -04:00
Daniel Nephin 233d03dbbd Apply suggestions from code review
Co-authored-by: R.B. Boyer <4903+rboyer@users.noreply.github.com>
2021-07-13 12:31:49 -04:00
Daniel Nephin 4ad80ccee3 command/envoy: stop using the DebugConfig from Self endpoint
The DebugConfig in the self endpoint can change at any time. It's not a stable API.

With the previous change to rename GRPCPort to XDSPort this command would have broken.
This commit adds the XDSPort to a stable part of the XDS api, and changes the envoy command to read
this new field.

It includes support for the old API as well, in case a newer CLI is used with an older API, and
adds a test for both cases.
2021-07-13 12:31:49 -04:00
Daniel Nephin c48f26b0a6 config: update config settings and flags for ports.xds 2021-07-13 12:31:48 -04:00
Dhia Ayachi 58bd817336
check expiry date of the root/intermediate before using it to sign a leaf (#10500)
* 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.

* ca: move SignCertificate to CAManager

To reduce the scope of Server, and keep all the CA logic together

* ca: move SignCertificate to the file where it is used

* auto-config: move autoConfigBackend impl off of Server

Most of these methods are used exclusively for the AutoConfig RPC
endpoint. This PR uses a pattern that we've used in other places as an
incremental step to reducing the scope of Server.

* fix linter issues

* check error when `raftApplyMsgpack`

* ca: move SignCertificate to CAManager

To reduce the scope of Server, and keep all the CA logic together

* check expiry date of the intermediate before using it to sign a leaf

* fix typo in comment

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

* Fix test name

* do not check cert start date

* wrap error to mention it is the intermediate expired

* Fix failing test

* update comment

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

* use shim to avoid sleep in test

* add root cert validation

* remove duplicate code

* Revert "fix linter issues"

This reverts commit 6356302b54f06c8f2dee8e59740409d49e84ef24.

* fix import issue

* gofmt leader_connect_ca

* add changelog entry

* update error message

Co-authored-by: Freddy <freddygv@users.noreply.github.com>

* fix error message in test

Co-authored-by: Daniel Nephin <dnephin@hashicorp.com>
Co-authored-by: Kyle Havlovitz <kylehav@gmail.com>
Co-authored-by: Freddy <freddygv@users.noreply.github.com>
2021-07-13 12:15:06 -04:00
R.B. Boyer 6c47efd532
connect/ca: ensure edits to the key type/bits for the connect builtin CA will regenerate the roots (#10330)
progress on #9572
2021-07-13 11:12:07 -05:00
R.B. Boyer 7bf9ea55cf
connect/ca: require new vault mount points when updating the key type/bits for the vault connect CA provider (#10331)
progress on #9572
2021-07-13 11:11:46 -05:00
Daniel Nephin 0ccad1d6f7
Merge pull request #10479 from hashicorp/dnephin/ca-provider-explore-2
ca: move Server.SignIntermediate to CAManager
2021-07-12 19:03:43 -04:00
Daniel Nephin b89ec826c8
Merge pull request #10445 from hashicorp/dnephin/ca-provider-explore
ca: isolate more of the CA logic in CAManager
2021-07-12 15:26:23 -04:00
Daniel Nephin bf292cbae4 ca: use provider constructors to be more consistent
Adds a contructor for the one provider that did not have one.
2021-07-12 14:04:34 -04:00
Dhia Ayachi 5ed56fc786 check error when `raftApplyMsgpack` 2021-07-12 13:42:51 -04:00