old_mixer_repo icon indicating copy to clipboard operation
old_mixer_repo copied to clipboard

Consider how to handle sensitive attributes

Open lizan opened this issue 8 years ago • 13 comments

Currently mixer gets all the HTTP headers from proxy, the headers might include sensitive information. For example Authorization header might include JWT token which can be replayed, or even worse with Basic auth.

While some of those attributes might be needed for some adapters handling authn/z, but it shouldn't exposed to all adapters (e.g. never include them in log or trace), or some of them should be stripped before sending to mixer.

Any thoughts?

lizan avatar Jul 14 '17 00:07 lizan

I think the notion of some sort of "restricted" attributes is an interesting and potentially valuable consideration.

Once attributes hit Mixer, however, I'm not quite sure how we can enforce it practically. For instance, it would be trivial to write an authn/z preprocessing adapter that copies a JWT into a different attribute which is then exposed to the rest of the adapters.

But maybe that isn't our particular concern -- maybe we just want to prevent accidental leakage?

Maybe this is something we could do at validation time, based on manifests (only preprocess adapters of kind 'authorization' can access attributes in the 'authorization' manifest)?

Stripping them before they are sent to Mixer seems easier to enforce in Envoy config generation.

One tangential item that this touches on as well, I think, is that we want/need mTLS connections between service proxies and Mixer soon.

douglas-reid avatar Jul 14 '17 06:07 douglas-reid

Yes the main concern is to prevent accidental leakage, and also possibly avoid SPoF for the (end-user) auth system.

mTLS connections between service proxies and Mixer is a requirement for this, without that proxies should never send credentials over the wire.

lizan avatar Jul 14 '17 07:07 lizan

Stripping them before they are sent to Mixer seems easier to enforce in Envoy config generation.

While this is easier in practical, then Mixer loses ability to handle authn. Authz is doable, once proxy verified the credential, and only pass the result as an attributes and send to Mixer.

lizan avatar Jul 14 '17 07:07 lizan

Agreed. I didn't mean to imply that we should prevent Mixer from handling authn, only that it should be straightforward to blacklist attributes that we never want to send to Mixer.

douglas-reid avatar Jul 14 '17 07:07 douglas-reid

Proxy should have a flag to strip out these sensitive info. filed https://github.com/istio/proxy/issues/387

qiwzhang avatar Jul 14 '17 15:07 qiwzhang

@lizan Is it enough to do these checks at config time, i.e. when an adapter is configured, check whether or not the adapter is cleared to get sensitive data, and whether or not the configuration maps any sensitive attributes to it.

Or, do we need the checks to be done at run-time?

For example, some configurations map an attribute only if present, e.g. source.user | "unknown". If we considered the source.user sensitive, do we need to, at runtime, prevent adapters from getting called if the source.user was used and allow the adapters to be called if "unknown" was used?

spikecurtis avatar Jul 18 '17 22:07 spikecurtis

I think that is a good place to start with. run-time checks sounds overkill to me.

lizan avatar Jul 18 '17 23:07 lizan

What is sensitive will vary from one environment to the next. Arbitrary headers and possibly even the URL itself. This means that doing it in the proxy will need to be very flexible. Synthesizing from the above comments, does the following approach make sense:

  • operators can group attributes into collections (an attribute can belong to multiple collections)
  • adapter is configured with access to a particular collection (a collection can be used by multiple adapters)
  • part of the preprocessing (resolution) of attributes step, an operator can configure an adapter that does redaction/anonymization based on operator configured sensitive info collection. The adapter can be configured with policies that are context sensitive (e.g,. remove certain URL or headers based on service, etc.)
  • audit and log adapters are have access to non-sensitive info only since they run after the preprocessing and resolution phases, after attributes have been redacted.

The above is based on operator configuring and provides leakage protection (we can enforce that an adapter only gets declared attributes which are defined for it, making accidental leakage more difficult).

For example, this helps in mitigating some of the risk associated with tokens validation in the mixer. The raw token is validated, then removed from attributes set and replaced with the claims. Only the token validation adapter is enabled for the Authorization header and is responsible for removing it. Customer can still decide to configure token validation in the proxy, if they're concerned with the implications of doing it at the mixer.

elevran avatar Jul 19 '17 11:07 elevran

The Mixer 0.2 design currently has operators configuring constructors which map attributes to templates for each adapter. With this design, I'm hoping collections as above won't be necessary---we can use the constructors for this purpose. This ensures adapters don't get arbitrary access to attributes, only those configured. Operators get explicit control over what adapters get what attributes.

I'm slightly hesitant to go further than this for the time being. I can imagine wanting some kind of additional checking to ensure an operator doesn't accidentally configure a sensitive attribute to be routed to a logging/tracing adapter. However, that's starting to get pretty meta, where we are defining attributes of attributes (e.g. this attribute is "sensitive") and attributes of adapters (e.g. this adapter is a "logger", or "insecure") and defining policies. I certainly don't think we want to be doing a general config policy system at this time.

You might argue that we don't need to solve the general case, and can just bake-in the sensitivity rules, but we'll need to agree that sensitivity really is an important special case that warrants special handling, rather than waiting for some yet-to-be-scheduled general config policy.

spikecurtis avatar Jul 19 '17 14:07 spikecurtis

@spikecurtis sounds reasonable to delay as control over adapter access to attributes can be configured in 0.2 model.

So following up on issues raised in this thread:

  1. control adapter access to attributes in mixer - operator has control in 0.2 model. Is there agreement that there is no need for additional work beyond that?
  2. allow operator to configure the proxy to not send some attributes to mixer. Is there a proxy issue tracking this?
  3. need mTLS between proxies and mixer. Do we have issues tracking that in proxy and mixer?

elevran avatar Jul 19 '17 18:07 elevran

  1. control adapter access to attributes in mixer - operator has control in 0.2 model. Is there agreement that there is no need for additional work beyond that?

+1

  1. allow operator to configure the proxy to not send some attributes to mixer. Is there a proxy issue tracking this?

istio/proxy#387

  1. need mTLS between proxies and mixer. Do we have issues tracking that in proxy and mixer?

Added. #924 Not sure if we need one in Proxy?

spikecurtis avatar Jul 19 '17 18:07 spikecurtis

#2. Proxy needs a configuration to declare which attributes to compute and send to Mixer. Currently, it's all done statically in C++ code.

#3. mTLS is required between Proxy-Mixer and Proxy-Pilot. The root problem there is how to bootstrap it all (mTLS depends on Pilot in 0.2) and level of automation. I'd be happy with embedding statically configured Proxies into the containers themselves.

kyessenov avatar Jul 19 '17 18:07 kyessenov

Just wanted to revisit: the original intent was to prevent accidental attribute leakage into logs. I can understand wanting to delay handling of that for the time being, but I think we should keep it on our radar as something to strive towards.

I think we are going to need/want a similar mechanism for distinguishing attribute contexts (tcp or http or ?) soon. It'd be nice if there was broad strategy for alignment there. Having an ABAC system for attributes does seem fairly meta, but it could perhaps be part of the value prop for Mixer down-the-road.

douglas-reid avatar Jul 22 '17 00:07 douglas-reid