kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ron Dagostino <rndg...@gmail.com>
Subject Re: [VOTE] KIP 368: Allow SASL Connections to Periodically Re-Authenticate
Date Tue, 25 Sep 2018 03:14:33 GMT
Hi everyone.  This concludes the vote for KIP 368.  The vote passes with
three binding + 1 votes, from Rajini, Harsha, and Jun, and seven
non-binding +1 votes, from Mike, Konstantin, Boerge, Edoardo, Stanislav,
Mickael, and myself.

I have marked the KIP as "Accepted".

The pull request, available at *https://github.com/apache/kafka/pull/5582
<https://github.com/apache/kafka/pull/5582>*, is far along and should be
finished in the next day or two.

Ron

On Mon, Sep 24, 2018 at 9:25 PM Ron Dagostino <rndgstn@gmail.com> wrote:

> Thanks, Jun.
>
> <<<100
> I added this line to the KIP to clarify the SSL issue:
> "This KIP has no impact on non-SASL connections (e.g. connections that use
> the PLAINTEXT or SSL security protocols) – no such connection will be
> re-authenticated, and no such connection will be closed."
>
> <<<101
> Your comment points out two issues, I think.  First, there is a direct
> clarity problem: in the KIP, by the phrase "requests that queued up during
> re-authentication" I was really intending to refer to both the in-flight
> responses that might return from the broker during re-authentication along
> with any pending Send request that is set on the KafkaChannel instance
> and has not yet been transmitted to the broker (this is the Send that
> triggers the re-authentication check to occur, and when the session is
> "expired" the re-authentication process then begins).  So I clarified the
> text in the KIP tp refer directly to both of these things.  But before I
> insert that amended text, note that the second issue (the implementation
> option of marking the channel unavailable for send during
> re-authentication) also points out a clarity problem in the KIP because the
> channel is in fact unavailable for send during re-authentication.  The
> reason is because KafkaChannel#ready() will return false until the
> Authenticator finishes the re-authentication, and this causes KafkaClient#isReady(Node,
> long) and KafkaClient#ready(Node, long) to both return false.  So in fact
> the client will not be able to queue up send after send.  I've therefore
> updated the KIP text as follows:
> "If re-authentication succeeds then any received responses that queued up
> during re-authentication along with the Send that triggered the
> re-authentication to occur in the first place will subsequently be able to flow
> through (back to the client and along to the broker, respectively), and
> eventually the connection will re-authenticate again, etc.  Note also
> that the client cannot queue up additional send requests beyond the one
> that triggers re-authentication to occur until re-authentication succeeds
> and the triggering one is sent."
>
> <<<102
> Good catch about the client-side metric not having a name or a clear
> definition of what it measures.  That is an oversight.  I will resolve this
> via a post on the DISCUSS thread:
> https://lists.apache.org/thread.html/a63c1612fe9ba2f31272087a00419c59ed7a9917c398721069cd1d01@%3Cdev.kafka.apache.org%3E
>
> <<<103
> We re-use the existing authentication code paths for re-authentication,
> and it appears (
> https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/common/security/authenticator/SaslClientAuthenticator.java#L182)
> that the version used by the broker when it is acting as an inter-broker
> SASL client is the max version supported by the destination broker.  Am I
> missing something?
>
> Thanks again, Jun.
>
> Ron
>
>
>
>
> On Mon, Sep 24, 2018 at 7:46 PM Jun Rao <jun@confluent.io> wrote:
>
>> Hi, Ron,
>>
>> Thanks for the KIP. Looks good to me overall. So, +1 assuming the
>> following
>> minor comments will be addressed.
>>
>> 100. connections.max.reauth.ms: A client can authenticate with the broker
>> using SSL. This config has no impact on such connections. It would be
>> useful to make it clear in the documentation. Also, in this case, I guess
>> the broker won't kill the SSL connection after connections.max.reauth.ms?
>>
>> 101. "If re-authentication succeeds then any requests that queued up
>> during
>> re-authentication will subsequently be able to flow through, and
>> eventually
>> the connection will re-authenticate again, etc.". This is more of an
>> implementation detail. I guess the proposal is to queue up new requests in
>> the client when there is is pending re-authentication. An alternative is
>> to
>> mark the Channel unavailable for send during re-authentication. This has
>> the slight benefit of reducing the client memory footprint.
>>
>> 102. "A client-side metric will be created that documents the latency
>> imposed by re-authentication." What's the name of this metric? Does it
>> measure avg or max?
>>
>> 103. "Upgrade all brokers to v2.1.0 or later at whatever rate is desired
>> with 'connections.max.reauth.ms' allowed to default to 0.  If SASL is
>> used
>> for the inter-broker protocol then brokers will check the
>> SASL_AUTHENTICATE
>> API version and use a V1 request when communicating to a broker that has
>> been upgraded to 2.1.0, but the client will see the "0" session max
>> lifetime and will not re-authenticate. ". Currently, for the inter broker
>> usage of NetworkClient (ReplicaFetcherThread, ControllerChannelManager,
>> etc), the broker version discovery logic is actually disabled and the
>> client is expected to use the new version of the request after
>> inter.broker.protocol.version is set to the current version. So, we will
>> need to rely on this for deciding whether the NetworkClient should use the
>> re-authenticate request or not, during upgrade.
>>
>> Jun
>>
>> On Mon, Sep 24, 2018 at 4:39 PM, Ron Dagostino <rndgstn@gmail.com> wrote:
>>
>> > Still looking for a final +1 binding vote to go with the 9 votes so far
>> (2
>> > binding, 7 non-binding).
>> >
>> > Ron
>> >
>> > > On Sep 24, 2018, at 3:53 PM, Ron Dagostino <rndgstn@gmail.com> wrote:
>> > >
>> > >  **Please vote** . It's getting late in the day and this KIP still
>> > requires 1 more binding up-vote to be considered for the 2.1.0 release.
>> > >
>> > > The current vote is 2 binding +1 votes (Rajini and Harsha) and 7
>> > non-binding +1 votes (myself, Mike, Konstantin, Boerge, Edoardo,
>> Stanislav,
>> > and Mickael).
>> > >
>> > > Ron
>> > >
>> > >> On Mon, Sep 24, 2018 at 9:47 AM Ron Dagostino <rndgstn@gmail.com>
>> > wrote:
>> > >> Hi Everyone.  This KIP still requires 1 more binding up-vote to be
>> > considered for the 2.1.0 release.  **Please vote before today's
>> end-of-day
>> > deadline.**
>> > >>
>> > >> The current vote is 2 binding +1 votes (Rajini and Harsha) and 7
>> > non-binding +1 votes (myself, Mike, Konstantin, Boerge, Edoardo,
>> Stanislav,
>> > and Mickael).
>> > >>
>> > >> Ron
>> > >>
>> > >>> On Fri, Sep 21, 2018 at 11:48 AM Mickael Maison <
>> > mickael.maison@gmail.com> wrote:
>> > >>> +1 (non-binding)
>> > >>> Thanks for the KIP, this is a very nice feature.
>> > >>> On Fri, Sep 21, 2018 at 4:56 PM Stanislav Kozlovski
>> > >>> <stanislav@confluent.io> wrote:
>> > >>> >
>> > >>> > Thanks for the KIP, Ron!
>> > >>> > +1 (non-binding)
>> > >>> >
>> > >>> > On Fri, Sep 21, 2018 at 5:26 PM Ron Dagostino <rndgstn@gmail.com>
>> > wrote:
>> > >>> >
>> > >>> > > Hi Everyone.  This KIP requires 1 more binding up-vote
to be
>> > considered for
>> > >>> > > the 2.1.0 release; please vote before the Monday deadline.
>> > >>> > >
>> > >>> > > The current vote is 2 binding +1 votes (Rajini and Harsha)
and 5
>> > >>> > > non-binding +1 votes (myself, Mike, Konstantin, Boerge,
and
>> > Edoardo).
>> > >>> > >
>> > >>> > > Ron
>> > >>> > >
>> > >>> > > On Wed, Sep 19, 2018 at 12:40 PM Harsha <kafka@harsha.io>
>> wrote:
>> > >>> > >
>> > >>> > > > KIP looks good. +1 (binding)
>> > >>> > > >
>> > >>> > > > Thanks,
>> > >>> > > > Harsha
>> > >>> > > >
>> > >>> > > > On Wed, Sep 19, 2018, at 7:44 AM, Rajini Sivaram
wrote:
>> > >>> > > > > Hi Ron,
>> > >>> > > > >
>> > >>> > > > > Thanks for the KIP!
>> > >>> > > > >
>> > >>> > > > > +1 (binding)
>> > >>> > > > >
>> > >>> > > > > On Tue, Sep 18, 2018 at 6:24 PM, Konstantin
Chukhlomin <
>> > >>> > > > chuhlomin@gmail.com>
>> > >>> > > > > wrote:
>> > >>> > > > >
>> > >>> > > > > > +1 (non binding)
>> > >>> > > > > >
>> > >>> > > > > > > On Sep 18, 2018, at 1:18 PM,
>> michael.kaminski@nytimes.com
>> > wrote:
>> > >>> > > > > > >
>> > >>> > > > > > >
>> > >>> > > > > > >
>> > >>> > > > > > > On 2018/09/18 14:59:09, Ron Dagostino
<
>> rndgstn@gmail.com>
>> > wrote:
>> > >>> > > > > > >> Hi everyone.  I would like to
start the vote for
>> KIP-368:
>> > >>> > > > > > >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>> > >>> > > > > > 368%3A+Allow+SASL+Connections+to+Periodically+Re-
>> > Authenticate
>> > >>> > > > > > >>
>> > >>> > > > > > >> This KIP proposes adding the
ability for SASL clients
>> > (and brokers
>> > >>> > > > when
>> > >>> > > > > > a
>> > >>> > > > > > >> SASL mechanism is the inter-broker
protocol) to
>> > re-authenticate
>> > >>> > > > their
>> > >>> > > > > > >> connections to brokers and for
brokers to close
>> > connections that
>> > >>> > > > > > continue
>> > >>> > > > > > >> to use expired sessions.
>> > >>> > > > > > >>
>> > >>> > > > > > >> Ron
>> > >>> > > > > > >>
>> > >>> > > > > > >
>> > >>> > > > > > > +1 (non binding)
>> > >>> > > > > >
>> > >>> > > > > >
>> > >>> > > >
>> > >>> > >
>> > >>> >
>> > >>> >
>> > >>> > --
>> > >>> > Best,
>> > >>> > Stanislav
>> >
>>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message