Fix: pgsql_users.use_ssl not being enforced on frontend connections#5285
Fix: pgsql_users.use_ssl not being enforced on frontend connections#5285renecannao merged 6 commits intov3.0from
Conversation
Summary of ChangesHello @rahim-kanji, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a critical security oversight in PostgreSQL frontend connections where the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request correctly fixes a security issue where the use_ssl setting for PostgreSQL users was not being enforced, allowing non-SSL connections when SSL was required. The fix is straightforward and effective, assigning the use_ssl value from the database to the session object. The inclusion of a comprehensive test file is a great addition that validates the fix across different scenarios. I've added a couple of suggestions to improve the new test code's robustness and cleanliness. The PR also includes some code cleanup by removing the unused change_user_auth_switch logic, which is a welcome improvement.
test/tap/tests/pgsql-reg_test_5284_frontend_ssl_enforcement-t.cpp
Outdated
Show resolved
Hide resolved
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThese changes enforce SSL usage for PostgreSQL connections by removing authentication-switch logic, propagating SSL flags to session state, and introducing a comprehensive test validating SSL enforcement when Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
Comment |
Signed-off-by: René Cannaò <rene@proxysql.com>
|
Problem:
The
use_sslcolumn inpgsql_userstable was not being enforced. Clients could connect without SSL/TLS even whenuse_ssl=1was configured.Root Cause:
During PostgreSQL frontend authentication, the
use_sslvalue retrieved from the database was never assigned to the session object. The SSL enforcement logic was already in place, but it evaluated a session field that remained at its default value (false) regardless of the database configuration.Solution:
Assign the retrieved
use_sslvalue to the session after successful authentication lookup, consistent with how other user properties (default_hostgroup, transaction_persistent, fast_forward, user_max_connections) are already being set.Behavior After Fix:
Closes #5284
Summary by CodeRabbit
Release Notes
Tests
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.