Moves public daprd metadata server to new authorized port

[JoshVanL]

Currently, daprd is exposing the public port which exposes the healthz and metadata endpoints. This is exposed publicly on the network. This server has no authentication and is serving requests in plaintext.

The metadata endpoint returns information like actors, subscriptions, components etc.

This represents a massive privacy exposure. This port must be disabled on any multi-tenant deployment of Dapr, however this is not documented anywhere and the daprd logs don't explain that this is happening either. Users are currently exposing this information without knowledge.

This PR moves the public metadata endpoint to a new port, and adds authentication and authorization to the endpoint. If mTLS is enabled, the endpoint will serve using the same TLS as the internal API server. The server will only serve requests to SPIFFE peers that present in the --dapr-metadata-authorized-ids CLI flag. If mTLS is disabled, this flag is ignored.

PR extends the mTLS Configuration API with a new metadataAuthorizedIDs fields which takes a slice of SPIFFE IDs. This slice is appended to the authorized list when consumed by daprd. When set on the global dapr-system configuration, the sidecar-injector sets this value on the sidecar container. This allows users to configure the authorized IDs globally, or on a per-pod basis.

Closes #7397

[ItalyPaleAle]

Although I appreciate that this is done the "right way" and requires mTLS, we should consider how the metadata API is likely being used, which is that apps may call it to get information on the sidecar, or possibly even external monitoring/cataloging services.

Adding the requirement of using mTLS, fetching certs from Sentry, in this case is IMHO very burdensome given the kind of consumers.

I would rather add a more simple IP-based allowlisting. Or just trust that users will not expose the port in the K8s service.

[codecov]

Codecov Report

Attention: Patch coverage is 27.77778% with 78 lines in your changes are missing coverage. Please review.

Project coverage is 61.99%. Comparing base (aa25dd9) to head (7881640).
Report is 67 commits behind head on master.

Files	Patch %	Lines
pkg/api/http/server.go	2.85%	33 Missing and 1 partial ⚠️
pkg/runtime/config.go	7.69%	11 Missing and 1 partial ⚠️
pkg/security/fake/fake.go	0.00%	12 Missing ⚠️
pkg/security/security.go	0.00%	11 Missing ⚠️
pkg/api/http/http.go	0.00%	4 Missing ⚠️
pkg/injector/service/globalconfig.go	80.00%	2 Missing and 1 partial ⚠️
pkg/injector/service/pod_patch.go	60.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7423      +/-   ##
==========================================
- Coverage   62.13%   61.99%   -0.14%     
==========================================
  Files         245      246       +1     
  Lines       22479    22563      +84     
==========================================
+ Hits        13967    13988      +21     
- Misses       7345     7406      +61     
- Partials     1167     1169       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[artursouza]

Although I appreciate that this is done the "right way" and requires mTLS, we should consider how the metadata API is likely being used, which is that apps may call it to get information on the sidecar, or possibly even external monitoring/cataloging services.

Adding the requirement of using mTLS, fetching certs from Sentry, in this case is IMHO very burdensome given the kind of consumers.

I would rather add a more simple IP-based allowlisting. Or just trust that users will not expose the port in the K8s service.

I agree with @ItalyPaleAle. At least, offer a way for people to keep the existing behavior. We should do that anyway because it is a breaking change.

[artursouza]

The current behavior must be kept and the new behavior should be done via a config flag. Default must be current behavior, following the deprecation policy.

[JoshVanL]

The project should always strive to do things the right way, especially so for security. I don't think we should follow the deprecation policy in this instance as this is an undocumented privacy vulnerability in multi tenant deployments.

Applications that currently consume the metadata API can continue to do so via the localhost endpoints. Communication between Dapr hosts must always be mTLS by default in Kubernetes. Any system which is currently consuming this API should have enough knowledge of Dapr already to be able to request an identity certificate from sentry using the gRPC protos. We must have a mechanism to authorize this endpoint- IP allowlists are not appropriate in cloud native environments as IP addresses are ephemeral.

[artursouza]

The project should always strive to do things the right way, especially so for security. I don't think we should follow the deprecation policy in this instance as this is an undocumented privacy vulnerability in multi tenant deployments.

Applications that currently consume the metadata API can continue to do so via the localhost endpoints. Communication between Dapr hosts must always be mTLS by default in Kubernetes. Any system which is currently consuming this API should have enough knowledge of Dapr already to be able to request an identity certificate from sentry using the gRPC protos. We must have a mechanism to authorize this endpoint- IP allowlists are not appropriate in cloud native environments as IP addresses are ephemeral.

I understand your rationale. On the other hand, an OSS project have to be careful on forcing a migration on a minor version upgrade. It can lead to the opposite result, where users stick to an older version of Dapr to plan ahead their migration in order to upgrade to a version that requires a significant change of their environment - there can be internal tools depending on this behavior that only the user knows.

[ItalyPaleAle]

(Please also note that this metadata API is not enabled by default, so users must enable it explicitly)

Before we make decisions here we should really inquire on how users are using this API. My feeling is that it used by external services for cataloging or other purposes, for whom requesting a certificate from Sentry would not be feasible and would make the API unusable. (Also, we do not have public-facing SDKs for Sentry)

Agree IP allowlisting isn't great, but this leaves us with 2 options:

1. Let users figure it out. We can add a warning when the metadata endpoint is enabled that it can expose sensitive information and users should be careful when exposing that.
2. Shared key authentication, combined with TLS (not mTLS). Not great but better than nothing.

[JoshVanL]

@ItalyPaleAle it is enabled by default in Kubernetes.

[ItalyPaleAle]

@ItalyPaleAle it is enabled by default in Kubernetes.

Ok that's a different problem then. I thought the PR that implemented that made it optional

[dapr-bot]

This pull request has been automatically marked as stale because it has not had activity in the last 60 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!