Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix nullpointer that occurs when OAuthKafkaPrincipalBuilder is used with Kerberos listener #207

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ jobs:
- hydra
- hydra-jwt
- mockoauth
- kerberos
apt:
packages:
- maven
Expand All @@ -26,6 +27,7 @@ jobs:
- hydra
- hydra-jwt
- mockoauth
- kerberos
apt:
packages:
- maven
Expand All @@ -43,6 +45,7 @@ addons:
- hydra
- hydra-jwt
- mockoauth
- kerberos
env:
global:
- PULL_REQUEST=${TRAVIS_PULL_REQUEST}
Expand Down
2 changes: 1 addition & 1 deletion .travis/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
set -e

clearDockerEnv() {
docker rm -f kafka zookeeper keycloak keycloak-import hydra hydra-import hydra-jwt hydra-jwt-import || true
docker rm -f kafka zookeeper keycloak keycloak-import hydra hydra-import hydra-jwt hydra-jwt-import kerberos || true
DOCKER_TEST_NETWORKS=$(docker network ls | grep test | awk '{print $1}')
[ "$DOCKER_TEST_NETWORKS" != "" ] && docker network rm $DOCKER_TEST_NETWORKS
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@
import org.apache.kafka.common.security.auth.KafkaPrincipal;
import org.apache.kafka.common.security.auth.SaslAuthenticationContext;
import org.apache.kafka.common.security.authenticator.DefaultKafkaPrincipalBuilder;
import org.apache.kafka.common.security.kerberos.KerberosShortNamer;
import org.apache.kafka.common.security.oauthbearer.OAuthBearerLoginModule;
import org.apache.kafka.common.security.oauthbearer.internals.OAuthBearerSaslServer;
import org.apache.kafka.common.security.plain.internals.PlainSaslServer;

import javax.security.auth.kerberos.KerberosPrincipal;
import javax.security.sasl.SaslServer;
import java.io.IOException;
import java.lang.reflect.Field;
Expand Down Expand Up @@ -51,7 +53,8 @@
*/
public class OAuthKafkaPrincipalBuilder extends DefaultKafkaPrincipalBuilder implements Configurable {

private static final SetAccessibleAction SET_PRINCIPAL_MAPPER = SetAccessibleAction.newInstance();
private static final SetAccessibleAction SET_PRINCIPAL_MAPPER = SetAccessibleAction.newInstance("sslPrincipalMapper");
private static final SetAccessibleAction SET_KERBEROS_SHORT_NAMER = SetAccessibleAction.newInstance("kerberosShortNamer");

private static final int OAUTH_DATA_TAG = 575;

Expand All @@ -74,9 +77,9 @@ void invoke(DefaultKafkaPrincipalBuilder target, Object value) throws IllegalAcc
field.set(target, value);
}

static SetAccessibleAction newInstance() {
static SetAccessibleAction newInstance(String fieldName) {
try {
return new SetAccessibleAction(DefaultKafkaPrincipalBuilder.class.getDeclaredField("sslPrincipalMapper"));
return new SetAccessibleAction(DefaultKafkaPrincipalBuilder.class.getDeclaredField(fieldName));
} catch (NoSuchFieldException e) {
throw new IllegalStateException("Failed to install OAuthKafkaPrincipalBuilder. This Kafka version does not seem to be supported", e);
}
Expand All @@ -91,12 +94,21 @@ public OAuthKafkaPrincipalBuilder() {
super(null, null);
}



@Override
public void configure(Map<String, ?> configs) {

Object sslPrincipalMappingRules = configs.get(BrokerSecurityConfigs.SSL_PRINCIPAL_MAPPING_RULES_CONFIG);
Object sslPrincipalMapper;


@SuppressWarnings("unchecked")
List<String> principalToLocalRules = (List<String>) configs.get(BrokerSecurityConfigs.SASL_KERBEROS_PRINCIPAL_TO_LOCAL_RULES_CONFIG);
String defaultRealm;
Object kerberosShortNamer;


try {
Class<?> clazz = Class.forName("org.apache.kafka.common.security.ssl.SslPrincipalMapper");
try {
Expand All @@ -120,6 +132,18 @@ public void configure(Map<String, ?> configs) {

SET_PRINCIPAL_MAPPER.invoke(this, sslPrincipalMapper);

try {
defaultRealm = new KerberosPrincipal("tmp", 1).getRealm();
} catch (Exception ex) {
defaultRealm = "";
}

if (principalToLocalRules != null) {
kerberosShortNamer = KerberosShortNamer.fromUnparsedRules(defaultRealm, principalToLocalRules);
SET_KERBEROS_SHORT_NAMER.invoke(this, kerberosShortNamer);
}


} catch (RuntimeException
| ClassNotFoundException
| NoSuchMethodException
Expand Down
1 change: 1 addition & 0 deletions testsuite/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ Then, you have to add some entries to your `/etc/hosts` file:
127.0.0.1 hydra-jwt
127.0.0.1 kafka
127.0.0.1 mockoauth
127.0.0.1 kerberos
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this have the same alignment as the lines above? I guess technically, it does not matter, but would be more readable.


That's needed for host resolution, because Kafka brokers and Kafka clients connecting to Keycloak / Hydra have to use the
same hostname to ensure compatibility of generated access tokens.
Expand Down
10 changes: 10 additions & 0 deletions testsuite/docker/kerberos/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
FROM ubuntu:22.04
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mstruk Are you fine with this being based on Ubuntu? Strimzi IMHO does not use Ubuntu images anywhere else. So I would feel better if this used Red Hat UBI images as all our other projects. But I can live with this if you are fine with it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that you pointed this out, if I remember correctly, we had some Travis build issues with Ubuntu images on some architectures.

@akaczano Could you try use:
FROM registry.access.redhat.com/ubi8/ubi

or

FROM registry.access.redhat.com/ubi8/openjdk-17

You may need to install some additional packages.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure; I'll give that a try.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took a look at this and it looks very challenging to set it up on ubi8. Tried a few leads with no luck. The needed package krb5-server doesn't seem to be available in the repo or is behind some kind of subscription wall. The setup is apparently very complicated as it is, and putting it together with any alternative would be just as complicated if not more. I suggest we keep it as is, and exclude the test run on architectures that will cause problems due to unavailability of platform specific container images.


RUN DEBIAN_FRONTEND=noninteractive apt-get update -y && apt-get install -y krb5-kdc krb5-admin-server

EXPOSE 88 749

ADD ./config.sh /config.sh

ENTRYPOINT ["/config.sh"]

97 changes: 97 additions & 0 deletions testsuite/docker/kerberos/config.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
#!/bin/bash

[[ "TRACE" ]] && set -x

: ${REALM:=KERBEROS}
: ${DOMAIN_REALM:=kerberos}
: ${KERB_MASTER_KEY:=masterkey}
: ${KERB_ADMIN_USER:=admin}
: ${KERB_ADMIN_PASS:=admin}
: ${KAFKA_USER:=kafka}
: ${KAFKA_HOST:=kafka}
: ${KAFKA_CLIENT_USER:=client}

fix_nameserver() {
cat>/etc/resolv.conf<<EOF
nameserver $NAMESERVER_IP
search $SEARCH_DOMAINS
EOF
}

fix_hostname() {
sed -i "/^hosts:/ s/ *files dns/ dns files/" /etc/nsswitch.conf
}

create_config() {
: ${KDC_ADDRESS:=$(hostname -f)}

cat>/etc/krb5.conf<<EOF
[logging]
default = FILE:/var/log/kerberos/krb5libs.log
kdc = FILE:/var/log/kerberos/krb5kdc.log
admin_server = FILE:/var/log/kerberos/kadmind.log

[libdefaults]
default_realm = $REALM
dns_lookup_realm = false
dns_lookup_kdc = false
ticket_lifetime = 24h
renew_lifetime = 7d
forwardable = true

[realms]
$REALM = {
kdc = $KDC_ADDRESS
admin_server = $KDC_ADDRESS
}

[domain_realm]
.$DOMAIN_REALM = $REALM
$DOMAIN_REALM = $REALM
EOF
}

create_db() {
kdb5_util -P $KERB_MASTER_KEY -r $REALM create -s
}

start_kdc() {
service krb5-kdc start
service krb5-admin-server start
}

restart_kdc() {
service krb5-kdc restart
service krb5-admin-server restart
}

create_admin_user() {
kadmin.local -q "addprinc -pw $KERB_ADMIN_PASS $KERB_ADMIN_USER/admin"
echo "*/admin@$REALM *" > /etc/krb5kdc/kadm5.acl
}

create_kafka_user() {
kadmin.local -q "addprinc -randkey $KAFKA_HOST/$KAFKA_USER@$REALM"
kadmin.local -q "ktadd -k /keytabs/kafka_broker.keytab $KAFKA_HOST/$KAFKA_USER@$REALM"
kadmin.local -q "addprinc -randkey $KAFKA_HOST/$KAFKA_CLIENT_USER@$REALM"
kadmin.local -q "ktadd -k /keytabs/kafka_client.keytab $KAFKA_HOST/$KAFKA_CLIENT_USER@$REALM"
chmod 666 /keytabs/kafka_broker.keytab
chmod 666 /keytabs/kafka_client.keytab
}



if [ ! -f /kerberos_initialized ]; then
mkdir -p /var/log/kerberos
create_config
create_db
create_admin_user
create_kafka_user
start_kdc

touch /kerberos_initialized
else
start_kdc
fi

tail -F /var/log/kerberos/krb5kdc.log
7 changes: 7 additions & 0 deletions testsuite/docker/kerberos/kafka_server_jaas.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
KafkaServer {
com.sun.security.auth.module.Krb5LoginModule required
useKeyTab=true
storeKey=true
keyTab="/opt/kafka/keytabs/kafka_broker.keytab"
principal="kafka/kafka@KERBEROS";
};
21 changes: 21 additions & 0 deletions testsuite/docker/kerberos/krb5.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
[logging]
default = FILE:/var/log/kerberos/krb5libs.log
kdc = FILE:/var/log/kerberos/krb5kdc.log
admin_server = FILE:/var/log/kerberos/kadmind.log
[libdefaults]
default_realm = KERBEROS
dns_lookup_realm = false
dns_lookup_kdc = false
ticket_lifetime = 24h
renew_lifetime = 7d
forwardable = true
rdns = false
ignore_acceptor_hostname = true
[realms]
KERBEROS = {
kdc = kerberos
admin_server = kerberos
}
[domain_realm]
.kerberos = KERBEROS
kerberos = KERBEROS
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ public void doTest() throws Exception {

} catch (Throwable e) {
log.error("Keycloak ZK Authorization Test failed: ", e);
e.printStackTrace();
throw e;
}
}
Expand Down
Loading
Loading