[
https://issues.apache.org/jira/browse/CASSANDRA-21546?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18101835#comment-18101835
]
Francisco Guerrero commented on CASSANDRA-21546:
------------------------------------------------
Thanks Aparna for the patch and Stefan for the suggestions. Regarding the
nesting of the configuration I think the concern underneath your suggestion is
a good one, but I'd like to push back on the specific mechanism (nesting under
{{role_manager.parameters}}) and keep {{default_role_initializer}} as a
top-level {{ParameterizedClass}}. Here's my take:
The concern is real, but it's an API problem, not a configuration placement
one. You're right that {{IDefaultRoleInitializer}} shouldn't be invisibly glued
into {{CassandraRoleManager}}, and that a custom {{IRoleManager}} should be
aware of it rather than silently ignore the capability. But where the keys sit
in {{cassandra.yaml}} has no relationship on that; a custom implementor
discovers the feature by reading the {{IRoleManager}} interface, not the YAML.
I've addressed awareness where it actually lives:
{{IRoleManager.defaultRoleInitializer()}} is the integration point,
{{AuthConfig.applyAuth()}} instantiates the initializer from the top-level
config and wires it in (before the role manager, since the role manager depends
on it), and a custom {{IRoleManager}} can override that method to integrate
however it needs. Nesting the keys wouldn't move that needle.
Here are some of the drawbacks for the nesting approach:
- {{ParameterizedClass.parameters}} is {{Map<String,String>}}, so you can't
embed a {{ParameterizedClass}} inside it. Nesting forces flat, stringly-typed
keys ({{default_role_initializer_class_name}},
{{default_role_initializer_role}}, …) plus hand-rolled prefix-stripping —
exactly the shape {{ParameterizedClass}} exists to avoid.
- It stays consistent with every sibling — {{authenticator}}, {{authorizer}},
{{role_manager}}, {{network_authorizer}}, {{cidr_authorizer}},
{{internode_authenticator}} are all top-level {{ParameterizedClass}} fields
that cross-validate at startup, and the initializer validates the same way.
- The initializer isn't purely a role-manager concern:
{{MutualTlsDefaultRoleInitializer.validateConfiguration()}} reaches across to
the authenticator (it rejects an authenticator that can't do MTLS). Nesting it
under {{role_manager}} misrepresents that coupling.
- The {{authenticator.parameters.validator_class_name}} analogy you argue for
is a leaf. A bare class name with no params of its own. The initializer carries
its own parameters ({{role}}, {{password}}/{{identity}}/{{hash}}) and its own
validation, so it's a full {{ParameterizedClass}}, not a leaf string.
- Top-level also keeps it compatible with {{ParameterizedClass}}
config-generation tooling, which understands the {{{class_name, parameters}}}
shape; flat prefixed strings reuse none of it.
I think the concern about the shipped initializers is valid. The initializers
do {{INSERT INTO system_auth.roles}}, so they're storage-coupled to
{{CassandraRoleManager}} and a genuinely custom {{IRoleManager}} couldn't reuse
them as-is. I think the right answer to that is the typed contract on
{{IRoleManager}} plus startup cross-validation (same as the rest of the auth
stack), not YAML nesting.
I have pushed a couple of commits on top of Aparna's original patch, and
Stefan's suggestions:
https://github.com/apache/cassandra/compare/trunk...frankgh:cassandra:CASSANDRA-21546?expand=1
In the first commit, I try to fix the tests that broke after the rewrite and
cleanup some dead code. However, this is where I decide to go back to the top
level configuration approach. That's what the second commit does. I also add
tests backfilling the change in the {{ParameterizedClass.toString}} method, and
update the {{MutualTlsCertificateValidityPeriodTest}} test to use this feature
so avoid having to add the role and identity needed for the test.
Happy to keep discussing if you still feel the coupling warrants nesting — but
I'd argue the interface contract carries that weight better than the config
layout does.
> Support pluggable default role initialization (avoid hardcoded superuser
> password)
> ----------------------------------------------------------------------------------
>
> Key: CASSANDRA-21546
> URL: https://issues.apache.org/jira/browse/CASSANDRA-21546
> Project: Apache Cassandra
> Issue Type: Bug
> Reporter: Aparna Naik
> Assignee: Aparna Naik
> Priority: Normal
>
> Cassandra's first-boot bootstrap hardcodes the creation of a cassandra
> superuser role with a default password (cassandra). Every new cluster starts
> with this guessable credential exposed until an operator manually rotates or
> drops it, and deployments that already use mutual TLS have no way to
> bootstrap a superuser identity without also creating this password-based one.
> This ticket will make the default role bootstrap pluggable via a new
> IDefaultRoleInitializer interface and default_role_initializer config option.
> The existing password-based behavior will be the default implementation for
> backward compatibility, and it will add a MutualTlsDefaultRoleInitializer
> that instead maps a client certificate identity to the superuser role, so no
> password credential needs to exist at all.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
