Skip to content

Conversation

@vyavdoshenko
Copy link
Contributor

Fixes: #5084

Dragonfly required password authentication when TLS was enabled. However, the Memcached protocol doesn't support password authentication, making it impossible to use Memcached with TLS.

Modified the ValidateServerTlsFlags() function to allow TLS without authentication when the Memcached port is enabled.

@vyavdoshenko vyavdoshenko requested review from kostasrim and romange May 8, 2025 18:42
Copy link
Collaborator

@romange romange left a comment

Choose a reason for hiding this comment

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

@vyavdoshenko thank you for solving it so quickly! 🙏🏼

@vyavdoshenko vyavdoshenko requested a review from romange May 8, 2025 19:25
ssl_context.verify_mode = ssl.CERT_NONE

# Output port information for diagnostics
print(f"Connecting to memcached port: {server.mc_port} on host: 127.0.0.1")
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: we do not use prints in tests - only logging.info/debug etc

@romange romange merged commit 705d61e into main May 8, 2025
@romange romange deleted the bobik/memcache_tls_require_password branch May 8, 2025 21:01
@vyavdoshenko vyavdoshenko self-assigned this May 9, 2025
@romange
Copy link
Collaborator

romange commented May 9, 2025

@vyavdoshenko not urgent - can wait for next week, but i just I realized we fixed it wrong. We could still provide a password but always mark memcached connections as authenticated. in fact it's not related to TLS, this should be done regardless because otherwise nobody can use memcache port if password is defined for the datastore. I think this is what @kostasrim tried to say yesterday .

@vyavdoshenko
Copy link
Contributor Author

@vyavdoshenko not urgent - can wait for next week, but i just I realized we fixed it wrong. We could still provide a password but always mark memcached connections as authenticated. in fact it's not related to TLS, this should be done regardless because otherwise nobody can use memcache port if password is defined for the datastore. I think this is what @kostasrim tried to say yesterday .

I see. I will fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support TLS with memcache

4 participants