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

haproxy: add --haproxy-clientip flag to spoof client IPs #10779

Closed
wants to merge 1 commit into from

Conversation

RaitoBezarius
Copy link
Contributor

This PR enable --haproxy-clientip as per TODO item 5.2 (#5125).

I'm not certain yet I have done the appropriate things yet. I will add tests and docs soon. :)

@bagder bagder added the feature-window A merge of this requires an open feature window label Mar 16, 2023
@bagder bagder added needs-info and removed feature-window A merge of this requires an open feature window labels Mar 30, 2023
@RaitoBezarius RaitoBezarius marked this pull request as ready for review May 8, 2023 19:50
@github-actions github-actions bot added the tests label May 8, 2023
@RaitoBezarius
Copy link
Contributor Author

@bagder Hi there, I updated the PR and added some test cases, is there anything else to do so the contribution can be accepted?

@RaitoBezarius
Copy link
Contributor Author

(I see that my tests needs to be fixed too.)

@bagder
Copy link
Member

bagder commented May 9, 2023

The test 1912 problem can be fixed with this:

diff --git a/include/curl/typecheck-gcc.h b/include/curl/typecheck-gcc.h
index bc8d7a78c..b880f3dc6 100644
--- a/include/curl/typecheck-gcc.h
+++ b/include/curl/typecheck-gcc.h
@@ -278,10 +278,11 @@ CURLWARNING(_curl_easy_getinfo_err_curl_off_t,
    (option) == CURLOPT_EGDSOCKET ||                                           \
    (option) == CURLOPT_FTP_ACCOUNT ||                                         \
    (option) == CURLOPT_FTP_ALTERNATIVE_TO_USER ||                             \
    (option) == CURLOPT_FTPPORT ||                                             \
    (option) == CURLOPT_HSTS ||                                                \
+   (option) == CURLOPT_HAPROXY_CLIENT_IP ||                                   \
    (option) == CURLOPT_INTERFACE ||                                           \
    (option) == CURLOPT_ISSUERCERT ||                                          \
    (option) == CURLOPT_KEYPASSWD ||                                           \
    (option) == CURLOPT_KRBLEVEL ||                                            \
    (option) == CURLOPT_LOGIN_OPTIONS ||                                       \

@bagder
Copy link
Member

bagder commented May 9, 2023

The 1139 test failure happens because you have not created a docs/cmdline/haproxy-clientip.d file that documents the command line option (so it is missing from the curl.1 man page). Also add that file name to Makefile.inc and the DPAGES list to make it used in the build.

lib/setopt.c Show resolved Hide resolved
tests/data/test3201 Outdated Show resolved Hide resolved
@RaitoBezarius
Copy link
Contributor Author

Thank you for the guidance, I have fixed the remaining issues, I am checking the CI on my side but hopefully should be good.
I guess there's some adjustements for the documentation now that we enable --haproxy-protocol by default.
Do you see something else?

@RaitoBezarius
Copy link
Contributor Author

Hmm, it's unclear to me why 3201/3022 are failing now. Testing manually seems to work.

@RaitoBezarius RaitoBezarius force-pushed the haproxy-client-ip branch 2 times, most recently from 65f837b to 2835f34 Compare May 9, 2023 20:20
@bagder bagder added feature-window A merge of this requires an open feature window and removed needs-info labels May 14, 2023
docs/cmdline-opts/haproxy-clientip.d Show resolved Hide resolved
docs/libcurl/opts/CURLOPT_HAPROXY_CLIENT_IP.3 Outdated Show resolved Hide resolved
docs/libcurl/opts/CURLOPT_HAPROXY_CLIENT_IP.3 Outdated Show resolved Hide resolved
docs/libcurl/opts/CURLOPT_HAPROXY_CLIENT_IP.3 Outdated Show resolved Hide resolved
docs/libcurl/opts/CURLOPT_HAPROXY_CLIENT_IP.3 Outdated Show resolved Hide resolved
docs/libcurl/opts/CURLOPT_HAPROXY_CLIENT_IP.3 Outdated Show resolved Hide resolved
docs/libcurl/opts/CURLOPT_HAPROXY_CLIENT_IP.3 Outdated Show resolved Hide resolved
src/tool_cfgable.h Outdated Show resolved Hide resolved
@RaitoBezarius RaitoBezarius force-pushed the haproxy-client-ip branch 2 times, most recently from 1a31724 to cfa44f1 Compare May 18, 2023 19:01
@RaitoBezarius
Copy link
Contributor Author

@bagder I addressed the comments, rebased the PR, let me know if I can do anything more. :)

@RaitoBezarius RaitoBezarius force-pushed the haproxy-client-ip branch 2 times, most recently from 33be06a to b433705 Compare May 21, 2023 22:00
@bagder
Copy link
Member

bagder commented Jun 5, 2023

Thanks!

@bagder bagder closed this in 0a75964 Jun 5, 2023
@RaitoBezarius RaitoBezarius deleted the haproxy-client-ip branch June 5, 2023 18:19
bch pushed a commit to bch/curl that referenced this pull request Jul 19, 2023
CURLOPT_HAPROXY_CLIENT_IP in the library

Closes curl#10779
ptitSeb pushed a commit to wasix-org/curl that referenced this pull request Sep 25, 2023
CURLOPT_HAPROXY_CLIENT_IP in the library

Closes curl#10779
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

2 participants