You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pulsar.apache.org by Yunze Xu <yz...@streamnative.io.INVALID> on 2022/05/09 05:31:32 UTC

Re: [DISCUSS] PIP-158: Split client TLS transport encryption from authentication

It totally LGTM. I have a suggestion that it might be better to configure a
class like `TlsConfiguration` instead of multiple TLS related configs added to
`ClientBuilder`.

Thanks,
Yunze




> 2022年4月24日 14:15,Zixuan Liu <no...@gmail.com> 写道:
> 
> Hi Pulsar community,
> 
> I open a https://github.com/apache/pulsar/issues/15289 for Split client TLS
> transport encryption from authentication.
> 
> Let me know what you think.
> 
> Thanks,
> Zixuan
> 
> ------
> 
> Motivation
> 
> The client supports TLS transport encryption and TLS authentication, this
> code so like:
> 
> PulsarClient client = PulsarClient.builder()
>                .serviceUrl("pulsar+ssl://localhost:6651")
>                .tlsTrustCertsFilePath("/path/to/cacert.pem")
>                .authentication(AuthenticationTls.class.getName(), authParams)
>                .build()
> 
> This causes an issue that cannot use other authentication with TLS
> transport encryption, and also made our confusion if we use TLS transport
> encryption by setting authentication.
> Goal
> 
> Split client TLS transport encryption from authentication is used to
> support TLS transport encryption with any authentication.
> API Changes
> 
>   - Add new methods in org.apache.pulsar.client.api.ClientBuilder
> 
> public interface ClientBuilder extends Serializable, Cloneable {
>    /**     * Set the path to the TLS key file.     *     * @param
> tlsKeyFilePath     * @return the client builder instance     */
>    ClientBuilder tlsKeyFilePath(String tlsKeyFilePath);
> 
>    /**     * Set the path to the TLS certificate file.     *     *
> @param tlsCertificateFilePath     * @return the client builder
> instance     */
>    ClientBuilder tlsCertificateFilePath(String tlsCertificateFilePath);
> }
> 
> ImplementationTLS transport encryption
> 
> We can call the tlsKeyFilePath(), tlsCertificateFilePath() and
> tlsTrustCertsFilePath() to configurate the TLS transport encryption, the
> code so like:
> 
> PulsarClient client = PulsarClient.builder()
>        .serviceUrl("pulsar+ssl://my-host:6650")
>        .tlsTrustCertsFilePath("/path/to/cacert.pem")
>        .tlsKeyFilePath("/path/to/client-key.pem")
>        .tlsCertificateFilePath("/path/to/client-cert.pem")
>        .build();
> 
> TLS transport encryption with any authentication
> 
> We can call the tlsKeyFilePath(), tlsCertificateFilePath(),
> tlsTrustCertsFilePath() and authentication() to configurate the TLS
> transport encryption with any authentication, the code so like:
> 
> PulsarClient client = PulsarClient.builder()
>        .serviceUrl("pulsar+ssl://my-host:6650")
>        .tlsTrustCertsFilePath("/path/to/cacert.pem")
>        .tlsKeyFilePath("/path/to/client-key.pem")
>        .tlsCertificateFilePath("/path/to/client-cert.pem")
>        .authentication(AuthenticationTls.class.getName() /*
> AuthenticationToken.class.getName()*/, authParams)
>        .builder()
> 
> For AuthenticationTls, we need to do check the authParams, when the
> authParams is empty, we need to read TLS config from ClientBuilder,
> otherwise read from the authParams
> Compatibility
> 
> None.


Re: [DISCUSS] PIP-158: Split client TLS transport encryption from authentication

Posted by Zixuan Liu <no...@gmail.com>.
> Is it possible to deprecate the option to configure the certificates
in the AuthenticationTls class? I think it should be since the certs
are now configured as top level configuration, and they need only be
configured once.

We can't do that deprecate the option to configure the certificates in the
AuthenticationTls class, if do this, we need to deal with the
AuthenticationDataProvider, it is complex.

When using `AuthenticationTls` and TLS config of `ClientBuilder`, we only
use the `AuthenticationTls`, which can override the top level
configuration, it is easier.

I submitted https://github.com/apache/pulsar/pull/15634 for this PIP.

---
PIP has some changes:

~For `AuthenticationTls`, we need to check the authParams, when the
authParams is empty, we need to read TLS config from `ClientBuilder`,
otherwise read from the authParams, the authParams can override the config
from `ClientBuilder`.~

When using `AuthenticationTls` and TLS config of `ClientBuilder`, we only
use the `AuthenticationTls`, which can override the top level
configuration.

**Sort priority:** `AuthenticationTls` > TLS config of `ClientBuilder`.
---

Thanks,
Zixuan


Michael Marshall <mm...@apache.org> 于2022年6月1日周三 12:54写道:

> I know it's a bit late to respond, but just want to confirm that
> Zixuan is correct that we cannot deprecate the `AuthenticationTls`
> class because we rely on that class to set the authentication mode in
> the pulsar protocol. That mode is then used by the ServerCnx for
> authentication and authorization.
>
> Is it possible to deprecate the option to configure the certificates
> in the AuthenticationTls class? I think it should be since the certs
> are now configured as top level configuration, and they need only be
> configured once.
>
> Thanks,
> Michael
>
> On Sat, May 14, 2022 at 3:29 AM Zixuan Liu <no...@gmail.com> wrote:
> >
> > Hi Michael, It's not the same here.
> >
> > If you use AuthenticationTLS, which means you enable TLS authentication
> and
> > transport.
> >
> > ```
> > PulsarClient client = PulsarClient.builder()
> >         .serviceUrl("pulsar://my-host:6651")
> >         .tlsTrustCertsFilePath("/path/to/cacert.pem")
> >         .tlsKeyFilePath("/path/to/client-key.pem")
> >         .tlsCertificateFilePath("/path/to/client-cert.pem")
> >         .authentication(AuthenticationTls.class.getName()) //
> > AuthenticationTls will uses the above certificate.
> >         .build();
> > ```
> >
> > If you remove AuthenticationTLS, means we only use TLS transport.
> >
> > Thanks,
> > Zixuan
> >
> >
> >
> >
> > Michael Marshall <mm...@apache.org> 于2022年5月14日周六 13:27写道:
> >
> > > Thanks for your responses, Zixuan.
> > >
> > > I think it might make sense to eventually deprecate the
> > > AuthenticationTLS class, if only because I think it can be confusing
> > > to give users two ways to configure the same thing. However, that is a
> > > minor detail. For now, we'll need to support both.
> > >
> > > Thanks,
> > > Michael
> > >
> > > On Thu, May 12, 2022 at 4:43 AM Zixuan Liu <no...@gmail.com> wrote:
> > > >
> > > > You can see the code in the implementation part, this will be
> consistent
> > > > with the actual document.
> > > >
> > > > Zixuan Liu <no...@gmail.com> 于2022年5月12日周四 17:03写道:
> > > >
> > > > > Hi Michael,
> > > > >
> > > > > Thanks for your feedback!
> > > > >
> > > > > >  I notice that the PIP doesn't
> > > > > mention documentation. Since we're adding another way to configure
> > > > > mTLS, please make sure to document the recommended way that users
> > > > > should take advantage of this feature and how this feature relates
> to
> > > the
> > > > > existing AuthenticationTLS feature.
> > > > >
> > > > > Good idea, let me add a simple document that how to use TLS
> transport
> > > and
> > > > > TLS authentication.
> > > > >
> > > > > > We are removing the client's need to use the AuthenticationTLS
> class
> > > > > to perform TLS authentication of clients by the server.
> > > > >
> > > > > We don't remove the use of the AuthenticationTLS.
> > > > >
> > > > > > If a user wants to use TLS certificates for authorization, they
> can
> > > > > still put
> > > > > roles in their client certificates and continue to use the
> > > > > AuthenticationProviderTLS class to map a TLS certificate to a role
> on
> > > > > the server side.
> > > > >
> > > > > You are right, the users still can use the AuthenticationTLS to
> perform
> > > > > the TLS transport and TLS authentication.
> > > > >
> > > > > Currently, the AuthenticationTLS includes TLS transport and TLS
> > > > > authentication, if the user only uses the TLS transport, not use
> the
> > > TLS
> > > > > authentication, it is confusing, so I want to add a TLS transport
> > > config in
> > > > > `ClientBuilder`.
> > > > >
> > > > > Thanks,
> > > > > Zixuan
> > > > >
> > > > >
> > > > > Michael Marshall <mm...@apache.org> 于2022年5月12日周四 01:51写道:
> > > > >
> > > > >> I agree that the current state of this feature is a bit
> confusing, and
> > > > >> I think the proposed changes make sense. I notice that the PIP
> doesn't
> > > > >> mention documentation. Since we're adding another way to configure
> > > > >> mTLS, please make sure to document the recommended way that users
> > > > >> should take advantage of this feature and how this feature
> relates to
> > > the
> > > > >> existing AuthenticationTLS feature.
> > > > >>
> > > > >> In order to make sure I understand the feature correctly, can you
> > > > >> confirm that the following is correct?
> > > > >>
> > > > >> We are removing the client's need to use the AuthenticationTLS
> class
> > > > >> to perform TLS authentication of clients by the server. If a user
> > > > >> wants to use TLS certificates for authorization, they can still
> put
> > > > >> roles in their client certificates and continue to use the
> > > > >> AuthenticationProviderTLS class to map a TLS certificate to a
> role on
> > > > >> the server side.
> > > > >>
> > > > >> Thanks,
> > > > >> Michael
> > > > >>
> > > > >>
> > > > >>
> > > > >>
> > > > >>
> > > > >>
> > > > >> On Mon, May 9, 2022 at 12:58 AM Yunze Xu
> <yzxu@streamnative.io.invalid
> > > >
> > > > >> wrote:
> > > > >> >
> > > > >> > Thanks for your clarification. Let’s continue maintaining these
> > > configs
> > > > >> in
> > > > >> > `ClientBuilder`.
> > > > >> >
> > > > >> > Thanks,
> > > > >> > Yunze
> > > > >> >
> > > > >> >
> > > > >> >
> > > > >> >
> > > > >> > > 2022年5月9日 13:54,Zixuan Liu <no...@gmail.com> 写道:
> > > > >> > >
> > > > >> > > Hi Yunze,
> > > > >> > >
> > > > >> > > Thanks for your suggestion, your idea is great, but we have
> the
> > > > >> > > `tlsProtocols()` and `tlsCiphers()` in `ClientBuilder`, so I
> use
> > > this
> > > > >> style.
> > > > >> > >
> > > > >> > > Thanks,
> > > > >> > > Zixuan
> > > > >> > >
> > > > >> > > Yunze Xu <yz...@streamnative.io.invalid> 于2022年5月9日周一 13:31写道:
> > > > >> > >
> > > > >> > >> It totally LGTM. I have a suggestion that it might be better
> to
> > > > >> configure a
> > > > >> > >> class like `TlsConfiguration` instead of multiple TLS related
> > > configs
> > > > >> > >> added to
> > > > >> > >> `ClientBuilder`.
> > > > >> > >>
> > > > >> > >> Thanks,
> > > > >> > >> Yunze
> > > > >> > >>
> > > > >> > >>
> > > > >> > >>
> > > > >> > >>
> > > > >> > >>> 2022年4月24日 14:15,Zixuan Liu <no...@gmail.com> 写道:
> > > > >> > >>>
> > > > >> > >>> Hi Pulsar community,
> > > > >> > >>>
> > > > >> > >>> I open a https://github.com/apache/pulsar/issues/15289 for
> > > Split
> > > > >> client
> > > > >> > >> TLS
> > > > >> > >>> transport encryption from authentication.
> > > > >> > >>>
> > > > >> > >>> Let me know what you think.
> > > > >> > >>>
> > > > >> > >>> Thanks,
> > > > >> > >>> Zixuan
> > > > >> > >>>
> > > > >> > >>> ------
> > > > >> > >>>
> > > > >> > >>> Motivation
> > > > >> > >>>
> > > > >> > >>> The client supports TLS transport encryption and TLS
> > > > >> authentication, this
> > > > >> > >>> code so like:
> > > > >> > >>>
> > > > >> > >>> PulsarClient client = PulsarClient.builder()
> > > > >> > >>>               .serviceUrl("pulsar+ssl://localhost:6651")
> > > > >> > >>>               .tlsTrustCertsFilePath("/path/to/cacert.pem")
> > > > >> > >>>
>  .authentication(AuthenticationTls.class.getName(),
> > > > >> > >> authParams)
> > > > >> > >>>               .build()
> > > > >> > >>>
> > > > >> > >>> This causes an issue that cannot use other authentication
> with
> > > TLS
> > > > >> > >>> transport encryption, and also made our confusion if we use
> TLS
> > > > >> transport
> > > > >> > >>> encryption by setting authentication.
> > > > >> > >>> Goal
> > > > >> > >>>
> > > > >> > >>> Split client TLS transport encryption from authentication is
> > > used to
> > > > >> > >>> support TLS transport encryption with any authentication.
> > > > >> > >>> API Changes
> > > > >> > >>>
> > > > >> > >>>  - Add new methods in
> org.apache.pulsar.client.api.ClientBuilder
> > > > >> > >>>
> > > > >> > >>> public interface ClientBuilder extends Serializable,
> Cloneable {
> > > > >> > >>>   /**     * Set the path to the TLS key file.     *     *
> @param
> > > > >> > >>> tlsKeyFilePath     * @return the client builder instance
>  */
> > > > >> > >>>   ClientBuilder tlsKeyFilePath(String tlsKeyFilePath);
> > > > >> > >>>
> > > > >> > >>>   /**     * Set the path to the TLS certificate file.     *
> > >  *
> > > > >> > >>> @param tlsCertificateFilePath     * @return the client
> builder
> > > > >> > >>> instance     */
> > > > >> > >>>   ClientBuilder tlsCertificateFilePath(String
> > > > >> tlsCertificateFilePath);
> > > > >> > >>> }
> > > > >> > >>>
> > > > >> > >>> ImplementationTLS transport encryption
> > > > >> > >>>
> > > > >> > >>> We can call the tlsKeyFilePath(), tlsCertificateFilePath()
> and
> > > > >> > >>> tlsTrustCertsFilePath() to configurate the TLS transport
> > > > >> encryption, the
> > > > >> > >>> code so like:
> > > > >> > >>>
> > > > >> > >>> PulsarClient client = PulsarClient.builder()
> > > > >> > >>>       .serviceUrl("pulsar+ssl://my-host:6650")
> > > > >> > >>>       .tlsTrustCertsFilePath("/path/to/cacert.pem")
> > > > >> > >>>       .tlsKeyFilePath("/path/to/client-key.pem")
> > > > >> > >>>       .tlsCertificateFilePath("/path/to/client-cert.pem")
> > > > >> > >>>       .build();
> > > > >> > >>>
> > > > >> > >>> TLS transport encryption with any authentication
> > > > >> > >>>
> > > > >> > >>> We can call the tlsKeyFilePath(), tlsCertificateFilePath(),
> > > > >> > >>> tlsTrustCertsFilePath() and authentication() to configurate
> the
> > > TLS
> > > > >> > >>> transport encryption with any authentication, the code so
> like:
> > > > >> > >>>
> > > > >> > >>> PulsarClient client = PulsarClient.builder()
> > > > >> > >>>       .serviceUrl("pulsar+ssl://my-host:6650")
> > > > >> > >>>       .tlsTrustCertsFilePath("/path/to/cacert.pem")
> > > > >> > >>>       .tlsKeyFilePath("/path/to/client-key.pem")
> > > > >> > >>>       .tlsCertificateFilePath("/path/to/client-cert.pem")
> > > > >> > >>>       .authentication(AuthenticationTls.class.getName() /*
> > > > >> > >>> AuthenticationToken.class.getName()*/, authParams)
> > > > >> > >>>       .builder()
> > > > >> > >>>
> > > > >> > >>> For AuthenticationTls, we need to do check the authParams,
> when
> > > the
> > > > >> > >>> authParams is empty, we need to read TLS config from
> > > ClientBuilder,
> > > > >> > >>> otherwise read from the authParams
> > > > >> > >>> Compatibility
> > > > >> > >>>
> > > > >> > >>> None.
> > > > >> > >>
> > > > >> > >>
> > > > >> >
> > > > >>
> > > > >
> > >
>

Re: [DISCUSS] PIP-158: Split client TLS transport encryption from authentication

Posted by Michael Marshall <mm...@apache.org>.
I know it's a bit late to respond, but just want to confirm that
Zixuan is correct that we cannot deprecate the `AuthenticationTls`
class because we rely on that class to set the authentication mode in
the pulsar protocol. That mode is then used by the ServerCnx for
authentication and authorization.

Is it possible to deprecate the option to configure the certificates
in the AuthenticationTls class? I think it should be since the certs
are now configured as top level configuration, and they need only be
configured once.

Thanks,
Michael

On Sat, May 14, 2022 at 3:29 AM Zixuan Liu <no...@gmail.com> wrote:
>
> Hi Michael, It's not the same here.
>
> If you use AuthenticationTLS, which means you enable TLS authentication and
> transport.
>
> ```
> PulsarClient client = PulsarClient.builder()
>         .serviceUrl("pulsar://my-host:6651")
>         .tlsTrustCertsFilePath("/path/to/cacert.pem")
>         .tlsKeyFilePath("/path/to/client-key.pem")
>         .tlsCertificateFilePath("/path/to/client-cert.pem")
>         .authentication(AuthenticationTls.class.getName()) //
> AuthenticationTls will uses the above certificate.
>         .build();
> ```
>
> If you remove AuthenticationTLS, means we only use TLS transport.
>
> Thanks,
> Zixuan
>
>
>
>
> Michael Marshall <mm...@apache.org> 于2022年5月14日周六 13:27写道:
>
> > Thanks for your responses, Zixuan.
> >
> > I think it might make sense to eventually deprecate the
> > AuthenticationTLS class, if only because I think it can be confusing
> > to give users two ways to configure the same thing. However, that is a
> > minor detail. For now, we'll need to support both.
> >
> > Thanks,
> > Michael
> >
> > On Thu, May 12, 2022 at 4:43 AM Zixuan Liu <no...@gmail.com> wrote:
> > >
> > > You can see the code in the implementation part, this will be consistent
> > > with the actual document.
> > >
> > > Zixuan Liu <no...@gmail.com> 于2022年5月12日周四 17:03写道:
> > >
> > > > Hi Michael,
> > > >
> > > > Thanks for your feedback!
> > > >
> > > > >  I notice that the PIP doesn't
> > > > mention documentation. Since we're adding another way to configure
> > > > mTLS, please make sure to document the recommended way that users
> > > > should take advantage of this feature and how this feature relates to
> > the
> > > > existing AuthenticationTLS feature.
> > > >
> > > > Good idea, let me add a simple document that how to use TLS transport
> > and
> > > > TLS authentication.
> > > >
> > > > > We are removing the client's need to use the AuthenticationTLS class
> > > > to perform TLS authentication of clients by the server.
> > > >
> > > > We don't remove the use of the AuthenticationTLS.
> > > >
> > > > > If a user wants to use TLS certificates for authorization, they can
> > > > still put
> > > > roles in their client certificates and continue to use the
> > > > AuthenticationProviderTLS class to map a TLS certificate to a role on
> > > > the server side.
> > > >
> > > > You are right, the users still can use the AuthenticationTLS to perform
> > > > the TLS transport and TLS authentication.
> > > >
> > > > Currently, the AuthenticationTLS includes TLS transport and TLS
> > > > authentication, if the user only uses the TLS transport, not use the
> > TLS
> > > > authentication, it is confusing, so I want to add a TLS transport
> > config in
> > > > `ClientBuilder`.
> > > >
> > > > Thanks,
> > > > Zixuan
> > > >
> > > >
> > > > Michael Marshall <mm...@apache.org> 于2022年5月12日周四 01:51写道:
> > > >
> > > >> I agree that the current state of this feature is a bit confusing, and
> > > >> I think the proposed changes make sense. I notice that the PIP doesn't
> > > >> mention documentation. Since we're adding another way to configure
> > > >> mTLS, please make sure to document the recommended way that users
> > > >> should take advantage of this feature and how this feature relates to
> > the
> > > >> existing AuthenticationTLS feature.
> > > >>
> > > >> In order to make sure I understand the feature correctly, can you
> > > >> confirm that the following is correct?
> > > >>
> > > >> We are removing the client's need to use the AuthenticationTLS class
> > > >> to perform TLS authentication of clients by the server. If a user
> > > >> wants to use TLS certificates for authorization, they can still put
> > > >> roles in their client certificates and continue to use the
> > > >> AuthenticationProviderTLS class to map a TLS certificate to a role on
> > > >> the server side.
> > > >>
> > > >> Thanks,
> > > >> Michael
> > > >>
> > > >>
> > > >>
> > > >>
> > > >>
> > > >>
> > > >> On Mon, May 9, 2022 at 12:58 AM Yunze Xu <yzxu@streamnative.io.invalid
> > >
> > > >> wrote:
> > > >> >
> > > >> > Thanks for your clarification. Let’s continue maintaining these
> > configs
> > > >> in
> > > >> > `ClientBuilder`.
> > > >> >
> > > >> > Thanks,
> > > >> > Yunze
> > > >> >
> > > >> >
> > > >> >
> > > >> >
> > > >> > > 2022年5月9日 13:54,Zixuan Liu <no...@gmail.com> 写道:
> > > >> > >
> > > >> > > Hi Yunze,
> > > >> > >
> > > >> > > Thanks for your suggestion, your idea is great, but we have the
> > > >> > > `tlsProtocols()` and `tlsCiphers()` in `ClientBuilder`, so I use
> > this
> > > >> style.
> > > >> > >
> > > >> > > Thanks,
> > > >> > > Zixuan
> > > >> > >
> > > >> > > Yunze Xu <yz...@streamnative.io.invalid> 于2022年5月9日周一 13:31写道:
> > > >> > >
> > > >> > >> It totally LGTM. I have a suggestion that it might be better to
> > > >> configure a
> > > >> > >> class like `TlsConfiguration` instead of multiple TLS related
> > configs
> > > >> > >> added to
> > > >> > >> `ClientBuilder`.
> > > >> > >>
> > > >> > >> Thanks,
> > > >> > >> Yunze
> > > >> > >>
> > > >> > >>
> > > >> > >>
> > > >> > >>
> > > >> > >>> 2022年4月24日 14:15,Zixuan Liu <no...@gmail.com> 写道:
> > > >> > >>>
> > > >> > >>> Hi Pulsar community,
> > > >> > >>>
> > > >> > >>> I open a https://github.com/apache/pulsar/issues/15289 for
> > Split
> > > >> client
> > > >> > >> TLS
> > > >> > >>> transport encryption from authentication.
> > > >> > >>>
> > > >> > >>> Let me know what you think.
> > > >> > >>>
> > > >> > >>> Thanks,
> > > >> > >>> Zixuan
> > > >> > >>>
> > > >> > >>> ------
> > > >> > >>>
> > > >> > >>> Motivation
> > > >> > >>>
> > > >> > >>> The client supports TLS transport encryption and TLS
> > > >> authentication, this
> > > >> > >>> code so like:
> > > >> > >>>
> > > >> > >>> PulsarClient client = PulsarClient.builder()
> > > >> > >>>               .serviceUrl("pulsar+ssl://localhost:6651")
> > > >> > >>>               .tlsTrustCertsFilePath("/path/to/cacert.pem")
> > > >> > >>>               .authentication(AuthenticationTls.class.getName(),
> > > >> > >> authParams)
> > > >> > >>>               .build()
> > > >> > >>>
> > > >> > >>> This causes an issue that cannot use other authentication with
> > TLS
> > > >> > >>> transport encryption, and also made our confusion if we use TLS
> > > >> transport
> > > >> > >>> encryption by setting authentication.
> > > >> > >>> Goal
> > > >> > >>>
> > > >> > >>> Split client TLS transport encryption from authentication is
> > used to
> > > >> > >>> support TLS transport encryption with any authentication.
> > > >> > >>> API Changes
> > > >> > >>>
> > > >> > >>>  - Add new methods in org.apache.pulsar.client.api.ClientBuilder
> > > >> > >>>
> > > >> > >>> public interface ClientBuilder extends Serializable, Cloneable {
> > > >> > >>>   /**     * Set the path to the TLS key file.     *     * @param
> > > >> > >>> tlsKeyFilePath     * @return the client builder instance     */
> > > >> > >>>   ClientBuilder tlsKeyFilePath(String tlsKeyFilePath);
> > > >> > >>>
> > > >> > >>>   /**     * Set the path to the TLS certificate file.     *
> >  *
> > > >> > >>> @param tlsCertificateFilePath     * @return the client builder
> > > >> > >>> instance     */
> > > >> > >>>   ClientBuilder tlsCertificateFilePath(String
> > > >> tlsCertificateFilePath);
> > > >> > >>> }
> > > >> > >>>
> > > >> > >>> ImplementationTLS transport encryption
> > > >> > >>>
> > > >> > >>> We can call the tlsKeyFilePath(), tlsCertificateFilePath() and
> > > >> > >>> tlsTrustCertsFilePath() to configurate the TLS transport
> > > >> encryption, the
> > > >> > >>> code so like:
> > > >> > >>>
> > > >> > >>> PulsarClient client = PulsarClient.builder()
> > > >> > >>>       .serviceUrl("pulsar+ssl://my-host:6650")
> > > >> > >>>       .tlsTrustCertsFilePath("/path/to/cacert.pem")
> > > >> > >>>       .tlsKeyFilePath("/path/to/client-key.pem")
> > > >> > >>>       .tlsCertificateFilePath("/path/to/client-cert.pem")
> > > >> > >>>       .build();
> > > >> > >>>
> > > >> > >>> TLS transport encryption with any authentication
> > > >> > >>>
> > > >> > >>> We can call the tlsKeyFilePath(), tlsCertificateFilePath(),
> > > >> > >>> tlsTrustCertsFilePath() and authentication() to configurate the
> > TLS
> > > >> > >>> transport encryption with any authentication, the code so like:
> > > >> > >>>
> > > >> > >>> PulsarClient client = PulsarClient.builder()
> > > >> > >>>       .serviceUrl("pulsar+ssl://my-host:6650")
> > > >> > >>>       .tlsTrustCertsFilePath("/path/to/cacert.pem")
> > > >> > >>>       .tlsKeyFilePath("/path/to/client-key.pem")
> > > >> > >>>       .tlsCertificateFilePath("/path/to/client-cert.pem")
> > > >> > >>>       .authentication(AuthenticationTls.class.getName() /*
> > > >> > >>> AuthenticationToken.class.getName()*/, authParams)
> > > >> > >>>       .builder()
> > > >> > >>>
> > > >> > >>> For AuthenticationTls, we need to do check the authParams, when
> > the
> > > >> > >>> authParams is empty, we need to read TLS config from
> > ClientBuilder,
> > > >> > >>> otherwise read from the authParams
> > > >> > >>> Compatibility
> > > >> > >>>
> > > >> > >>> None.
> > > >> > >>
> > > >> > >>
> > > >> >
> > > >>
> > > >
> >

Re: [DISCUSS] PIP-158: Split client TLS transport encryption from authentication

Posted by Zixuan Liu <no...@gmail.com>.
Hi Michael, It's not the same here.

If you use AuthenticationTLS, which means you enable TLS authentication and
transport.

```
PulsarClient client = PulsarClient.builder()
        .serviceUrl("pulsar://my-host:6651")
        .tlsTrustCertsFilePath("/path/to/cacert.pem")
        .tlsKeyFilePath("/path/to/client-key.pem")
        .tlsCertificateFilePath("/path/to/client-cert.pem")
        .authentication(AuthenticationTls.class.getName()) //
AuthenticationTls will uses the above certificate.
        .build();
```

If you remove AuthenticationTLS, means we only use TLS transport.

Thanks,
Zixuan




Michael Marshall <mm...@apache.org> 于2022年5月14日周六 13:27写道:

> Thanks for your responses, Zixuan.
>
> I think it might make sense to eventually deprecate the
> AuthenticationTLS class, if only because I think it can be confusing
> to give users two ways to configure the same thing. However, that is a
> minor detail. For now, we'll need to support both.
>
> Thanks,
> Michael
>
> On Thu, May 12, 2022 at 4:43 AM Zixuan Liu <no...@gmail.com> wrote:
> >
> > You can see the code in the implementation part, this will be consistent
> > with the actual document.
> >
> > Zixuan Liu <no...@gmail.com> 于2022年5月12日周四 17:03写道:
> >
> > > Hi Michael,
> > >
> > > Thanks for your feedback!
> > >
> > > >  I notice that the PIP doesn't
> > > mention documentation. Since we're adding another way to configure
> > > mTLS, please make sure to document the recommended way that users
> > > should take advantage of this feature and how this feature relates to
> the
> > > existing AuthenticationTLS feature.
> > >
> > > Good idea, let me add a simple document that how to use TLS transport
> and
> > > TLS authentication.
> > >
> > > > We are removing the client's need to use the AuthenticationTLS class
> > > to perform TLS authentication of clients by the server.
> > >
> > > We don't remove the use of the AuthenticationTLS.
> > >
> > > > If a user wants to use TLS certificates for authorization, they can
> > > still put
> > > roles in their client certificates and continue to use the
> > > AuthenticationProviderTLS class to map a TLS certificate to a role on
> > > the server side.
> > >
> > > You are right, the users still can use the AuthenticationTLS to perform
> > > the TLS transport and TLS authentication.
> > >
> > > Currently, the AuthenticationTLS includes TLS transport and TLS
> > > authentication, if the user only uses the TLS transport, not use the
> TLS
> > > authentication, it is confusing, so I want to add a TLS transport
> config in
> > > `ClientBuilder`.
> > >
> > > Thanks,
> > > Zixuan
> > >
> > >
> > > Michael Marshall <mm...@apache.org> 于2022年5月12日周四 01:51写道:
> > >
> > >> I agree that the current state of this feature is a bit confusing, and
> > >> I think the proposed changes make sense. I notice that the PIP doesn't
> > >> mention documentation. Since we're adding another way to configure
> > >> mTLS, please make sure to document the recommended way that users
> > >> should take advantage of this feature and how this feature relates to
> the
> > >> existing AuthenticationTLS feature.
> > >>
> > >> In order to make sure I understand the feature correctly, can you
> > >> confirm that the following is correct?
> > >>
> > >> We are removing the client's need to use the AuthenticationTLS class
> > >> to perform TLS authentication of clients by the server. If a user
> > >> wants to use TLS certificates for authorization, they can still put
> > >> roles in their client certificates and continue to use the
> > >> AuthenticationProviderTLS class to map a TLS certificate to a role on
> > >> the server side.
> > >>
> > >> Thanks,
> > >> Michael
> > >>
> > >>
> > >>
> > >>
> > >>
> > >>
> > >> On Mon, May 9, 2022 at 12:58 AM Yunze Xu <yzxu@streamnative.io.invalid
> >
> > >> wrote:
> > >> >
> > >> > Thanks for your clarification. Let’s continue maintaining these
> configs
> > >> in
> > >> > `ClientBuilder`.
> > >> >
> > >> > Thanks,
> > >> > Yunze
> > >> >
> > >> >
> > >> >
> > >> >
> > >> > > 2022年5月9日 13:54,Zixuan Liu <no...@gmail.com> 写道:
> > >> > >
> > >> > > Hi Yunze,
> > >> > >
> > >> > > Thanks for your suggestion, your idea is great, but we have the
> > >> > > `tlsProtocols()` and `tlsCiphers()` in `ClientBuilder`, so I use
> this
> > >> style.
> > >> > >
> > >> > > Thanks,
> > >> > > Zixuan
> > >> > >
> > >> > > Yunze Xu <yz...@streamnative.io.invalid> 于2022年5月9日周一 13:31写道:
> > >> > >
> > >> > >> It totally LGTM. I have a suggestion that it might be better to
> > >> configure a
> > >> > >> class like `TlsConfiguration` instead of multiple TLS related
> configs
> > >> > >> added to
> > >> > >> `ClientBuilder`.
> > >> > >>
> > >> > >> Thanks,
> > >> > >> Yunze
> > >> > >>
> > >> > >>
> > >> > >>
> > >> > >>
> > >> > >>> 2022年4月24日 14:15,Zixuan Liu <no...@gmail.com> 写道:
> > >> > >>>
> > >> > >>> Hi Pulsar community,
> > >> > >>>
> > >> > >>> I open a https://github.com/apache/pulsar/issues/15289 for
> Split
> > >> client
> > >> > >> TLS
> > >> > >>> transport encryption from authentication.
> > >> > >>>
> > >> > >>> Let me know what you think.
> > >> > >>>
> > >> > >>> Thanks,
> > >> > >>> Zixuan
> > >> > >>>
> > >> > >>> ------
> > >> > >>>
> > >> > >>> Motivation
> > >> > >>>
> > >> > >>> The client supports TLS transport encryption and TLS
> > >> authentication, this
> > >> > >>> code so like:
> > >> > >>>
> > >> > >>> PulsarClient client = PulsarClient.builder()
> > >> > >>>               .serviceUrl("pulsar+ssl://localhost:6651")
> > >> > >>>               .tlsTrustCertsFilePath("/path/to/cacert.pem")
> > >> > >>>               .authentication(AuthenticationTls.class.getName(),
> > >> > >> authParams)
> > >> > >>>               .build()
> > >> > >>>
> > >> > >>> This causes an issue that cannot use other authentication with
> TLS
> > >> > >>> transport encryption, and also made our confusion if we use TLS
> > >> transport
> > >> > >>> encryption by setting authentication.
> > >> > >>> Goal
> > >> > >>>
> > >> > >>> Split client TLS transport encryption from authentication is
> used to
> > >> > >>> support TLS transport encryption with any authentication.
> > >> > >>> API Changes
> > >> > >>>
> > >> > >>>  - Add new methods in org.apache.pulsar.client.api.ClientBuilder
> > >> > >>>
> > >> > >>> public interface ClientBuilder extends Serializable, Cloneable {
> > >> > >>>   /**     * Set the path to the TLS key file.     *     * @param
> > >> > >>> tlsKeyFilePath     * @return the client builder instance     */
> > >> > >>>   ClientBuilder tlsKeyFilePath(String tlsKeyFilePath);
> > >> > >>>
> > >> > >>>   /**     * Set the path to the TLS certificate file.     *
>  *
> > >> > >>> @param tlsCertificateFilePath     * @return the client builder
> > >> > >>> instance     */
> > >> > >>>   ClientBuilder tlsCertificateFilePath(String
> > >> tlsCertificateFilePath);
> > >> > >>> }
> > >> > >>>
> > >> > >>> ImplementationTLS transport encryption
> > >> > >>>
> > >> > >>> We can call the tlsKeyFilePath(), tlsCertificateFilePath() and
> > >> > >>> tlsTrustCertsFilePath() to configurate the TLS transport
> > >> encryption, the
> > >> > >>> code so like:
> > >> > >>>
> > >> > >>> PulsarClient client = PulsarClient.builder()
> > >> > >>>       .serviceUrl("pulsar+ssl://my-host:6650")
> > >> > >>>       .tlsTrustCertsFilePath("/path/to/cacert.pem")
> > >> > >>>       .tlsKeyFilePath("/path/to/client-key.pem")
> > >> > >>>       .tlsCertificateFilePath("/path/to/client-cert.pem")
> > >> > >>>       .build();
> > >> > >>>
> > >> > >>> TLS transport encryption with any authentication
> > >> > >>>
> > >> > >>> We can call the tlsKeyFilePath(), tlsCertificateFilePath(),
> > >> > >>> tlsTrustCertsFilePath() and authentication() to configurate the
> TLS
> > >> > >>> transport encryption with any authentication, the code so like:
> > >> > >>>
> > >> > >>> PulsarClient client = PulsarClient.builder()
> > >> > >>>       .serviceUrl("pulsar+ssl://my-host:6650")
> > >> > >>>       .tlsTrustCertsFilePath("/path/to/cacert.pem")
> > >> > >>>       .tlsKeyFilePath("/path/to/client-key.pem")
> > >> > >>>       .tlsCertificateFilePath("/path/to/client-cert.pem")
> > >> > >>>       .authentication(AuthenticationTls.class.getName() /*
> > >> > >>> AuthenticationToken.class.getName()*/, authParams)
> > >> > >>>       .builder()
> > >> > >>>
> > >> > >>> For AuthenticationTls, we need to do check the authParams, when
> the
> > >> > >>> authParams is empty, we need to read TLS config from
> ClientBuilder,
> > >> > >>> otherwise read from the authParams
> > >> > >>> Compatibility
> > >> > >>>
> > >> > >>> None.
> > >> > >>
> > >> > >>
> > >> >
> > >>
> > >
>

Re: [DISCUSS] PIP-158: Split client TLS transport encryption from authentication

Posted by Michael Marshall <mm...@apache.org>.
Thanks for your responses, Zixuan.

I think it might make sense to eventually deprecate the
AuthenticationTLS class, if only because I think it can be confusing
to give users two ways to configure the same thing. However, that is a
minor detail. For now, we'll need to support both.

Thanks,
Michael

On Thu, May 12, 2022 at 4:43 AM Zixuan Liu <no...@gmail.com> wrote:
>
> You can see the code in the implementation part, this will be consistent
> with the actual document.
>
> Zixuan Liu <no...@gmail.com> 于2022年5月12日周四 17:03写道:
>
> > Hi Michael,
> >
> > Thanks for your feedback!
> >
> > >  I notice that the PIP doesn't
> > mention documentation. Since we're adding another way to configure
> > mTLS, please make sure to document the recommended way that users
> > should take advantage of this feature and how this feature relates to the
> > existing AuthenticationTLS feature.
> >
> > Good idea, let me add a simple document that how to use TLS transport and
> > TLS authentication.
> >
> > > We are removing the client's need to use the AuthenticationTLS class
> > to perform TLS authentication of clients by the server.
> >
> > We don't remove the use of the AuthenticationTLS.
> >
> > > If a user wants to use TLS certificates for authorization, they can
> > still put
> > roles in their client certificates and continue to use the
> > AuthenticationProviderTLS class to map a TLS certificate to a role on
> > the server side.
> >
> > You are right, the users still can use the AuthenticationTLS to perform
> > the TLS transport and TLS authentication.
> >
> > Currently, the AuthenticationTLS includes TLS transport and TLS
> > authentication, if the user only uses the TLS transport, not use the TLS
> > authentication, it is confusing, so I want to add a TLS transport config in
> > `ClientBuilder`.
> >
> > Thanks,
> > Zixuan
> >
> >
> > Michael Marshall <mm...@apache.org> 于2022年5月12日周四 01:51写道:
> >
> >> I agree that the current state of this feature is a bit confusing, and
> >> I think the proposed changes make sense. I notice that the PIP doesn't
> >> mention documentation. Since we're adding another way to configure
> >> mTLS, please make sure to document the recommended way that users
> >> should take advantage of this feature and how this feature relates to the
> >> existing AuthenticationTLS feature.
> >>
> >> In order to make sure I understand the feature correctly, can you
> >> confirm that the following is correct?
> >>
> >> We are removing the client's need to use the AuthenticationTLS class
> >> to perform TLS authentication of clients by the server. If a user
> >> wants to use TLS certificates for authorization, they can still put
> >> roles in their client certificates and continue to use the
> >> AuthenticationProviderTLS class to map a TLS certificate to a role on
> >> the server side.
> >>
> >> Thanks,
> >> Michael
> >>
> >>
> >>
> >>
> >>
> >>
> >> On Mon, May 9, 2022 at 12:58 AM Yunze Xu <yz...@streamnative.io.invalid>
> >> wrote:
> >> >
> >> > Thanks for your clarification. Let’s continue maintaining these configs
> >> in
> >> > `ClientBuilder`.
> >> >
> >> > Thanks,
> >> > Yunze
> >> >
> >> >
> >> >
> >> >
> >> > > 2022年5月9日 13:54,Zixuan Liu <no...@gmail.com> 写道:
> >> > >
> >> > > Hi Yunze,
> >> > >
> >> > > Thanks for your suggestion, your idea is great, but we have the
> >> > > `tlsProtocols()` and `tlsCiphers()` in `ClientBuilder`, so I use this
> >> style.
> >> > >
> >> > > Thanks,
> >> > > Zixuan
> >> > >
> >> > > Yunze Xu <yz...@streamnative.io.invalid> 于2022年5月9日周一 13:31写道:
> >> > >
> >> > >> It totally LGTM. I have a suggestion that it might be better to
> >> configure a
> >> > >> class like `TlsConfiguration` instead of multiple TLS related configs
> >> > >> added to
> >> > >> `ClientBuilder`.
> >> > >>
> >> > >> Thanks,
> >> > >> Yunze
> >> > >>
> >> > >>
> >> > >>
> >> > >>
> >> > >>> 2022年4月24日 14:15,Zixuan Liu <no...@gmail.com> 写道:
> >> > >>>
> >> > >>> Hi Pulsar community,
> >> > >>>
> >> > >>> I open a https://github.com/apache/pulsar/issues/15289 for Split
> >> client
> >> > >> TLS
> >> > >>> transport encryption from authentication.
> >> > >>>
> >> > >>> Let me know what you think.
> >> > >>>
> >> > >>> Thanks,
> >> > >>> Zixuan
> >> > >>>
> >> > >>> ------
> >> > >>>
> >> > >>> Motivation
> >> > >>>
> >> > >>> The client supports TLS transport encryption and TLS
> >> authentication, this
> >> > >>> code so like:
> >> > >>>
> >> > >>> PulsarClient client = PulsarClient.builder()
> >> > >>>               .serviceUrl("pulsar+ssl://localhost:6651")
> >> > >>>               .tlsTrustCertsFilePath("/path/to/cacert.pem")
> >> > >>>               .authentication(AuthenticationTls.class.getName(),
> >> > >> authParams)
> >> > >>>               .build()
> >> > >>>
> >> > >>> This causes an issue that cannot use other authentication with TLS
> >> > >>> transport encryption, and also made our confusion if we use TLS
> >> transport
> >> > >>> encryption by setting authentication.
> >> > >>> Goal
> >> > >>>
> >> > >>> Split client TLS transport encryption from authentication is used to
> >> > >>> support TLS transport encryption with any authentication.
> >> > >>> API Changes
> >> > >>>
> >> > >>>  - Add new methods in org.apache.pulsar.client.api.ClientBuilder
> >> > >>>
> >> > >>> public interface ClientBuilder extends Serializable, Cloneable {
> >> > >>>   /**     * Set the path to the TLS key file.     *     * @param
> >> > >>> tlsKeyFilePath     * @return the client builder instance     */
> >> > >>>   ClientBuilder tlsKeyFilePath(String tlsKeyFilePath);
> >> > >>>
> >> > >>>   /**     * Set the path to the TLS certificate file.     *     *
> >> > >>> @param tlsCertificateFilePath     * @return the client builder
> >> > >>> instance     */
> >> > >>>   ClientBuilder tlsCertificateFilePath(String
> >> tlsCertificateFilePath);
> >> > >>> }
> >> > >>>
> >> > >>> ImplementationTLS transport encryption
> >> > >>>
> >> > >>> We can call the tlsKeyFilePath(), tlsCertificateFilePath() and
> >> > >>> tlsTrustCertsFilePath() to configurate the TLS transport
> >> encryption, the
> >> > >>> code so like:
> >> > >>>
> >> > >>> PulsarClient client = PulsarClient.builder()
> >> > >>>       .serviceUrl("pulsar+ssl://my-host:6650")
> >> > >>>       .tlsTrustCertsFilePath("/path/to/cacert.pem")
> >> > >>>       .tlsKeyFilePath("/path/to/client-key.pem")
> >> > >>>       .tlsCertificateFilePath("/path/to/client-cert.pem")
> >> > >>>       .build();
> >> > >>>
> >> > >>> TLS transport encryption with any authentication
> >> > >>>
> >> > >>> We can call the tlsKeyFilePath(), tlsCertificateFilePath(),
> >> > >>> tlsTrustCertsFilePath() and authentication() to configurate the TLS
> >> > >>> transport encryption with any authentication, the code so like:
> >> > >>>
> >> > >>> PulsarClient client = PulsarClient.builder()
> >> > >>>       .serviceUrl("pulsar+ssl://my-host:6650")
> >> > >>>       .tlsTrustCertsFilePath("/path/to/cacert.pem")
> >> > >>>       .tlsKeyFilePath("/path/to/client-key.pem")
> >> > >>>       .tlsCertificateFilePath("/path/to/client-cert.pem")
> >> > >>>       .authentication(AuthenticationTls.class.getName() /*
> >> > >>> AuthenticationToken.class.getName()*/, authParams)
> >> > >>>       .builder()
> >> > >>>
> >> > >>> For AuthenticationTls, we need to do check the authParams, when the
> >> > >>> authParams is empty, we need to read TLS config from ClientBuilder,
> >> > >>> otherwise read from the authParams
> >> > >>> Compatibility
> >> > >>>
> >> > >>> None.
> >> > >>
> >> > >>
> >> >
> >>
> >

Re: [DISCUSS] PIP-158: Split client TLS transport encryption from authentication

Posted by Zixuan Liu <no...@gmail.com>.
You can see the code in the implementation part, this will be consistent
with the actual document.

Zixuan Liu <no...@gmail.com> 于2022年5月12日周四 17:03写道:

> Hi Michael,
>
> Thanks for your feedback!
>
> >  I notice that the PIP doesn't
> mention documentation. Since we're adding another way to configure
> mTLS, please make sure to document the recommended way that users
> should take advantage of this feature and how this feature relates to the
> existing AuthenticationTLS feature.
>
> Good idea, let me add a simple document that how to use TLS transport and
> TLS authentication.
>
> > We are removing the client's need to use the AuthenticationTLS class
> to perform TLS authentication of clients by the server.
>
> We don't remove the use of the AuthenticationTLS.
>
> > If a user wants to use TLS certificates for authorization, they can
> still put
> roles in their client certificates and continue to use the
> AuthenticationProviderTLS class to map a TLS certificate to a role on
> the server side.
>
> You are right, the users still can use the AuthenticationTLS to perform
> the TLS transport and TLS authentication.
>
> Currently, the AuthenticationTLS includes TLS transport and TLS
> authentication, if the user only uses the TLS transport, not use the TLS
> authentication, it is confusing, so I want to add a TLS transport config in
> `ClientBuilder`.
>
> Thanks,
> Zixuan
>
>
> Michael Marshall <mm...@apache.org> 于2022年5月12日周四 01:51写道:
>
>> I agree that the current state of this feature is a bit confusing, and
>> I think the proposed changes make sense. I notice that the PIP doesn't
>> mention documentation. Since we're adding another way to configure
>> mTLS, please make sure to document the recommended way that users
>> should take advantage of this feature and how this feature relates to the
>> existing AuthenticationTLS feature.
>>
>> In order to make sure I understand the feature correctly, can you
>> confirm that the following is correct?
>>
>> We are removing the client's need to use the AuthenticationTLS class
>> to perform TLS authentication of clients by the server. If a user
>> wants to use TLS certificates for authorization, they can still put
>> roles in their client certificates and continue to use the
>> AuthenticationProviderTLS class to map a TLS certificate to a role on
>> the server side.
>>
>> Thanks,
>> Michael
>>
>>
>>
>>
>>
>>
>> On Mon, May 9, 2022 at 12:58 AM Yunze Xu <yz...@streamnative.io.invalid>
>> wrote:
>> >
>> > Thanks for your clarification. Let’s continue maintaining these configs
>> in
>> > `ClientBuilder`.
>> >
>> > Thanks,
>> > Yunze
>> >
>> >
>> >
>> >
>> > > 2022年5月9日 13:54,Zixuan Liu <no...@gmail.com> 写道:
>> > >
>> > > Hi Yunze,
>> > >
>> > > Thanks for your suggestion, your idea is great, but we have the
>> > > `tlsProtocols()` and `tlsCiphers()` in `ClientBuilder`, so I use this
>> style.
>> > >
>> > > Thanks,
>> > > Zixuan
>> > >
>> > > Yunze Xu <yz...@streamnative.io.invalid> 于2022年5月9日周一 13:31写道:
>> > >
>> > >> It totally LGTM. I have a suggestion that it might be better to
>> configure a
>> > >> class like `TlsConfiguration` instead of multiple TLS related configs
>> > >> added to
>> > >> `ClientBuilder`.
>> > >>
>> > >> Thanks,
>> > >> Yunze
>> > >>
>> > >>
>> > >>
>> > >>
>> > >>> 2022年4月24日 14:15,Zixuan Liu <no...@gmail.com> 写道:
>> > >>>
>> > >>> Hi Pulsar community,
>> > >>>
>> > >>> I open a https://github.com/apache/pulsar/issues/15289 for Split
>> client
>> > >> TLS
>> > >>> transport encryption from authentication.
>> > >>>
>> > >>> Let me know what you think.
>> > >>>
>> > >>> Thanks,
>> > >>> Zixuan
>> > >>>
>> > >>> ------
>> > >>>
>> > >>> Motivation
>> > >>>
>> > >>> The client supports TLS transport encryption and TLS
>> authentication, this
>> > >>> code so like:
>> > >>>
>> > >>> PulsarClient client = PulsarClient.builder()
>> > >>>               .serviceUrl("pulsar+ssl://localhost:6651")
>> > >>>               .tlsTrustCertsFilePath("/path/to/cacert.pem")
>> > >>>               .authentication(AuthenticationTls.class.getName(),
>> > >> authParams)
>> > >>>               .build()
>> > >>>
>> > >>> This causes an issue that cannot use other authentication with TLS
>> > >>> transport encryption, and also made our confusion if we use TLS
>> transport
>> > >>> encryption by setting authentication.
>> > >>> Goal
>> > >>>
>> > >>> Split client TLS transport encryption from authentication is used to
>> > >>> support TLS transport encryption with any authentication.
>> > >>> API Changes
>> > >>>
>> > >>>  - Add new methods in org.apache.pulsar.client.api.ClientBuilder
>> > >>>
>> > >>> public interface ClientBuilder extends Serializable, Cloneable {
>> > >>>   /**     * Set the path to the TLS key file.     *     * @param
>> > >>> tlsKeyFilePath     * @return the client builder instance     */
>> > >>>   ClientBuilder tlsKeyFilePath(String tlsKeyFilePath);
>> > >>>
>> > >>>   /**     * Set the path to the TLS certificate file.     *     *
>> > >>> @param tlsCertificateFilePath     * @return the client builder
>> > >>> instance     */
>> > >>>   ClientBuilder tlsCertificateFilePath(String
>> tlsCertificateFilePath);
>> > >>> }
>> > >>>
>> > >>> ImplementationTLS transport encryption
>> > >>>
>> > >>> We can call the tlsKeyFilePath(), tlsCertificateFilePath() and
>> > >>> tlsTrustCertsFilePath() to configurate the TLS transport
>> encryption, the
>> > >>> code so like:
>> > >>>
>> > >>> PulsarClient client = PulsarClient.builder()
>> > >>>       .serviceUrl("pulsar+ssl://my-host:6650")
>> > >>>       .tlsTrustCertsFilePath("/path/to/cacert.pem")
>> > >>>       .tlsKeyFilePath("/path/to/client-key.pem")
>> > >>>       .tlsCertificateFilePath("/path/to/client-cert.pem")
>> > >>>       .build();
>> > >>>
>> > >>> TLS transport encryption with any authentication
>> > >>>
>> > >>> We can call the tlsKeyFilePath(), tlsCertificateFilePath(),
>> > >>> tlsTrustCertsFilePath() and authentication() to configurate the TLS
>> > >>> transport encryption with any authentication, the code so like:
>> > >>>
>> > >>> PulsarClient client = PulsarClient.builder()
>> > >>>       .serviceUrl("pulsar+ssl://my-host:6650")
>> > >>>       .tlsTrustCertsFilePath("/path/to/cacert.pem")
>> > >>>       .tlsKeyFilePath("/path/to/client-key.pem")
>> > >>>       .tlsCertificateFilePath("/path/to/client-cert.pem")
>> > >>>       .authentication(AuthenticationTls.class.getName() /*
>> > >>> AuthenticationToken.class.getName()*/, authParams)
>> > >>>       .builder()
>> > >>>
>> > >>> For AuthenticationTls, we need to do check the authParams, when the
>> > >>> authParams is empty, we need to read TLS config from ClientBuilder,
>> > >>> otherwise read from the authParams
>> > >>> Compatibility
>> > >>>
>> > >>> None.
>> > >>
>> > >>
>> >
>>
>

Re: [DISCUSS] PIP-158: Split client TLS transport encryption from authentication

Posted by Zixuan Liu <no...@gmail.com>.
Hi Michael,

Thanks for your feedback!

>  I notice that the PIP doesn't
mention documentation. Since we're adding another way to configure
mTLS, please make sure to document the recommended way that users
should take advantage of this feature and how this feature relates to the
existing AuthenticationTLS feature.

Good idea, let me add a simple document that how to use TLS transport and
TLS authentication.

> We are removing the client's need to use the AuthenticationTLS class
to perform TLS authentication of clients by the server.

We don't remove the use of the AuthenticationTLS.

> If a user wants to use TLS certificates for authorization, they can still
put
roles in their client certificates and continue to use the
AuthenticationProviderTLS class to map a TLS certificate to a role on
the server side.

You are right, the users still can use the AuthenticationTLS to perform the
TLS transport and TLS authentication.

Currently, the AuthenticationTLS includes TLS transport and TLS
authentication, if the user only uses the TLS transport, not use the TLS
authentication, it is confusing, so I want to add a TLS transport config in
`ClientBuilder`.

Thanks,
Zixuan


Michael Marshall <mm...@apache.org> 于2022年5月12日周四 01:51写道:

> I agree that the current state of this feature is a bit confusing, and
> I think the proposed changes make sense. I notice that the PIP doesn't
> mention documentation. Since we're adding another way to configure
> mTLS, please make sure to document the recommended way that users
> should take advantage of this feature and how this feature relates to the
> existing AuthenticationTLS feature.
>
> In order to make sure I understand the feature correctly, can you
> confirm that the following is correct?
>
> We are removing the client's need to use the AuthenticationTLS class
> to perform TLS authentication of clients by the server. If a user
> wants to use TLS certificates for authorization, they can still put
> roles in their client certificates and continue to use the
> AuthenticationProviderTLS class to map a TLS certificate to a role on
> the server side.
>
> Thanks,
> Michael
>
>
>
>
>
>
> On Mon, May 9, 2022 at 12:58 AM Yunze Xu <yz...@streamnative.io.invalid>
> wrote:
> >
> > Thanks for your clarification. Let’s continue maintaining these configs
> in
> > `ClientBuilder`.
> >
> > Thanks,
> > Yunze
> >
> >
> >
> >
> > > 2022年5月9日 13:54,Zixuan Liu <no...@gmail.com> 写道:
> > >
> > > Hi Yunze,
> > >
> > > Thanks for your suggestion, your idea is great, but we have the
> > > `tlsProtocols()` and `tlsCiphers()` in `ClientBuilder`, so I use this
> style.
> > >
> > > Thanks,
> > > Zixuan
> > >
> > > Yunze Xu <yz...@streamnative.io.invalid> 于2022年5月9日周一 13:31写道:
> > >
> > >> It totally LGTM. I have a suggestion that it might be better to
> configure a
> > >> class like `TlsConfiguration` instead of multiple TLS related configs
> > >> added to
> > >> `ClientBuilder`.
> > >>
> > >> Thanks,
> > >> Yunze
> > >>
> > >>
> > >>
> > >>
> > >>> 2022年4月24日 14:15,Zixuan Liu <no...@gmail.com> 写道:
> > >>>
> > >>> Hi Pulsar community,
> > >>>
> > >>> I open a https://github.com/apache/pulsar/issues/15289 for Split
> client
> > >> TLS
> > >>> transport encryption from authentication.
> > >>>
> > >>> Let me know what you think.
> > >>>
> > >>> Thanks,
> > >>> Zixuan
> > >>>
> > >>> ------
> > >>>
> > >>> Motivation
> > >>>
> > >>> The client supports TLS transport encryption and TLS authentication,
> this
> > >>> code so like:
> > >>>
> > >>> PulsarClient client = PulsarClient.builder()
> > >>>               .serviceUrl("pulsar+ssl://localhost:6651")
> > >>>               .tlsTrustCertsFilePath("/path/to/cacert.pem")
> > >>>               .authentication(AuthenticationTls.class.getName(),
> > >> authParams)
> > >>>               .build()
> > >>>
> > >>> This causes an issue that cannot use other authentication with TLS
> > >>> transport encryption, and also made our confusion if we use TLS
> transport
> > >>> encryption by setting authentication.
> > >>> Goal
> > >>>
> > >>> Split client TLS transport encryption from authentication is used to
> > >>> support TLS transport encryption with any authentication.
> > >>> API Changes
> > >>>
> > >>>  - Add new methods in org.apache.pulsar.client.api.ClientBuilder
> > >>>
> > >>> public interface ClientBuilder extends Serializable, Cloneable {
> > >>>   /**     * Set the path to the TLS key file.     *     * @param
> > >>> tlsKeyFilePath     * @return the client builder instance     */
> > >>>   ClientBuilder tlsKeyFilePath(String tlsKeyFilePath);
> > >>>
> > >>>   /**     * Set the path to the TLS certificate file.     *     *
> > >>> @param tlsCertificateFilePath     * @return the client builder
> > >>> instance     */
> > >>>   ClientBuilder tlsCertificateFilePath(String
> tlsCertificateFilePath);
> > >>> }
> > >>>
> > >>> ImplementationTLS transport encryption
> > >>>
> > >>> We can call the tlsKeyFilePath(), tlsCertificateFilePath() and
> > >>> tlsTrustCertsFilePath() to configurate the TLS transport encryption,
> the
> > >>> code so like:
> > >>>
> > >>> PulsarClient client = PulsarClient.builder()
> > >>>       .serviceUrl("pulsar+ssl://my-host:6650")
> > >>>       .tlsTrustCertsFilePath("/path/to/cacert.pem")
> > >>>       .tlsKeyFilePath("/path/to/client-key.pem")
> > >>>       .tlsCertificateFilePath("/path/to/client-cert.pem")
> > >>>       .build();
> > >>>
> > >>> TLS transport encryption with any authentication
> > >>>
> > >>> We can call the tlsKeyFilePath(), tlsCertificateFilePath(),
> > >>> tlsTrustCertsFilePath() and authentication() to configurate the TLS
> > >>> transport encryption with any authentication, the code so like:
> > >>>
> > >>> PulsarClient client = PulsarClient.builder()
> > >>>       .serviceUrl("pulsar+ssl://my-host:6650")
> > >>>       .tlsTrustCertsFilePath("/path/to/cacert.pem")
> > >>>       .tlsKeyFilePath("/path/to/client-key.pem")
> > >>>       .tlsCertificateFilePath("/path/to/client-cert.pem")
> > >>>       .authentication(AuthenticationTls.class.getName() /*
> > >>> AuthenticationToken.class.getName()*/, authParams)
> > >>>       .builder()
> > >>>
> > >>> For AuthenticationTls, we need to do check the authParams, when the
> > >>> authParams is empty, we need to read TLS config from ClientBuilder,
> > >>> otherwise read from the authParams
> > >>> Compatibility
> > >>>
> > >>> None.
> > >>
> > >>
> >
>

Re: [DISCUSS] PIP-158: Split client TLS transport encryption from authentication

Posted by Michael Marshall <mm...@apache.org>.
I agree that the current state of this feature is a bit confusing, and
I think the proposed changes make sense. I notice that the PIP doesn't
mention documentation. Since we're adding another way to configure
mTLS, please make sure to document the recommended way that users
should take advantage of this feature and how this feature relates to the
existing AuthenticationTLS feature.

In order to make sure I understand the feature correctly, can you
confirm that the following is correct?

We are removing the client's need to use the AuthenticationTLS class
to perform TLS authentication of clients by the server. If a user
wants to use TLS certificates for authorization, they can still put
roles in their client certificates and continue to use the
AuthenticationProviderTLS class to map a TLS certificate to a role on
the server side.

Thanks,
Michael






On Mon, May 9, 2022 at 12:58 AM Yunze Xu <yz...@streamnative.io.invalid> wrote:
>
> Thanks for your clarification. Let’s continue maintaining these configs in
> `ClientBuilder`.
>
> Thanks,
> Yunze
>
>
>
>
> > 2022年5月9日 13:54,Zixuan Liu <no...@gmail.com> 写道:
> >
> > Hi Yunze,
> >
> > Thanks for your suggestion, your idea is great, but we have the
> > `tlsProtocols()` and `tlsCiphers()` in `ClientBuilder`, so I use this style.
> >
> > Thanks,
> > Zixuan
> >
> > Yunze Xu <yz...@streamnative.io.invalid> 于2022年5月9日周一 13:31写道:
> >
> >> It totally LGTM. I have a suggestion that it might be better to configure a
> >> class like `TlsConfiguration` instead of multiple TLS related configs
> >> added to
> >> `ClientBuilder`.
> >>
> >> Thanks,
> >> Yunze
> >>
> >>
> >>
> >>
> >>> 2022年4月24日 14:15,Zixuan Liu <no...@gmail.com> 写道:
> >>>
> >>> Hi Pulsar community,
> >>>
> >>> I open a https://github.com/apache/pulsar/issues/15289 for Split client
> >> TLS
> >>> transport encryption from authentication.
> >>>
> >>> Let me know what you think.
> >>>
> >>> Thanks,
> >>> Zixuan
> >>>
> >>> ------
> >>>
> >>> Motivation
> >>>
> >>> The client supports TLS transport encryption and TLS authentication, this
> >>> code so like:
> >>>
> >>> PulsarClient client = PulsarClient.builder()
> >>>               .serviceUrl("pulsar+ssl://localhost:6651")
> >>>               .tlsTrustCertsFilePath("/path/to/cacert.pem")
> >>>               .authentication(AuthenticationTls.class.getName(),
> >> authParams)
> >>>               .build()
> >>>
> >>> This causes an issue that cannot use other authentication with TLS
> >>> transport encryption, and also made our confusion if we use TLS transport
> >>> encryption by setting authentication.
> >>> Goal
> >>>
> >>> Split client TLS transport encryption from authentication is used to
> >>> support TLS transport encryption with any authentication.
> >>> API Changes
> >>>
> >>>  - Add new methods in org.apache.pulsar.client.api.ClientBuilder
> >>>
> >>> public interface ClientBuilder extends Serializable, Cloneable {
> >>>   /**     * Set the path to the TLS key file.     *     * @param
> >>> tlsKeyFilePath     * @return the client builder instance     */
> >>>   ClientBuilder tlsKeyFilePath(String tlsKeyFilePath);
> >>>
> >>>   /**     * Set the path to the TLS certificate file.     *     *
> >>> @param tlsCertificateFilePath     * @return the client builder
> >>> instance     */
> >>>   ClientBuilder tlsCertificateFilePath(String tlsCertificateFilePath);
> >>> }
> >>>
> >>> ImplementationTLS transport encryption
> >>>
> >>> We can call the tlsKeyFilePath(), tlsCertificateFilePath() and
> >>> tlsTrustCertsFilePath() to configurate the TLS transport encryption, the
> >>> code so like:
> >>>
> >>> PulsarClient client = PulsarClient.builder()
> >>>       .serviceUrl("pulsar+ssl://my-host:6650")
> >>>       .tlsTrustCertsFilePath("/path/to/cacert.pem")
> >>>       .tlsKeyFilePath("/path/to/client-key.pem")
> >>>       .tlsCertificateFilePath("/path/to/client-cert.pem")
> >>>       .build();
> >>>
> >>> TLS transport encryption with any authentication
> >>>
> >>> We can call the tlsKeyFilePath(), tlsCertificateFilePath(),
> >>> tlsTrustCertsFilePath() and authentication() to configurate the TLS
> >>> transport encryption with any authentication, the code so like:
> >>>
> >>> PulsarClient client = PulsarClient.builder()
> >>>       .serviceUrl("pulsar+ssl://my-host:6650")
> >>>       .tlsTrustCertsFilePath("/path/to/cacert.pem")
> >>>       .tlsKeyFilePath("/path/to/client-key.pem")
> >>>       .tlsCertificateFilePath("/path/to/client-cert.pem")
> >>>       .authentication(AuthenticationTls.class.getName() /*
> >>> AuthenticationToken.class.getName()*/, authParams)
> >>>       .builder()
> >>>
> >>> For AuthenticationTls, we need to do check the authParams, when the
> >>> authParams is empty, we need to read TLS config from ClientBuilder,
> >>> otherwise read from the authParams
> >>> Compatibility
> >>>
> >>> None.
> >>
> >>
>

Re: [DISCUSS] PIP-158: Split client TLS transport encryption from authentication

Posted by Yunze Xu <yz...@streamnative.io.INVALID>.
Thanks for your clarification. Let’s continue maintaining these configs in
`ClientBuilder`.

Thanks,
Yunze




> 2022年5月9日 13:54,Zixuan Liu <no...@gmail.com> 写道:
> 
> Hi Yunze,
> 
> Thanks for your suggestion, your idea is great, but we have the
> `tlsProtocols()` and `tlsCiphers()` in `ClientBuilder`, so I use this style.
> 
> Thanks,
> Zixuan
> 
> Yunze Xu <yz...@streamnative.io.invalid> 于2022年5月9日周一 13:31写道:
> 
>> It totally LGTM. I have a suggestion that it might be better to configure a
>> class like `TlsConfiguration` instead of multiple TLS related configs
>> added to
>> `ClientBuilder`.
>> 
>> Thanks,
>> Yunze
>> 
>> 
>> 
>> 
>>> 2022年4月24日 14:15,Zixuan Liu <no...@gmail.com> 写道:
>>> 
>>> Hi Pulsar community,
>>> 
>>> I open a https://github.com/apache/pulsar/issues/15289 for Split client
>> TLS
>>> transport encryption from authentication.
>>> 
>>> Let me know what you think.
>>> 
>>> Thanks,
>>> Zixuan
>>> 
>>> ------
>>> 
>>> Motivation
>>> 
>>> The client supports TLS transport encryption and TLS authentication, this
>>> code so like:
>>> 
>>> PulsarClient client = PulsarClient.builder()
>>>               .serviceUrl("pulsar+ssl://localhost:6651")
>>>               .tlsTrustCertsFilePath("/path/to/cacert.pem")
>>>               .authentication(AuthenticationTls.class.getName(),
>> authParams)
>>>               .build()
>>> 
>>> This causes an issue that cannot use other authentication with TLS
>>> transport encryption, and also made our confusion if we use TLS transport
>>> encryption by setting authentication.
>>> Goal
>>> 
>>> Split client TLS transport encryption from authentication is used to
>>> support TLS transport encryption with any authentication.
>>> API Changes
>>> 
>>>  - Add new methods in org.apache.pulsar.client.api.ClientBuilder
>>> 
>>> public interface ClientBuilder extends Serializable, Cloneable {
>>>   /**     * Set the path to the TLS key file.     *     * @param
>>> tlsKeyFilePath     * @return the client builder instance     */
>>>   ClientBuilder tlsKeyFilePath(String tlsKeyFilePath);
>>> 
>>>   /**     * Set the path to the TLS certificate file.     *     *
>>> @param tlsCertificateFilePath     * @return the client builder
>>> instance     */
>>>   ClientBuilder tlsCertificateFilePath(String tlsCertificateFilePath);
>>> }
>>> 
>>> ImplementationTLS transport encryption
>>> 
>>> We can call the tlsKeyFilePath(), tlsCertificateFilePath() and
>>> tlsTrustCertsFilePath() to configurate the TLS transport encryption, the
>>> code so like:
>>> 
>>> PulsarClient client = PulsarClient.builder()
>>>       .serviceUrl("pulsar+ssl://my-host:6650")
>>>       .tlsTrustCertsFilePath("/path/to/cacert.pem")
>>>       .tlsKeyFilePath("/path/to/client-key.pem")
>>>       .tlsCertificateFilePath("/path/to/client-cert.pem")
>>>       .build();
>>> 
>>> TLS transport encryption with any authentication
>>> 
>>> We can call the tlsKeyFilePath(), tlsCertificateFilePath(),
>>> tlsTrustCertsFilePath() and authentication() to configurate the TLS
>>> transport encryption with any authentication, the code so like:
>>> 
>>> PulsarClient client = PulsarClient.builder()
>>>       .serviceUrl("pulsar+ssl://my-host:6650")
>>>       .tlsTrustCertsFilePath("/path/to/cacert.pem")
>>>       .tlsKeyFilePath("/path/to/client-key.pem")
>>>       .tlsCertificateFilePath("/path/to/client-cert.pem")
>>>       .authentication(AuthenticationTls.class.getName() /*
>>> AuthenticationToken.class.getName()*/, authParams)
>>>       .builder()
>>> 
>>> For AuthenticationTls, we need to do check the authParams, when the
>>> authParams is empty, we need to read TLS config from ClientBuilder,
>>> otherwise read from the authParams
>>> Compatibility
>>> 
>>> None.
>> 
>> 


Re: [DISCUSS] PIP-158: Split client TLS transport encryption from authentication

Posted by Zixuan Liu <no...@gmail.com>.
Hi Yunze,

Thanks for your suggestion, your idea is great, but we have the
`tlsProtocols()` and `tlsCiphers()` in `ClientBuilder`, so I use this style.

Thanks,
Zixuan

Yunze Xu <yz...@streamnative.io.invalid> 于2022年5月9日周一 13:31写道:

> It totally LGTM. I have a suggestion that it might be better to configure a
> class like `TlsConfiguration` instead of multiple TLS related configs
> added to
> `ClientBuilder`.
>
> Thanks,
> Yunze
>
>
>
>
> > 2022年4月24日 14:15,Zixuan Liu <no...@gmail.com> 写道:
> >
> > Hi Pulsar community,
> >
> > I open a https://github.com/apache/pulsar/issues/15289 for Split client
> TLS
> > transport encryption from authentication.
> >
> > Let me know what you think.
> >
> > Thanks,
> > Zixuan
> >
> > ------
> >
> > Motivation
> >
> > The client supports TLS transport encryption and TLS authentication, this
> > code so like:
> >
> > PulsarClient client = PulsarClient.builder()
> >                .serviceUrl("pulsar+ssl://localhost:6651")
> >                .tlsTrustCertsFilePath("/path/to/cacert.pem")
> >                .authentication(AuthenticationTls.class.getName(),
> authParams)
> >                .build()
> >
> > This causes an issue that cannot use other authentication with TLS
> > transport encryption, and also made our confusion if we use TLS transport
> > encryption by setting authentication.
> > Goal
> >
> > Split client TLS transport encryption from authentication is used to
> > support TLS transport encryption with any authentication.
> > API Changes
> >
> >   - Add new methods in org.apache.pulsar.client.api.ClientBuilder
> >
> > public interface ClientBuilder extends Serializable, Cloneable {
> >    /**     * Set the path to the TLS key file.     *     * @param
> > tlsKeyFilePath     * @return the client builder instance     */
> >    ClientBuilder tlsKeyFilePath(String tlsKeyFilePath);
> >
> >    /**     * Set the path to the TLS certificate file.     *     *
> > @param tlsCertificateFilePath     * @return the client builder
> > instance     */
> >    ClientBuilder tlsCertificateFilePath(String tlsCertificateFilePath);
> > }
> >
> > ImplementationTLS transport encryption
> >
> > We can call the tlsKeyFilePath(), tlsCertificateFilePath() and
> > tlsTrustCertsFilePath() to configurate the TLS transport encryption, the
> > code so like:
> >
> > PulsarClient client = PulsarClient.builder()
> >        .serviceUrl("pulsar+ssl://my-host:6650")
> >        .tlsTrustCertsFilePath("/path/to/cacert.pem")
> >        .tlsKeyFilePath("/path/to/client-key.pem")
> >        .tlsCertificateFilePath("/path/to/client-cert.pem")
> >        .build();
> >
> > TLS transport encryption with any authentication
> >
> > We can call the tlsKeyFilePath(), tlsCertificateFilePath(),
> > tlsTrustCertsFilePath() and authentication() to configurate the TLS
> > transport encryption with any authentication, the code so like:
> >
> > PulsarClient client = PulsarClient.builder()
> >        .serviceUrl("pulsar+ssl://my-host:6650")
> >        .tlsTrustCertsFilePath("/path/to/cacert.pem")
> >        .tlsKeyFilePath("/path/to/client-key.pem")
> >        .tlsCertificateFilePath("/path/to/client-cert.pem")
> >        .authentication(AuthenticationTls.class.getName() /*
> > AuthenticationToken.class.getName()*/, authParams)
> >        .builder()
> >
> > For AuthenticationTls, we need to do check the authParams, when the
> > authParams is empty, we need to read TLS config from ClientBuilder,
> > otherwise read from the authParams
> > Compatibility
> >
> > None.
>
>