Great discussion.

I support making it pluggable for now, while I previously held
the belief that storing within metadata was feasible.

Regardless of the storage location, users always require a means
to upload the cert files. This can be accomplished either by directly
using the pulsar-admin CLI for uploading or by providing an accessible
path to the cert files for accessing different clusters.

In terms of providing a better user experience, I believe supporting
cert file uploads through the pulsar-admin CLI can be deemed worthy
of consideration; however, it should not be the sole option available to
users.

For example:

public class CertsStorage {

    CompletableFuture<Void> upload(String cluster, String cert);
    CompletableFuture<String> get(String cluster);

}

If users don't want to support uploading cert files by pulsar-admin CLI.
The upload method can be disregarded in the implementation.

Regards,
Penghui

On Sat, Sep 9, 2023 at 7:14 AM Michael Marshall <mmarsh...@apache.org>
wrote:

> Thanks for your PIP. I respect that this is a challenge for users. I have
> spent hours debugging basic cert mistakes, so I agree it is worth
> discussing. That being said, I have some concerns about the design.
>
> The PIP dropped the section that is supposed to describe the security
> implications of the feature. I’d like to see that section covered in detail
> for this PIP.
>
> This feature affects what remote peers a client broker will trust and then
> subsequently send user data and admin level auth data. As such, I view the
> trust store as a secret and think it should be handled accordingly.
>
> My understanding is that none of our metadata stores are intended for
> secrets, and if that is true, then I do not support storing secrets or
> recommending that any of our users store secrets in the metadata store.
>
> Thanks,
> Michael
>
> On Fri, Sep 8, 2023 at 12:58 PM Rajan Dhabalia <dhabalia...@gmail.com>
> wrote:
>
> > As I said, Pulsar is not made for cert management system and introducing
> > such extensions will bring a lot of other complexities such as cert
> > rotation, access rights, expiry of certs, securing certs, supporting
> > different types of keystore and cert format etc,..  and that will be out
> of
> > the scope of Pulsar as a messaging system. If we need such things then
> make
> > it pluggable same as how we do with encryption key and users can have
> their
> > custom implementation where users can store certs in any format and at
> any
> > location (metadata, file-system) but that implementation must be outside
> of
> > Pulsar repo to avoid scope of this complexity.
> >
> > Thanks,
> > Rajan
> >
> > On Fri, Sep 8, 2023 at 2:11 AM mattison chao <mattisonc...@gmail.com>
> > wrote:
> >
> > > Hi, Rajan
> > >
> > > I understand your concerns about the cert management. I just want to
> let
> > > cluster data support store base64 encoded brokerClientTrustCerts to
> avoid
> > > multiple cluster replication problems in the private cert assignment.
> We
> > > already have the ClusterDataImpl that stores the metadata. We can
> easily
> > > expand this entity to support base64 encoded private cert.
> > >
> > > Additionally, apart from the option of retaining the certificate of the
> > > destination cluster, are there any other recommendations you might have
> > in
> > > mind?
> > >
> > > Best,
> > > Mattison
> > > On 8 Sep 2023 at 16:47 +0800, Rajan Dhabalia <rdhaba...@apache.org>,
> > > wrote:
> > > > Hi,
> > > >
> > > > Pulsar stores different types of metadata into a metadata store which
> > > > contains tenant, namespaces and topic metadata. Metadata-store should
> > > store
> > > > metadata definition and should avoid combining other non metadata
> > related
> > > > information especially certificates or keys like
> encryption/decryption
> > > > keys. Apache Pulsar supports envelope encryption for which it
> requires
> > to
> > > > store and retrieve encryption keys but if Pulsar starts managing such
> > > keys
> > > > then Pulsar will start playing responsibilities of any other KMS
> > systems
> > > > and that MUST BE out of the scope of the Pulsar because KMS itself
> is a
> > > big
> > > > beast and it comes with lot of security requirements and
> > > responsibilities.
> > > > And that's why we made it a pluggable component and one can implement
> > its
> > > > own implementation. Similar way, managing certificates MUST be out of
> > > > Pulsar scope else it will become can of worms which comes with lot
> > > security
> > > > bugs and concerns , and definitely it's not something we want to
> store
> > in
> > > > Metadata store, So, we MUST not introduce a support of cert
> management
> > in
> > > > Pulsar and MUST NOT store certs into metadata-store, We can certainly
> > see
> > > > if we can provide interface and API to make it pluggable and let
> users
> > > > manage their own implementation without introducing such critical
> > > > complexities in Pulsar.
> > > >
> > > > Thanks,
> > > > Rajan
> > > >
> > > > On Fri, Sep 8, 2023 at 1:21 AM mattison chao <mattisonc...@gmail.com
> >
> > > wrote:
> > > >
> > > > >
> > > > > Hello, folks.
> > > > >
> > > > > I hope this email finds you well. I would like to start a
> discussion
> > > about
> > > > > PIP-296 Support storing broker internal client certificates in
> > metadata
> > > > > store[1].
> > > > >
> > > > > Please don't hesitate to leave any concerns or questions.
> > > > >
> > > > > Best,
> > > > > Mattison
> > > > >
> > > > > [1] https://github.com/apache/pulsar/pull/21044/files
> > > > >
> > > > >
> > >
> >
>

Reply via email to