You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@arrow.apache.org by Radu Teodorescu <ra...@yahoo.com.INVALID> on 2020/10/26 21:26:54 UTC

mutual TLS peer_identity in arrow flight

Hi,
I have a follow up question/feature proposal in the context of mutual TLS (introduced by https://issues.apache.org/jira/browse/ARROW-8742 <https://issues.apache.org/jira/browse/ARROW-8742>):
In the context of mutual TLS the client is authenticated at TLS level and the client identity is available in the grpc context’s authentication context but that information is not propagated to the peer_identity in the arrow flight context.
This is because Flight has its own authentication mechanism and the TLS client authentication was added afterwards without connecting the two.

I suggest the following change to mediate the above (and happy to deliver it myself):

In the case where the client is authenticated by the GRPC/TSL layer, I can have the flight_context.peer_identity default to the PeerIdentity as stored in the grpc auth_context. 
Pros: it’s a 4 line change and it would work out of the box for both python and C++ with no public interface changes and no relevant observed behavior for existing code (except for peer_identity context field being properly populated instead of empty).
Cons: If there is a flight Authentication Handler, the lower level identity would be ignored (but that is the case in the current implementation already).

I can send out a PR unless there is another solution in the works

Re: mutual TLS peer_identity in arrow flight

Posted by Radu Teodorescu <ra...@yahoo.com.INVALID>.
Thank you folks,
A PR says more than a thousand words :) 
https://github.com/apache/arrow/pull/8537 <https://github.com/apache/arrow/pull/8537>

Certainly a much less ambitious change than propose in the auth redesign (which I am looking forward to btw).
To James’ concern, the auth information is only passed on from the ssl layer if there is client authentication (i.e. mTLS) and there is no authentication handler defined so it will have no impact on any use cases where one uses an authentication handler.

Let me know if you want me to make any further changes.
Radu

> On Oct 26, 2020, at 8:22 PM, James Duong <ja...@bitquilltech.com> wrote:
> 
> The authentication redesign goes further down the path of getting the peer
> identity from the authentication information.
> I would say that getting the peer context through mTLS is valid, but we
> shouldn't change the behavior of
> existing implementations of FlightProducers that get this from the auth
> handler. In other words, it should be
> an option to get peer identity this way that you can opt into.
> 
> On Mon, Oct 26, 2020 at 4:41 PM David Li <li...@gmail.com> wrote:
> 
>> Hey Radu,
>> 
>> That sounds fine to me, presumably if someone layers an authentication
>> handler on top of mTLS, they don't want the mTLS identity anymore.
>> 
>> Also note there's another auth redesign ongoing, though I don't think
>> that conflicts with this, but maybe the authors there might think
>> about how/if mTLS fits their design.
>> 
>> https://lists.apache.org/thread.html/r485888f4f818e8e4722dc6c53491fb4c68ee7ac16d1c769612e61d21%40%3Cdev.arrow.apache.org%3E
>> 
>> Best,
>> David
>> 
>> On 10/26/20, Radu Teodorescu <ra...@yahoo.com.invalid> wrote:
>>> Hi,
>>> I have a follow up question/feature proposal in the context of mutual TLS
>>> (introduced by https://issues.apache.org/jira/browse/ARROW-8742
>>> <https://issues.apache.org/jira/browse/ARROW-8742>):
>>> In the context of mutual TLS the client is authenticated at TLS level and
>>> the client identity is available in the grpc context’s authentication
>>> context but that information is not propagated to the peer_identity in
>> the
>>> arrow flight context.
>>> This is because Flight has its own authentication mechanism and the TLS
>>> client authentication was added afterwards without connecting the two.
>>> 
>>> I suggest the following change to mediate the above (and happy to
>> deliver it
>>> myself):
>>> 
>>> In the case where the client is authenticated by the GRPC/TSL layer, I
>> can
>>> have the flight_context.peer_identity default to the PeerIdentity as
>> stored
>>> in the grpc auth_context.
>>> Pros: it’s a 4 line change and it would work out of the box for both
>> python
>>> and C++ with no public interface changes and no relevant observed
>> behavior
>>> for existing code (except for peer_identity context field being properly
>>> populated instead of empty).
>>> Cons: If there is a flight Authentication Handler, the lower level
>> identity
>>> would be ignored (but that is the case in the current implementation
>>> already).
>>> 
>>> I can send out a PR unless there is another solution in the works
>> 
> 
> 
> -- 
> 
> *James Duong*
> Lead Software Developer
> Bit Quill Technologies Inc.
> Direct: +1.604.562.6082 | jamesd@bitquilltech.com
> https://www.bitquilltech.com
> 
> This email message is for the sole use of the intended recipient(s) and may
> contain confidential and privileged information.  Any unauthorized review,
> use, disclosure, or distribution is prohibited.  If you are not the
> intended recipient, please contact the sender by reply email and destroy
> all copies of the original message.  Thank you.


Re: mutual TLS peer_identity in arrow flight

Posted by James Duong <ja...@bitquilltech.com>.
The authentication redesign goes further down the path of getting the peer
identity from the authentication information.
I would say that getting the peer context through mTLS is valid, but we
shouldn't change the behavior of
existing implementations of FlightProducers that get this from the auth
handler. In other words, it should be
an option to get peer identity this way that you can opt into.

On Mon, Oct 26, 2020 at 4:41 PM David Li <li...@gmail.com> wrote:

> Hey Radu,
>
> That sounds fine to me, presumably if someone layers an authentication
> handler on top of mTLS, they don't want the mTLS identity anymore.
>
> Also note there's another auth redesign ongoing, though I don't think
> that conflicts with this, but maybe the authors there might think
> about how/if mTLS fits their design.
>
> https://lists.apache.org/thread.html/r485888f4f818e8e4722dc6c53491fb4c68ee7ac16d1c769612e61d21%40%3Cdev.arrow.apache.org%3E
>
> Best,
> David
>
> On 10/26/20, Radu Teodorescu <ra...@yahoo.com.invalid> wrote:
> > Hi,
> > I have a follow up question/feature proposal in the context of mutual TLS
> > (introduced by https://issues.apache.org/jira/browse/ARROW-8742
> > <https://issues.apache.org/jira/browse/ARROW-8742>):
> > In the context of mutual TLS the client is authenticated at TLS level and
> > the client identity is available in the grpc context’s authentication
> > context but that information is not propagated to the peer_identity in
> the
> > arrow flight context.
> > This is because Flight has its own authentication mechanism and the TLS
> > client authentication was added afterwards without connecting the two.
> >
> > I suggest the following change to mediate the above (and happy to
> deliver it
> > myself):
> >
> > In the case where the client is authenticated by the GRPC/TSL layer, I
> can
> > have the flight_context.peer_identity default to the PeerIdentity as
> stored
> > in the grpc auth_context.
> > Pros: it’s a 4 line change and it would work out of the box for both
> python
> > and C++ with no public interface changes and no relevant observed
> behavior
> > for existing code (except for peer_identity context field being properly
> > populated instead of empty).
> > Cons: If there is a flight Authentication Handler, the lower level
> identity
> > would be ignored (but that is the case in the current implementation
> > already).
> >
> > I can send out a PR unless there is another solution in the works
>


-- 

*James Duong*
Lead Software Developer
Bit Quill Technologies Inc.
Direct: +1.604.562.6082 | jamesd@bitquilltech.com
https://www.bitquilltech.com

This email message is for the sole use of the intended recipient(s) and may
contain confidential and privileged information.  Any unauthorized review,
use, disclosure, or distribution is prohibited.  If you are not the
intended recipient, please contact the sender by reply email and destroy
all copies of the original message.  Thank you.

Re: mutual TLS peer_identity in arrow flight

Posted by David Li <li...@gmail.com>.
Hey Radu,

That sounds fine to me, presumably if someone layers an authentication
handler on top of mTLS, they don't want the mTLS identity anymore.

Also note there's another auth redesign ongoing, though I don't think
that conflicts with this, but maybe the authors there might think
about how/if mTLS fits their design.
https://lists.apache.org/thread.html/r485888f4f818e8e4722dc6c53491fb4c68ee7ac16d1c769612e61d21%40%3Cdev.arrow.apache.org%3E

Best,
David

On 10/26/20, Radu Teodorescu <ra...@yahoo.com.invalid> wrote:
> Hi,
> I have a follow up question/feature proposal in the context of mutual TLS
> (introduced by https://issues.apache.org/jira/browse/ARROW-8742
> <https://issues.apache.org/jira/browse/ARROW-8742>):
> In the context of mutual TLS the client is authenticated at TLS level and
> the client identity is available in the grpc context’s authentication
> context but that information is not propagated to the peer_identity in the
> arrow flight context.
> This is because Flight has its own authentication mechanism and the TLS
> client authentication was added afterwards without connecting the two.
>
> I suggest the following change to mediate the above (and happy to deliver it
> myself):
>
> In the case where the client is authenticated by the GRPC/TSL layer, I can
> have the flight_context.peer_identity default to the PeerIdentity as stored
> in the grpc auth_context.
> Pros: it’s a 4 line change and it would work out of the box for both python
> and C++ with no public interface changes and no relevant observed behavior
> for existing code (except for peer_identity context field being properly
> populated instead of empty).
> Cons: If there is a flight Authentication Handler, the lower level identity
> would be ignored (but that is the case in the current implementation
> already).
>
> I can send out a PR unless there is another solution in the works