Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-24 Thread Ron Dagostino
Hi everyone. Jun raised a good point/discovered an oversight in the KIP during the VOTE thread that we must resolve. Regarding this statement in the KIP: "A client-side metric will be created that documents the latency imposed by re-authentication." Jun correctly asked: What's the name of this

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-19 Thread Ron Dagostino
Hi Tyler. This KIP is written such that the sever (broker) specifies a session lifetime to the client, and then the client will re-authenticate at a time consistent with that with whatever credentials it has when the re-authentication kicks off. You could specify a low max session lifetime on the

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-19 Thread Tyler Monahan
Hello, I have a rather odd issue that I think this KIP might fix but wanted to check. I have a kafka setup using SASL/Kerberos and when a broker dies in aws a new one is created using the same name/id. The Kerberos credentials however are different on the new instances and existing brokers/consume

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-19 Thread Ron Dagostino
Thanks, Rajini -- I updated the KIP to fix this. Ron On Wed, Sep 19, 2018 at 4:54 AM Rajini Sivaram wrote: > I should have said `security configs` instead of `channel configs`. > > The KIP says: > >- The configuration option this KIP proposes to add to enable >server-side expired-connec

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-19 Thread Rajini Sivaram
I should have said `security configs` instead of `channel configs`. The KIP says: - The configuration option this KIP proposes to add to enable server-side expired-connection-kill is '*connections.max.reauth.ms *' (not prefixed with listener prefix or

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-18 Thread Ron Dagostino
HI Rajini. The KIP is updated as summarized below, and I will start a vote immediately. << wrote: > Hi Ron, > > Thanks for the updates. The KIP looks good. A few comments and minor points > below, but feel free to start vote to try and get it into 2.1.0. More > community feedback will be really

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-18 Thread Rajini Sivaram
Hi Ron, Thanks for the updates. The KIP looks good. A few comments and minor points below, but feel free to start vote to try and get it into 2.1.0. More community feedback will be really useful. 1) It may be useful to have a metric of expired connections killed by the broker. There could be a cl

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-17 Thread Ron Dagostino
HI again, Rajini. Would we ever want the max session time to be different across different SASL mechanisms? I'm wondering, now that we are supporting all SASL mechanisms via this KIP, if we still need to prefix this config with the "[listener].[mechanism]." prefix. I've kept the prefix in the KI

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-17 Thread Ron Dagostino
Hi Rajini. The KIP is updated. Aside from a once-over to make sure it is all accurate, I think we need to confirm the metrics. The decision to not reject authentications that use tokens with too-long a lifetime allowed the metrics to be simpler. I decided that in addition to tracking these metr

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-17 Thread Ron Dagostino
<< wrote: > Hi Ron, > > > *1) Is there a reason to communicate this value back to the client when the > client already knows it? It's an extra network round-trip, and since the > SASL round-tripsare defined by the spec I'm not certain adding an extra > round-trip is acceptable.* > > I wasn't sugg

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-17 Thread Rajini Sivaram
Hi Ron, *1) Is there a reason to communicate this value back to the client when the client already knows it? It's an extra network round-trip, and since the SASL round-tripsare defined by the spec I'm not certain adding an extra round-trip is acceptable.* I wasn't suggesting an extra round-trip

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-17 Thread Ron Dagostino
Hi yet again, Rajini. I also just realized that if the client is to learn the credential lifetime we wouldn't want to put special-case code in the Authenticator for GSSAPI and OAUTHBEARER; we would want to expose the value generically, probably as a negotiated property on the SaslClient instance.

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-17 Thread Ron Dagostino
Hi again, Rajini. After thinking about this a little while, it occurs to me that maybe the communication of max session lifetime should occur via SASL_HANDSHAKE after all. Here's why. The value communicated is the max session lifetime allowed, and the client can assume it is the the session life

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-17 Thread Ron Dagostino
Hi Rajini. I've updated the KIP to reflect the decision to fully support this for all SASL mechanisms and to not require the ExpiringCredential interface to be public. Ron On Mon, Sep 17, 2018 at 6:55 AM Ron Dagostino wrote: > Actually, I think the metric remains the same assuming the authenti

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-17 Thread Ron Dagostino
Actually, I think the metric remains the same assuming the authentication fails when the token lifetime is too long. Negative max config on server counts what would have been killed because maybe client re-auth is not turned on; positive max enables the kill, which is counted by a second metri

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-17 Thread Ron Dagostino
Hi Rajini. I see what you are saying. The background login refresh thread does have to factor into the decision for OAUTHBEARER because there is no new token to re-authenticate with until that refresh succeeds (similarly with Kerberos), but I think you are right that the interface doesn’t nece

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-17 Thread Rajini Sivaram
Hi Ron, Thinking a bit more about other SASL mechanisms, I think the issue is that in the current proposal, clients decide the re-authentication period. For mechanisms like PLAIN or SCRAM, we would actually want server to determine the re-authentication interval. For example, the PLAIN or SCRAM da

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-14 Thread Ron Dagostino
Hi everyone. I've updated the KIP to reflect all discussion to date, including the decision to go with the low-level approach. This latest version of the KIP includes the above "connections.max.reauth.ms" proposal, which I know has not been discussed yet. It mentions new metrics, some of which m

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-14 Thread Ron Dagostino
Minor correction: I'm proposing "connections.max.reauth.ms" as the broker-side configuration property, not " connections.max.expired.credentials.ms". Ron On Fri, Sep 14, 2018 at 8:40 PM Ron Dagostino wrote: > Hi Rajini. I'm going to choose *connections.max.expired.credentials.ms >

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-14 Thread Ron Dagostino
Hi Rajini. I'm going to choose *connections.max.expired.credentials.ms * as the option name because it is consistent with the comment I made in the section about how to get the client and server to agree on credential lifetime: "We could add a new AP

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-14 Thread Rajini Sivaram
It will be good to shorten the config name. We have a config named ` connections.max.idle.ms`. We could add something like ` connections.max.expired.credentials.ms`. As you said, it would be prefixed with listener and SASL mechanism name. We should be able to support connection termination in futur

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-14 Thread Ron Dagostino
Hi Rajini. Agreed, we will bump the SaslHandshakeRequest API number (no wire format change, of course). Agreed, responding to re-authentication will always be enabled on the broker since it is initiated by the client. Agreed, connection termination by the broker will be opt-in. I'm thinking *sa

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-14 Thread Rajini Sivaram
Hi Ron, 1) Yes, we should update the version of `SaslHandshakeRequest`. Clients would attempt to re-authenticate only if broker's SaslHandshakeRequest version >=2. 2) I think we should enable broker's re-authentication and connection termination code regardless of client version. Re-authentication

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-13 Thread Ron Dagostino
Hi Rajini. I'm thinking about how we deal with migration. For example, let's say we have an existing 2.0.0 cluster using SASL/OAUTHBEARER and we want to use this feature. The desired end state is to have all clients and brokers migrated to a version that supports the feature (call it 2.x) with

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-12 Thread Ron Dagostino
Ok, I am tempted to just say we go with the low-level approach since it is the quickest and seems to meet the clear requirements. We can always adjust later if we get to clarity on other requirements or we decide we need to approach it differently for whatever reason. But in the meantime, before f

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-12 Thread Rajini Sivaram
Hi Ron, Yes, I would leave out retries from this KIP for now. In the future, if there is a requirement for supporting retries, we can consider it. I think we can support retries with either approach if we needed to, but it would be better to do it along with other changes required to support authe

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-12 Thread Ron Dagostino
Hi Rajini. Here is some feedback/some things I thought of. First, I just realized that from a timing perspective that I am not sure retry is going to be necessary. The background login refresh thread triggers re-authentication when it refreshes the client's credential. The OAuth infrastructure

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-12 Thread Ron Dagostino
Thanks, Rajini. Before I digest/respond to that, here's an update that I just completed. I added a commit to the PR (https://github.com/apache/kafka/pull/5582/) that implements server-side kill of expired OAUTHBEARER connections. No tests yet since we still haven't settled on a final approach (l

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-12 Thread Rajini Sivaram
Hi Ron, Thank you for summarising, I think it covers the differences between the two approaches well. A few minor points to answer the questions in there: 1) When re-authetication is initiated in the Selector during poll(), we can move an idle channel to re-authentication state. It is similar to

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-12 Thread Stanislav Kozlovski
Hi Ron, Rajini Thanks for summarizing the discussion so far, Ron! 1) How often do we have such long-lived connection idleness (e.g 5-10 minutes) in practice? 3) I agree that retries for re-authentication are useful. 4) The interleaving of requests sounds like a great feature to have, but the tr

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-10 Thread Ron Dagostino
Hi everyone. I've updated the PR to reflect the latest conclusions from this ongoing discussion. The KIP still needs the suggested updates; I will do that later this week. II agree with Rajini that some additional feedback from the community at large would be very helpful at this point in time.

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-10 Thread Rajini Sivaram
Hi Ron, Thank you for the analysis. Yes, I agree with the differences you have pointed out between the two approaches. 1. Re-authenticating idle connections (or connections nearing idle timeout): With the lower-level approach, there is nothing stopping us from re-authenticating idle conn

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-08 Thread Ron Dagostino
Hi yet again, Rajini. If we decide that interleaving is impossible with the low-level approach but we still want to at least support the possibility given that it reduces the size of latency spikes, then I do have a suggestion. I documented in the KIP how I rejected a single, one-size-fits-all qu

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-07 Thread Ron Dagostino
Hi again, Rajini. It occurs to me that from a *behavior* perspective there are really 3 fundamental differences between the low-level approach you provided and the high-level approach as it currently exists in the PR: 1) *When re-authentication starts*. The low-level approach initiates re-authen

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-07 Thread Ron Dagostino
Hi Rajini. The code really helped me to understand what you were thinking -- thanks. I'm still digesting, but here are some quick observations: Your approach (I'll call it the "low-level" approach, as compared to the existing PR, which works at a higher level of abstraction) -- the low-level app

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-06 Thread Rajini Sivaram
Hi Ron, The commit https://github.com/rajinisivaram/kafka/commit/b9d711907ad843c11d17e80d6743bfb1d4e3f3fd shows the kind of flow I was thinking of. It is just a prototype with a fixed re-authentication period to explore the possibility of implementing re-authentication similar to authentication. T

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-06 Thread Ron Dagostino
Yeah, regarding ControllerChannelManager, it is one of the synchronous I/O use cases (along with 2 others: KafkaProducer, via Sender; and ReplicaFetcherBlockingSend, via ReplicaFetcherThread) where the assumption is complete ownership of the connection. The PR's approach to dealing with that assump

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-06 Thread Rajini Sivaram
Hi Ron, Disconnections on the broker-side: I think we should do disconnections as a separate KIP and PR as you originally intended. But that one could be done separately without requiring KIP-368 as a pre-req. As a simpler implementation and one that can be used without KIP-368 in some cases, we c

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-05 Thread Ron Dagostino
I just thought of another alternative if the imports are the concern. KafkaChannel could expose the fact that it can create an additional Authenticator instance on the side (what I referred to as notYetAuthenticatedAuthenticator in the PR) and it could let kafka.server.KafkaApis drive the whole thi

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-05 Thread Ron Dagostino
Hi again, Rajini, I realized a couple of potential concerns with using the TransportLayer directly during re-authentication. First, in the blocking I/O use case, the owner of the NetworkClient instance calls NetworkClientUtils.sendAndReceive() to send requests. This method assumes the caller h

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-05 Thread Ron Dagostino
Hi Rajini. I'm now skeptical of my "ConnectionState.REAUTHENTICATING" idea. The concept of a connection being "READY" or not can impact ConsumerCoordinator (see, for example, https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerCoordi

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-05 Thread Ron Dagostino
<< wrote: > On Wed, Sep 5, 2018, at 07:34, Ron Dagostino wrote: > > I added a "How To Support Re-Authentication for Other SASL Mechanisms" > > section to the KIP as Rajini suggested. I also added a "Rejected > > Alternative" for the idea of forcibly closing connections on the client > > side upon

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-05 Thread Colin McCabe
On Wed, Sep 5, 2018, at 07:34, Ron Dagostino wrote: > I added a "How To Support Re-Authentication for Other SASL Mechanisms" > section to the KIP as Rajini suggested. I also added a "Rejected > Alternative" for the idea of forcibly closing connections on the client > side upon token refresh or on

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-05 Thread Ron Dagostino
I added a "How To Support Re-Authentication for Other SASL Mechanisms" section to the KIP as Rajini suggested. I also added a "Rejected Alternative" for the idea of forcibly closing connections on the client side upon token refresh or on the server side upon token expiration. It may be a bit prem

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-05 Thread Colin McCabe
On Wed, Sep 5, 2018, at 01:41, Rajini Sivaram wrote: > *Re-authentication vs disconnection:* > In a vast number of secure Kafka deployments, SASL_SSL is the security > protocol (this is the recommended config for OAUTHBEARER). If we require > disconnections on token expiry, we would need new connec

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-05 Thread Rajini Sivaram
*Re-authentication vs disconnection:* In a vast number of secure Kafka deployments, SASL_SSL is the security protocol (this is the recommended config for OAUTHBEARER). If we require disconnections on token expiry, we would need new connections to be established with an expensive SSL handshake. This

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-04 Thread Colin McCabe
On Tue, Sep 4, 2018, at 17:43, Ron Dagostino wrote: > Hi Colin. Different organizations will rely on different token lifetimes, > but anything shorter than an hour feels like it would be pretty > aggressive. An hour or more will probably be most common. Thanks. That's helpful to give me a sense

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-04 Thread Ron Dagostino
<<< It works for authentication because at that point we know that nobody << wrote: > Hi Ron, > > Thanks for the response. > > Retries: If we reuse authentication code path for re-authentication, we > might want to treat failures in the same way without retries, but for now, > lets leave it in. >

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-04 Thread Ron Dagostino
Hi Colin. Different organizations will rely on different token lifetimes, but anything shorter than an hour feels like it would be pretty aggressive. An hour or more will probably be most common. << wrote: > Hi Ron, > > Thanks for the KIP. > > What is the frequency at which you envision bearer

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-04 Thread Colin McCabe
Hi Ron, Thanks for the KIP. What is the frequency at which you envision bearer tokens changing at? Did you consider the alternate solution of terminating connections when the bearer token changed? best, Colin On Tue, Aug 28, 2018, at 07:28, Ron Dagostino wrote: > Hi everyone. I created KIP 3

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-04 Thread Rajini Sivaram
Hi Ron, Thanks for the response. Retries: If we reuse authentication code path for re-authentication, we might want to treat failures in the same way without retries, but for now, lets leave it in. Implementation: I think we are on the same page. *I believe Streams and Connect are taken care of

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-04 Thread Ron Dagostino
Hi Rajini. I'm glad you agree this is a good security addition. Regarding your questions/comments: 1. Retries. << requestBuilder, long createdTimeMs, boolean expectResponse, Requ

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-04 Thread Rajini Sivaram
Hi Ron, Thanks for the KIP. This is going to be a really useful feature to tighten security. I have a few comments/questions: 1. Retries: What is the state of a client connection in between re-authentication retries? Can it continue to process requests? Also, can you describe scenarios

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-03 Thread Ron Dagostino
I just realized there was one place in the KIP that stated that retries could occur indefinitely (when a client attempts to change identity, which we arbitrarily disallow). This was a mistake, a holdover from a prior draft of the KIP. This is now fixed. Retries are never allowed indefinitely. <

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-03 Thread Ron Dagostino
Thanks for the feedback, Stanislav. << to retry after some delay depending on how many retries have been attempted > so far: after some small amount of time for the first retry (e.g. 1 > minute), double that for the second retry, and 4 times the initial delay > for every retry thereafter. Retry a

Re: [DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-09-03 Thread Stanislav Kozlovski
Hi Ron, I really like this KIP, it is very needed. I am still looking through it but unfortunately I am not too familiar with the networking code to evaluate your solution. I'd like to ask what happens if re-authentication consistently fails. Would we retry endlessly? Since the functionality for

[DISCUSS] KIP 368: Allow SASL Connections to Periodically Re-Authenticate

2018-08-28 Thread Ron Dagostino
Hi everyone. I created KIP 368: Allow SASL Connections to Periodically Re-Authenticate ( https://cwiki.apache.org/confluence/display/KAFKA/KIP-368%3A+Allow+SASL+Connections+to+Perio