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

feat(authentication): enhance logging and integrate Fail2Ban configuration (#187) #188

Merged
merged 4 commits into from
Oct 12, 2024

Conversation

jiongxuan
Copy link
Contributor

@jiongxuan jiongxuan commented Oct 10, 2024

This PR includes several improvements to the authentication system's logging, along with a guide to integrating Fail2Ban to enhance the overall security of the WebDAV server.

Key Changes:

  1. Improve IP Logging for Real Client IP Address:

    • Commit: feat(authentication): improve IP logging by extracting real client IP from X-Forwarded-For header
    • Added the getRealRemoteIP function to correctly extract the client's real IP address when requests come through a reverse proxy.
    • Updated authentication logging to use the extracted IP instead of r.RemoteAddr.
    • Ensured fallback to r.RemoteAddr if X-Forwarded-For header is not present, maintaining compatibility with both proxy and non-proxy environments.
  2. Authentication Logging Enhancements:

    • Commit: feat(authentication): enhance login failure logging and reduce log volume
    • Added logging for failed login attempts specifically indicating invalid usernames, enhancing the detail in logs.
    • Removed redundant "login attempt" logs, focusing only on final validation results, which helps reduce log volume.
    • Retained detailed logging for invalid passwords and successful authorizations for greater clarity and auditability.
  3. Fail2Ban Integration Guide:

    • Commit: docs: add Fail2Ban configuration guide to README
    • Added a section in README.md to guide users on how to configure Fail2Ban with WebDAV.
    • Included examples for filter.d and jail.d configurations, allowing users to automatically ban IP addresses that exceed the allowed number of failed login attempts.

Issue Reference:

This PR closes #187, which requested improvements in logging details and Fail2Ban integration for better security.


Additional Context:

  • Real Client IP Address Extraction:

    • The addition of getRealRemoteIP helps to accurately log the IP address of the actual client, especially when the server is behind a reverse proxy. This is crucial for accurate logging and security analysis.
  • Fail2Ban Configuration:

    • The integration of Fail2Ban helps protect the WebDAV server from brute-force attacks by banning IP addresses that repeatedly fail to authenticate. This increases server resilience against malicious attempts.
  • Log Volume Optimization:

    • Reducing log entries to only include validation outcomes (i.e., success or failure) instead of every login attempt will help maintain clean logs that are easier to monitor and analyze.

Feel free to review the changes. Let me know if any further adjustments are needed or if there are additional requirements!

… from X-Forwarded-For header

- Added getRealRemoteIP function to retrieve the real client IP address when behind a reverse proxy.
- Updated authentication logging to use the extracted IP instead of r.RemoteAddr.
- Ensured compatibility for both proxy and non-proxy setups, falling back to r.RemoteAddr when X-Forwarded-For is not present.
- Added a section in README.md explaining how to configure Fail2Ban for WebDAV security.
- Included examples for filter and jail configuration.
- Provided instructions on setting up and testing Fail2Ban to block IPs after failed login attempts.
…lume

- Added logging for invalid username attempts to provide more detailed failure reasons.
- Removed "login attempt" log entries to reduce log volume and focus on final verification results.
- Retained logging for invalid password and successful user authorization for clarity.
lib/handler.go Outdated Show resolved Hide resolved
Copy link
Owner

@hacdias hacdias left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution. I will merge this once the comment above is tackled.

@jiongxuan
Copy link
Contributor Author

Thanks for the feedback! I’ve removed the commented line as requested.

I initially left it there for comparison purposes, to highlight the change in log volume, but I agree that removing it entirely makes the code cleaner and more focused. Thanks again!

@hacdias hacdias merged commit a698e31 into hacdias:main Oct 12, 2024
3 checks passed
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.

Small Feature: Improve Logging for Authentication and Proxy Scenarios
2 participants