Re: CASSANDRA-18554 - mTLS based client and internode authenticators

2023-06-18 Thread Yuki Morishita
HI,

I was discussing with users the other day regarding a similar feature.
They were thinking of implementing the custom Authenticator similar to what
MySQL offers:

CREATE USER 'jeffrey'@'localhost'
  REQUIRE SUBJECT '/C=SE/ST=Stockholm/L=Stockholm/
O=MySQL demo client certificate/
CN=client/emailAddress=cli...@example.com';

(https://dev.mysql.com/doc/refman/8.0/en/create-user.html#create-user-tls)

I think they can implement a custom Validator that validates the identity
(for their case, CN) associated with a role using the certificate's
subject, so that's great!

Regarding new CQL syntax,

> ADD IDENTITY 'testIdentity' TO ROLE 'testRole';
> DROP IDENTITY 'testIdentity';

This means a role can have multiple identities, and each identities must be
unique?
How can users check what identities are associated with certain roles?


On Sun, Jun 18, 2023 at 12:15 AM Dinesh Joshi  wrote:

> Folks, any feedback here?
>
> On 6/15/23 12:46, Jyothsna Konisa wrote:
> > Hi Everyone!
> >
> > We are adding the following CQL queries in this patch for adding and
> dropping identities in the new `system_auth.identity_to_role` table.
> >
> > ADD IDENTITY 'testIdentity' TO ROLE 'testRole';
> > DROP IDENTITY 'testIdentity';
> >
> > Please let us know if anyone has any concerns!
> >
> > Thanks,
> > Jyothsna Konisa.
> >
> >
> > On Sat, Jun 3, 2023 at 7:18 AM Derek Chen-Becker  > > wrote:
> >
> > Sounds great, thanks for the clarification!
> >
> > Cheers,
> >
> > Derek
> >
> > On Sat, Jun 3, 2023 at 12:48 AM Dinesh Joshi  > > wrote:
> >
> >> On Jun 2, 2023, at 9:06 PM, Derek Chen-Becker
> >> mailto:de...@chen-becker.org>> wrote:
> >>
> >> This certainly looks like a nice addition to the operator's
> >> tools for securing cluster access. Out of curiosity, is there
> >> anything in this work that would *preclude* a different
> >> authentication scheme for internode at some point in the
> >> future? Has there ever been discussion of pluggability similar
> >> to the client protocol?
> >
> > This is a pluggable implementation so it's not mandatory to use
> > it and doesn't preclude one from using a different mechanism in
> > the future. We haven't explicitly discussed pluggability i.e.
> > part of protocol negotiation in the past for internode
> > connections. However, this work also does not preclude us from
> > implementing such changes. If we do add negotiation this could
> > be one of the authentication mechanisms. So it would be
> > complimentary.
> >
> >
> >> Also, am I correct in understanding that this would allow for
> >> multiple certificates for the same identity (e.g. distinct
> >> cert per node)? I certainly understand the decision to keep
> >> things simple and have all nodes share identity from the
> >> perspective of operational simplicity, but I also don't want
> >> to get in a situation where a single compromised node would
> >> require an invalidation and redeployment on all nodes in the
> >> cluster.
> >
> > I don't recommend all nodes share the same certificate. Each
> > node in the cluster should obtain a unique certificate with the
> > same SPIFFE. In the event a node is compromised, the operator
> > can revoke that node's certificate without having to redeploy to
> > all nodes in the cluster.
> >
> > thanks,
> >
> > Dinesh
> >
> >
> >
> > --
> > +---+
> > | Derek Chen-Becker |
> > | GPG Key available at https://keybase.io/dchenbecker
> > and   |
> > | https://pgp.mit.edu/pks/lookup?search=derek%40chen-becker.org
> >  |
> > | Fngrprnt: EB8A 6480 F0A3 C8EB C1E7  7F42 AFC5 AFEE 96E4 6ACC  |
> > +---+
> >
>
>


Re: [DISCUSSIONS] Replace ant eclipse-warnings with CheckerFramework

2023-06-18 Thread Ekaterina Dimitrova
Thank you all! It seems there is a consensus here so I updated accordingly
CASSANDRA-18239

On Fri, 16 Jun 2023 at 8:56, Jeremiah Jordan 
wrote:

> +1 from me.
>
> On Jun 15, 2023 at 1:01:01 PM, Ekaterina Dimitrova 
> wrote:
>
>> Hi everyone,
>> Happy Thursday!
>> Some time ago, Jacek raised the point that ant eclipse-warnings is 
>> generating too many false positives and not really working as expected. 
>> (CASSANDRA-18239)
>>
>> Reminder: ant eclipse-warnings is a task we run with the goal to check 
>> Cassandra code - static analysis to warn on unsafe use of Autocloseable 
>> instances; checks against two related particular compiler options
>>
>> While trying to upgrade ECJ compiler that we use for this task 
>> (CASSANDRA-18190) so we can switch the task from running it with JDK8 to 
>> JDK11 in preparation for dropping JDK8, I hit the following issues:
>> - the latest version of ECJ is throwing more than 300 Potential Resource 
>> Leak warnings. I looked at 10-15, and they were all false positives.
>> - Even if we file a bug report to the Eclipse community, JDK11 is about to 
>> be removed with the next version of the compiler
>>
>> So I shared this information with Jacek. He came up with a different 
>> solution:
>> It seems we already pull through Guava CheckerFramework with an MIT license, 
>> which appears to be acceptable according to this link -  
>> https://www.apache.org/legal/resolved.html#category-a
>> He already has an initial integration with Cassandra which shows the 
>> following:
>> - CheckerFramework does not understand the @SuppressWarnings("resource") 
>> (there is a different one to be used), so it is immediately visible how it 
>> does not report all those false positives that eclipse-warnings does. On the 
>> flip side, I got the feedback that what it has witnessed so far is something 
>> we should investigate.
>> - Also, there are additional annotations like @Owning that let you fix many 
>> problems at once because the tool understands that the ownership of the 
>> resources was passed to another entity; It also enables you to do something 
>> impossible with eclipse-warnings - you can tell the tool that there is 
>> another method that needs to be called to release the resources, like 
>> release, free, disconnect, etc.
>> - the tool works with JDK8, JDK11, JDK17, and JDK20, so we can backport it 
>> even to older branches (while at the same time keeping eclipse-warnings 
>> there)
>> - though it runs 8 minutes so, we should not run it with every test, some 
>> reorganization around ant tasks will be covered as even for eclipse-warnings 
>> it was weird to call it on every single test run locally by default
>>
>>
>> If there are no concerns, we will continue replacing ant eclipse-warnings 
>> with the CheckerFramework as part of CASSANDRA-18239 and CASSANDRA-18190 in 
>> trunk.
>>
>> Best regards,
>>
>> Ekaterina
>>
>>