Summary
Two bugs in MemqNettyServer.enableAuthenticationAuthorizationAuditing() that together make authorization permanently non-functional:
Bug 1 — Null-authorizer fail-open. When authorizerConfig is absent from the deployment config (which is the state of both shipped configs: deploy/configs/clustered.yaml and nonclustered.yaml), enableAuthenticationAuthorizationAuditing() returns null. PacketSwitchingHandler.authorize() wraps its entire body in if (authorizer != null) — so when authorizer is null, the block is skipped and all WRITE, READ, and TOPIC_METADATA requests proceed without any auth check.
Bug 2 — ClassCastException when auth IS configured. Even if an operator adds authorizerConfig: to their config, the instantiation is broken:
// Wrong: calls asSubclass on AuthorizerConfig.class, which doesn't implement Authorizer
Authorizer authorizer = authorizerConfig.getClass().asSubclass(Authorizer.class).newInstance();
// Should be:
Authorizer authorizer = Class.forName(authorizerConfig.getAuthorizerClass()).asSubclass(Authorizer.class).newInstance();
This always throws ClassCastException at startup, crashing the server. There is no path to a running server with authorization enabled in the current code.
Affected code
// MemqNettyServer.java
private Authorizer enableAuthenticationAuthorizationAuditing(MemqConfig configuration) throws Exception {
AuthorizerConfig authorizerConfig = configuration.getAuthorizerConfig();
if (authorizerConfig != null) {
Authorizer authorizer = authorizerConfig.getClass().asSubclass(Authorizer.class) // ClassCastException
.newInstance();
authorizer.init(authorizerConfig);
return authorizer;
}
return null; // default state with shipped configs
}
// PacketSwitchingHandler.java
private void authorize(RequestPacket requestPacket, Principal principal, String clientAddress) throws Exception {
if (authorizer != null) { // skips entire auth block when null
// ... authorization logic ...
}
// If null: falls through, request proceeds
}
Suggested fixes
- Fix the class instantiation:
Class.forName(authorizerConfig.getAuthorizerClass()).asSubclass(Authorizer.class).newInstance()
- Fail closed when auth isn't configured: throw
IllegalStateException when authorizerConfig is null so the server won't start without explicit auth config.
- Remove the
if (authorizer != null) guard in authorize() so a null authorizer always fails rather than silently passing (defense in depth).
- Add a reference
Authorizer implementation (even a simple allowlist-based one) so operators have a concrete starting point — currently Authorizer.java is an interface with no implementations in the codebase.
Happy to send a PR for these fixes.
Summary
Two bugs in
MemqNettyServer.enableAuthenticationAuthorizationAuditing()that together make authorization permanently non-functional:Bug 1 — Null-authorizer fail-open. When
authorizerConfigis absent from the deployment config (which is the state of both shipped configs:deploy/configs/clustered.yamlandnonclustered.yaml),enableAuthenticationAuthorizationAuditing()returnsnull.PacketSwitchingHandler.authorize()wraps its entire body inif (authorizer != null)— so when authorizer is null, the block is skipped and all WRITE, READ, and TOPIC_METADATA requests proceed without any auth check.Bug 2 — ClassCastException when auth IS configured. Even if an operator adds
authorizerConfig:to their config, the instantiation is broken:This always throws
ClassCastExceptionat startup, crashing the server. There is no path to a running server with authorization enabled in the current code.Affected code
Suggested fixes
Class.forName(authorizerConfig.getAuthorizerClass()).asSubclass(Authorizer.class).newInstance()IllegalStateExceptionwhenauthorizerConfigis null so the server won't start without explicit auth config.if (authorizer != null)guard inauthorize()so a null authorizer always fails rather than silently passing (defense in depth).Authorizerimplementation (even a simple allowlist-based one) so operators have a concrete starting point — currentlyAuthorizer.javais an interface with no implementations in the codebase.Happy to send a PR for these fixes.