Skip to content

Commit

Permalink
[DPE-5208] - fix: enforce client auth (#100)
Browse files Browse the repository at this point in the history
* fix: enforce sasl client auth

* chore: fix failing tls unit-test

* chore: restore ssl.*.clientAuth=none
  • Loading branch information
marcoppenheimer authored Sep 4, 2024
1 parent e494ded commit 50813f1
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 12 deletions.
10 changes: 5 additions & 5 deletions lib/charms/zookeeper/v0/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ def update_cluster(new_members: List[str], event: EventBase) -> None:

# Increment this PATCH version before using `charmcraft publish-lib` or reset
# to 0 if you are raising the major API version
LIBPATCH = 6
LIBPATCH = 7


logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -369,7 +369,7 @@ def leader_znodes(self, path: str) -> Set[str]:

return all_znode_children

def create_znode_leader(self, path: str, acls: List[ACL]) -> None:
def create_znode_leader(self, path: str, acls: List[ACL] | None = None) -> None:
"""Creates a new zNode on the current quorum leader with given ACLs.
Args:
Expand All @@ -388,7 +388,7 @@ def create_znode_leader(self, path: str, acls: List[ACL]) -> None:
) as zk:
zk.create_znode(path=path, acls=acls)

def set_acls_znode_leader(self, path: str, acls: List[ACL]) -> None:
def set_acls_znode_leader(self, path: str, acls: List[ACL] | None = None) -> None:
"""Updates ACLs for an existing zNode on the current quorum leader.
Args:
Expand Down Expand Up @@ -577,7 +577,7 @@ def delete_znode(self, path: str) -> None:
return
self.client.delete(path, recursive=True)

def create_znode(self, path: str, acls: List[ACL]) -> None:
def create_znode(self, path: str, acls: List[ACL] | None = None) -> None:
"""Create new znode.
Args:
Expand All @@ -599,7 +599,7 @@ def get_acls(self, path: str) -> List[ACL]:

return acl_list if acl_list else []

def set_acls(self, path: str, acls: List[ACL]) -> None:
def set_acls(self, path: str, acls: List[ACL] | None = None) -> None:
"""Sets acls for a desired znode path.
Args:
Expand Down
3 changes: 3 additions & 0 deletions src/managers/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@
quorum.auth.learnerRequireSasl=true
quorum.auth.serverRequireSasl=true
authProvider.sasl=org.apache.zookeeper.server.auth.SASLAuthenticationProvider
enforce.auth.enabled=true
enforce.auth.schemes=sasl
sessionRequireClientSASLAuth=true
audit.enable=true
"""

Expand Down
5 changes: 1 addition & 4 deletions src/managers/quorum.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,6 @@ def update_acls(self, event: RelationEvent | None = None) -> None:
leader_chroots = self.client.leader_znodes(path="/")
logger.debug(f"{leader_chroots=}")

requested_acls = set()
requested_chroots = set()

for client in self.state.clients:
Expand All @@ -209,8 +208,6 @@ def update_acls(self, event: RelationEvent | None = None) -> None:
)
logger.info(f"{generated_acl=}")

requested_acls.add(generated_acl)

# FIXME: data-platform-libs should handle this when it's implemented
if client.database:
if event and client.relation and client.relation.id == event.relation.id:
Expand All @@ -224,7 +221,7 @@ def update_acls(self, event: RelationEvent | None = None) -> None:
self.client.create_znode_leader(path=client.database, acls=[generated_acl])

# Looks for existing related applications
logger.info(f"UPDATE CHROOT - {client.database}")
logger.debug(f"UPDATE CHROOT - {client.database}")
self.client.set_acls_znode_leader(path=client.database, acls=[generated_acl])

# Looks for applications no longer in the relation but still in config
Expand Down
13 changes: 10 additions & 3 deletions tests/unit/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,14 +132,21 @@ def test_multiple_jaas_users_are_added(harness):
def test_tls_enabled(harness):
with harness.hooks_disabled():
harness.update_relation_data(
harness.charm.state.peer_relation.id, CHARM_KEY, {"tls": "enabled"}
harness.charm.state.peer_relation.id,
CHARM_KEY,
{"tls": "enabled", "quorum": "ssl"},
)
harness.update_relation_data(
harness.charm.state.peer_relation.id,
f"{CHARM_KEY}/0",
{"certificate": "foo"},
)

assert "ssl.clientAuth=none" in harness.charm.config_manager.zookeeper_properties
assert "sslQuorum=true" in harness.charm.config_manager.zookeeper_properties


def test_tls_disabled(harness):
assert "ssl.clientAuth=none" not in harness.charm.config_manager.zookeeper_properties
assert "sslQuorum=true" not in harness.charm.config_manager.zookeeper_properties


def test_tls_switching_encryption(harness):
Expand Down

0 comments on commit 50813f1

Please sign in to comment.