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

[feature]: make HTTP read header timeout configurable #7232

Closed
guggero opened this issue Dec 5, 2022 · 4 comments · Fixed by #7715
Closed

[feature]: make HTTP read header timeout configurable #7232

guggero opened this issue Dec 5, 2022 · 4 comments · Fixed by #7715
Assignees
Labels
config Parameters/arguments/config file related issues/PRs enhancement Improvements to existing features / behaviour good first issue Issues suitable for first time contributors to LND P3 might get fixed, nice to have REST

Comments

@guggero
Copy link
Collaborator

guggero commented Dec 5, 2022

Is your feature request related to a problem? Please describe.

One of the new linters has a rule to detect potential problems with intentionally slow clients:
G112: Potential Slowloris Attack because ReadHeaderTimeout is not configured in the http.Server (gosec)

Describe the solution you'd like

We should add the ReadHeaderTimeout config value everywhere we use a http.Server (non-exhaustive list of examples: REST proxy server, pprof server, let's encrypt cert server) and also make the value globally configurable.

@guggero guggero added enhancement Improvements to existing features / behaviour config Parameters/arguments/config file related issues/PRs REST P3 might get fixed, nice to have labels Dec 5, 2022
@saubyk saubyk added the good first issue Issues suitable for first time contributors to LND label Dec 8, 2022
@ErikEk
Copy link
Contributor

ErikEk commented Jan 10, 2023

I suppose we keep the current behavior as is (which means no timeout) if undefined. I did however notice Nginx have its default read header timeout set to 60s (http://nginx.org/en/docs/http/ngx_http_core_module.html#client_header_timeout), while the default setup for Apache has no timeout.

@Chinwendu20
Copy link
Contributor

Chinwendu20 commented Jan 31, 2023

Please I am a first timer here and I am looking for an issue to get started with. I would like to know if I can take on this

@Chinwendu20
Copy link
Contributor

Hello, I did a quick search for HTTP. Server and these are the files, I found it:

image
Also, what is your proposal on the value for the ReadHeaderTimeout, 60s?

@ErikEk
Copy link
Contributor

ErikEk commented Mar 9, 2023

I would leave it unchanged for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
config Parameters/arguments/config file related issues/PRs enhancement Improvements to existing features / behaviour good first issue Issues suitable for first time contributors to LND P3 might get fixed, nice to have REST
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants