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

[bitnami/schema-registry] keystore is not mandatory for SASL_SSL protocol #71059

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

masfworld
Copy link

Description of the change

There are multiple protocols for security to connect Schema Registry to Kafka. SASL_SSL is one of them. Before this change, a keystore is mandatory to use SASL_SSL protocol. This change fixes that requirement. So, keystore won't be a requirement for SASL_SSL anymore

Possible drawbacks

Applicable issues

Additional information

This code has been tested using AWS MSK with SASL_SSL protocol, with the following configuration

        - name: SCHEMA_REGISTRY_KAFKA_BROKERS
          value: SASL_SSL://b-1:9096,SASL_SSL://b-2:9096,SASL_SSL://b-3:9096
        - name: SCHEMA_REGISTRY_KAFKA_SASL_MECHANISM
          value: SCRAM-SHA-512
        - name: SCHEMA_REGISTRY_KAFKA_SASL_USERS
          value: confluent-registry
        - name: SCHEMA_REGISTRY_KAFKA_SASL_PASSWORDS
          value: password
        - name: SCHEMA_REGISTRY_LISTENERS
          value: http://0.0.0.0:8081
        - name: SCHEMA_REGISTRY_AVRO_COMPATIBILY_LEVEL
          value: NONE
        - name: SCHEMA_REGISTRY_HEAP_OPTS
          value: -XX:InitialRAMPercentage=80.0 -XX:MaxRAMPercentage=80.0
        - name: SCHEMA_REGISTRY_JVM_PERFORMANCE_OPTS
          value: -XX:MetaspaceSize=96m -XX:+UseG1GC -XX:MaxGCPauseMillis=20 -XX:InitiatingHeapOccupancyPercent=35
            -XX:G1HeapRegionSize=16M -XX:MinMetaspaceFreeRatio=50 -XX:MaxMetaspaceFreeRatio=80
        - name: SCHEMA_REGISTRY_JMX_OPTS
          value: -javaagent:/opt/jmx_prometheus_javaagent.jar=5556:/etc/jmx-schema-registry/jmx-schema-registry-prometheus.yml
            -Dcom.sun.management.jmxremote -Dcom.sun.management.jmxremote.authenticate=false
            -Dcom.sun.management.jmxremote.ssl=false
        image: XXXXX

Signed-off-by: Miguel Sotomayor <[email protected]>
Signed-off-by: masfworld <[email protected]>
…ithub.com-masfworld:masfworld/bitnami-containers into sasl_ssl_no_keystore
@carrodher carrodher added verify Execute verification workflow for these changes in-progress labels Aug 21, 2024
@github-actions github-actions bot removed the triage Triage is needed label Aug 21, 2024
@github-actions github-actions bot removed the request for review from javsalgar August 21, 2024 07:00
@github-actions github-actions bot requested a review from migruiz4 August 21, 2024 07:00
Copy link
Member

@migruiz4 migruiz4 left a comment

Choose a reason for hiding this comment

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

Hi @masfworld,

Thank you for your contribution!

I would like to ask several questions:

  • How does schema-registry not need certificates (keystore) when SASL_SSL protocol is used?
  • What is the reason to add the [[ -v "$SCHEMA_REGISTRY_CERTS_DIR" ]] conditional? The environment variable 'SCHEMA_REGISTRY_CERTS_DIR' is configured by default, so what would be the scenario where it could be empty?

@masfworld
Copy link
Author

Hi @migruiz4 👋

* How does schema-registry not need certificates (keystore) when SASL_SSL protocol is used?

Username and password should be enough for Kafka authentication. Confluent Schema Registry official docker image doesn't require a keystore for SASL_SSL.

* What is the reason to add the `[[ -v "$SCHEMA_REGISTRY_CERTS_DIR" ]]` conditional? The environment variable 'SCHEMA_REGISTRY_CERTS_DIR' is configured by default, so what would be the scenario where it could be empty?

The same reason, SCHEMA_REGISTRY_CERTS_DIR is not need as we don't need to provide any certificate. A JAAS config should be enough.

@miguelbirdie
Copy link

We are just waiting for a review on this PR

Copy link

This Pull Request has been automatically marked as "stale" because it has not had recent activity (for 15 days). It will be closed if no further activity occurs. Thank you for your contribution.

@github-actions github-actions bot added the stale 15 days without activity label Sep 20, 2024
Copy link

Due to the lack of activity in the last 5 days since it was marked as "stale", we proceed to close this Pull Request. Do not hesitate to reopen it later if necessary.

@bitnami-bot bitnami-bot added stale 15 days without activity and removed stale 15 days without activity labels Sep 25, 2024
@dev-jimbo
Copy link

Hi @masfworld,

Thank you for your contribution!

I would like to ask several questions:

  • How does schema-registry not need certificates (keystore) when SASL_SSL protocol is used?
  • What is the reason to add the [[ -v "$SCHEMA_REGISTRY_CERTS_DIR" ]] conditional? The environment variable 'SCHEMA_REGISTRY_CERTS_DIR' is configured by default, so what would be the scenario where it could be empty?

Can you approve the PR #71059
I am having same issue and @masfworld provided the fix already
Thanks @masfworld !

@dev-jimbo
Copy link

BTW if your running SASL_SSL for an EXTERNAL KAFKA provider, why would you need certs?
Many other implementations don't require jks for ssl connection
Its essentially similar to saying:

for every website, you manually have to import cert into jks truststore prior to visiting the website

now if your running kafka local, then you would need certs, either signed or self-signed, i think the config does not make all combos clear...

@carrodher carrodher reopened this Oct 4, 2024
@carrodher carrodher removed stale 15 days without activity solved labels Oct 4, 2024
@github-actions github-actions bot added the triage Triage is needed label Oct 4, 2024
@carrodher carrodher removed the triage Triage is needed label Oct 5, 2024
@carrodher carrodher removed the request for review from javsalgar October 5, 2024 09:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
schema-registry verify Execute verification workflow for these changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bitnami/schema-registry] SASL_SSL is not working
7 participants