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

[Jyothsna Konisa]

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 
>> 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  |
> +---+
>
>

[DISCUSSIONS] Replace ant eclipse-warnings with CheckerFramework

[Ekaterina Dimitrova]

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

Re: [DISCUSS] Remove org.apache.cassandra.io.sstable.SSTableHeaderFix in trunk (5.0)?

[David Capwell]

Not heard any feedback yet, so tomorrow plan to remove… the feature was local 
to 3.6+ so all users migrating from 3.0 to 4.0 never had this issue

> On Jun 13, 2023, at 10:22 AM, David Capwell  wrote:
> 
> org.apache.cassandra.io.sstable.SSTableHeaderFix was added due to bugs in 3.6 
> causing invalidate types or incompatible types (due to toString changes) in 
> the SSTableHeader… this logic runs on start and rewrites all Stats files that 
> had a mismatch from the local schema; with 5.0 requiring upgrades from 4.x 
> only, this logic should have already run as its a 3.x to 4.0 migration step 
> (though users are able to opt out [1]) which should have already fixed the 
> SSTables to have correct schema…
> 
> Why is this a problem now?  CASSANDRA-18504 is adding a lot of property/fuzz 
> tests to the type system and the read/write path, which has found several 
> bugs; fixing some of the bugs actually impacts SSTableHeader because it 
> requires generating and working with types that are not valid, so it can fix 
> them…   By removing this logic, we can push this type validation into the 
> type system to avoid generating incorrect types.  
> 
> If we wish to keep this class, we need to maintain allowing invalid types to 
> be created, which may cause bugs down the road.
> 
> 
> [1] if a user opts out there are 2 real cases that are impacted: UDTs, and 
> collections of collections…
> * For UDTs, the frozen vs non-frozen type are not the same, so mixing these 
> causes us to fail to read the data, failing the read…. I believe 
> writes/compactions will not corrupt the data, but anything that touches these 
> SSTables will fail due to the schema mismatch… the only way to resolve this 
> is to fix the SSTables… If you disabled in 4.x, you were living with broken / 
> unreadable SSTables, so by removing 5.0 would loose the ability to repair 
> them (but 4.x would still be able to)
> * for collections of collections, this is less of an issue.  The logic would 
> detect that the collection has a non-frozen collection as the element, so 
> would migrate them to frozen.  This behavior has been moved to the type 
> system, so a read from SSTable of “list>” automagically becomes a 
> "ListType(FrozenType(ListType(Int32Type)))”.  The SSTables are not “fixed”, 
> but compaction is able to read the data correctly, and the new SSTables will 
> have the correct header.  

[DISCUSS] CEP 33 - CIDR filtering authorizer

[Shailaja Koppu]

Adding [DISCUSS] to the subject. I request everyone here to please share your 
thoughts/comments on this CEP.

Thank you @Nate.


> On Jun 15, 2023, at 12:48 AM, Nate McCall  wrote:
> 
> Hi Shailaja,
> This looks super interesting. I particularly like the MONITOR switch. This is 
> a huge pain-point for a lot of cluster migrations. 
> 
> Cheers,
> -Nate
> 
> On Thu, Jun 15, 2023 at 6:43 AM Shailaja Koppu  > wrote:
> Hi Team,
> 
> I have created CEP 33 - CIDR filtering authorizer 
> https://cwiki.apache.org/confluence/display/CASSANDRA/CEP-33%3A+CIDR+filtering+authorizer
>  
> .
> 
> Purpose of this feature is to add the ability to restrict users accesses 
> based on the client’s IP (or region). We can map set of CIDRs to CIDR groups 
> (aka, regions), and then enable or disable roles to access from certain CIDR 
> groups. CEP page details why are we doing this and how. Please go through it, 
> comment here on the discussion thread and vote. 
> 
> For your reference, code for this feature is at 
> https://github.com/apache/cassandra/pull/2414 
> . PR description contains an 
> example usage.
> 
> 
> Thanks,
> Shailaja

[DISCUSS] Slab allocation and memory measurements

[Benjamin Lerer]

Hi,

While working on integrating the new version of Jamm to Cassandra, I
realized that our way to create slabs and how we measure their memory
consumption may not be optimal.

For the sake of simplicity I will only talk about read-write heap buffers
here. Considering that the same principles can be applied to other types of
buffers.

Looking at
*org.apache.cassandra.utils.memory.SlabAllocator.Region::allocate* what we
do is:
data.duplicate().position((newOffset)).limit(newOffset + size)

This results in a ByteBuffer with the same capacity as the original one
with a position and a limit that can be moved outside the new limit that we
defined.
>From a measurement perspective if we want to avoid taking into account the
shared array structure for each buffer, we need to be able to determine if
your buffer can be considered as a slab or not. The current approach is to
consider as a slab anything where the underlying array size is greater than
the number of remaining bytes. This approach seems fragile to me.

If we were using the slice method to create our slab:
data.duplicate().position((newOffset)).limit(newOffset + size).slice()
The slab ByteBuffer would have a capacity that represent the real size of
the slab and will prevent us to change the position or limit to an
incorrect value. It will allow us to reliably identify a slab buffer as its
capacity will always be smaller than the underlying array.

For DirectBuffer using slice after a duplicate is not a good idea before
Java 12 due to a Java bug (https://bugs.openjdk.org/browse/JDK-8208362)
which would result in using 64 extra bytes by direct slab buffer.
Nevertheless, I was wondering if there were other reasons for not using
slice when allocating slabs and if we should not consider using them for
heap buffer for the moment and for all buffers once we do not support only
Java 17 and 21.

Re: [VOTE] CEP-8 Datastax Drivers Donation

[Chris Lohfink]

+1

On Wed, Jun 14, 2023 at 9:05 PM Jon Haddad 
wrote:

> +1
>
> On 2023/06/13 14:14:35 Jeremy Hanna wrote:
> > Calling for a vote on CEP-8 [1].
> >
> > To clarify the intent, as Benjamin said in the discussion thread [2],
> the goal of this vote is simply to ensure that the community is in favor of
> the donation. Nothing more.
> > The plan is to introduce the drivers, one by one. Each driver donation
> will need to be accepted first by the PMC members, as it is the case for
> any donation. Therefore the PMC should have full control on the pace at
> which new drivers are accepted.
> >
> > If this vote passes, we can start this process for the Java driver under
> the direction of the PMC.
> >
> > Jeremy
> >
> > 1.
> https://cwiki.apache.org/confluence/display/CASSANDRA/CEP-8%3A+Datastax+Drivers+Donation
> > 2. https://lists.apache.org/thread/opt630do09phh7hlt28odztxdv6g58dp
>