drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sorabh Hamirwasia <shamirwa...@mapr.com>
Subject Re: Drill SASL Forward Compatibility
Date Tue, 31 Oct 2017 08:40:31 GMT
Hi Laurent,

Thanks for pointing issue with <= 1.9 version Drillbit, I looked into supported_methods
field and it doesn't advertise SASL support using that [1][2]. Do you mean that checking if
supported_methods list is non-empty should suffice since it was introduced in 1.10 ?


For security flaw let's consider an example. Let say client is connecting to a Drillbit with
authentication (and or encryption) enabled for Kerberos mechanism. The message flow will happen
something like below:


Good Case:

  *   DrillClient sends Handshake Request to Drillbit
  *   Drillbit sends the response back to DrillClient with AUTH_REQD as status
  *   DrillClient exchange SASL handshake messages with Drillbit.
  *   Once handshake is successful DrillClient is connected to secure Drillbit.
  *   App using DrillClient has actually established a connection to secure Drillbit with
authentication (and or encryption) and can submit it's query.

Bad Case:

  *   Step 1 as above
  *   Step 2 as above. This message was intercepted by MITM and status was changed to SUCCESS.
  *   Without the recent change DrillClient will not initiate SASL handshake and will return
connection successful.
  *   App using DrillClient will think it has successfully connected to secure Drillbit which
is NOT the case.

I think what you are pointing out is even in good case and in authentication only scenario,
even if connection is successful, the messages between DrillClient and Drillbit can still
be intercepted since they will be in plain text. The only way to avoid that is using encryption.
But the fix was more of to avoid wrong behavior in that case too where connection should fail,
instead of client just relying on server response.

[1]: https://github.com/apache/drill/blob/1.11.0/exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserServer.java#L254
[2]: https://github.com/apache/drill/blob/1.11.0/exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserRpcConfig.java#L89


Thanks,
Sorabh

________________________________
From: Laurent Goujon <laurent@dremio.com>
Sent: Monday, October 30, 2017 5:47 PM
To: dev
Cc: Arina Lelchieva; sudheesh@apache.org
Subject: Re: Drill SASL Forward Compatibility

Regarding DRILL-5582, I see that fix as a breakage of the work to maintain
compatibility for an newer client to connect to a older version of the
server. Or put it differently: current (master) client does not connect
anymore to a server not supporting SASL (<=1.9). Note that the client could
detect if the server supports SASL as it is advertised in the
supported_methods field of the BitToUserHandshake, and it would restore
compatibility, but it seems the fix was done in response to a potential
security flaw (although I have to admin not sure what issue it does prevent
since it is still possible for a MITM to intercept all traffic between a
client and a server).

Laurent

On Mon, Oct 30, 2017 at 5:18 PM, Sorabh Hamirwasia <shamirwasia@mapr.com>
wrote:

> Hi All,
>
> We recently added a check (as part of DRILL-5582<https://issues.
> apache.org/jira/browse/DRILL-5582>) on DrillClient side to enforce that
> if client showed intent for authentication and Drillbit say's it doesn't
> require authentication then connection will fail with proper error message.
>
>
> With this change we found a hidden issue w.r.t forward compatibility of
> Drill. New clients running on 1.11+ authenticating to older Drillbit
> running on 1.10 are treated as client running without any SASL support or
> (<=1.9 version). The root cause for this issue is as follows:
>
>
> In 1.10 a new field SASL_SUPPORT was introduced in Handshake message
> between DrillCilent and Drillbit. The supported values for that field are:
>
>
> Drill 1.10: [1]
>
>
> enum SASL_SUPPORT {
>     UNKNOWN_SASL_SUPPORT = 0;
>     SASL_AUTH = 1;
> }
>
>
> Drill 1.11/1.12: [2]
>
>
> enum SASL_SUPPORT {
>     UNKNOWN_SASL_SUPPORT = 0;
>     SASL_AUTH = 1;
>     SASL_PRIVACY = 2;
> }
>
> A 1.11 client always has SASL_PRIVACY set in handshake. A 1.10 Drillbit
> getting the message interprets the value as UNKNOWN_SASL_SUPPORT [3]. In
> 1.10 Drillbit treats that value as an indication of older client < 1.10
> [4], and it will try to authenticate using the 1.9 mechanism and send the
> SUCCESS/FAILURE state in Handshake Response. The 1.12 DrillClient will fail
> the connection as it will expect AUTH_REQUIRED from Drillbit instead of
> SUCCESS/FAILURE.
>
>
> The fix for this issue can be to use only absence of field as indication
> of client < 1.10 and if the field is present but it evaluates to
> UNKNOWN_SASL_SUPPORT value then Drillbit should consider corresponding
> client to be of future version at least for authentication purpose and
> speak SASL protocol.
>
> We have to either back port above forward compatibility fix into 1.10 and
> 1.11 or just fix in current release and support forward compatibility post
> 1.12+.
>
>
> Arina/Sudheesh - Please suggest if the analysis and fix sounds good and
> what are the releases we should consider the fix for. Considering 1.12
> release is planned soon can we take the fix in 1.12 release ?
>
>
>
> [1]: https://github.com/apache/drill/blob/1.10.0/protocol/
> src/main/protobuf/User.proto#L70
>
> [2]: https://github.com/apache/drill/blob/1.11.0/protocol/
> src/main/protobuf/User.proto#L70
>
> [3]: http://androiddevblog.com/protocol-buffers-pitfall-
> adding-enum-values/
>
> [4]: https://github.com/apache/drill/blob/1.10.0/exec/java-
> exec/src/main/java/org/apache/drill/exec/rpc/user/UserServer.java#L297
>
>
> Thanks,
> Sorabh
>

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