Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Require tap user to be explicitly authorized for the tap #142

Closed
3 tasks
briansmith opened this issue Jan 12, 2018 · 2 comments
Closed
3 tasks

Require tap user to be explicitly authorized for the tap #142

briansmith opened this issue Jan 12, 2018 · 2 comments
Labels
area/security priority/P2 Nice-to-have for Release

Comments

@briansmith
Copy link
Contributor

briansmith commented Jan 12, 2018

Tap should be restricted to authorized users. I suggest we focus on RBAC-based authorization:

Initial Version:

  • Create a ClusterRole that for using the tap feature.
  • Have the tap controller authenticate and authorize the user for that ClusterRole. if the user doing the tapping doesn't have the role then the tap request should be denied.
  • Document how to grant people access to the tap feature through this role.

Future Version:
We need to be able to scope the authorization for tap in a flexible way. For example, we need to enable rules like "Allow Bob to tap all pods except those in the certificate-authority namespace" or "Allow Bob to only tap pods in the playground-bob" namespace.

I think for production environments we can assume that RBAC is enabled and working. However, note that current versions of minikube do not enable RBAC by default: kubernetes/minikube#1722. We probably should accomodate running Conduit in minikube with RBAC is disabled until the version of minikube with RBAC enabled by default is released. Alternatively we should tell people that enabling RBAC in minikube is required for Conduit.

@briansmith briansmith added this to the Conduit 0.4 milestone Feb 14, 2018
@olix0r olix0r added the priority/P2 Nice-to-have for Release label Mar 13, 2018
@olix0r olix0r modified the milestones: 0.4.0, 0.5.0 Apr 4, 2018
@olix0r olix0r removed this from the 0.5.0 milestone Jul 30, 2018
@stale
Copy link

stale bot commented Oct 28, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Oct 28, 2018
@stale stale bot removed the wontfix label Oct 29, 2018
khappucino pushed a commit to Nordstrom/linkerd2 that referenced this issue Mar 5, 2019
Previously, as the proxy processed requests, it would:

Obtain the taps mutex ~4x per request to determine whether taps are active.
Construct an "event" ~4x per request, regardless of whether any taps were
active.
Furthermore, this relied on fragile caching logic, where the grpc server
manages individual stream states in a Map to determine when all streams have
been completed. And, beyond the complexity of caching, this approach makes it
difficult to expand Tap functionality (for instance, to support tapping of
payloads).

This change entirely rewrites the proxy's Tap logic to (1) prevent the need
to acquire muteces in the request path, (2) only produce events as needed to
satisfy tap requests, and (3) provide clear (private) API boundaries between
the Tap server and Stack, completely hiding gRPC details from the tap service.

The tap::service module now provides a middleware that is generic over a
way to discover Taps; and the tap::grpc module (previously,
control::observe), implements a gRPC service that advertises Taps such that
their lifetimes are managed properly, leveraging RAII instead of hand-rolled
map-based caching.

There is one user-facing change: tap stream IDs are now calculated relative to
the tap server. The base id is assigned from the count of tap requests that have
been made to the proxy; and the stream ID corresponds to an integer on [0, limit).
@grampelberg
Copy link
Contributor

Fixed by #3171.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/security priority/P2 Nice-to-have for Release
Projects
None yet
Development

No branches or pull requests

4 participants