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

address Potential Slowloris Attack because ReadHeaderTimeout is not configured in the http.Server #735

Merged
merged 2 commits into from
Aug 12, 2022

Conversation

cpanato
Copy link
Member

@cpanato cpanato commented Aug 11, 2022

Summary

  • address Potential Slowloris Attack because ReadHeaderTimeout is not configured in the http.Server
  • update golangci-lint to 1.48.0

Release Note

  • address Potential Slowloris Attack because ReadHeaderTimeout is not configured in the http.Server

@haydentherapper
Copy link
Contributor

@cpanato Is there an equivalent setting for the gRPC handler?

@cpanato
Copy link
Member Author

cpanato commented Aug 11, 2022

@cpanato Is there an equivalent setting for the gRPC handler?

i'm not aware, but we can check.

UPDATE: i'm not sure, did not find anything, but I dont have much experience with grpc :/

cmd/app/serve.go Outdated
@@ -80,6 +80,7 @@ func newServeCmd() *cobra.Command {
cmd.Flags().String("grpc-host", "0.0.0.0", "The host on which to serve requests for GRPC")
cmd.Flags().String("grpc-port", "8081", "The port on which to serve requests for GRPC")
cmd.Flags().String("metrics-port", "2112", "The port on which to serve prometheus metrics endpoint")
cmd.Flags().Duration("read-header-timeout", 60*time.Second, "The time allowed to read the headers of the requests in seconds")
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this too long, should it be ~10 seconds or less (not much in the headers besides an auth token)? I assume we have request timeouts that would trigger before this, though I'm not sure where those are configured (k8s?).

Copy link
Member Author

Choose a reason for hiding this comment

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

nginx uses 60s by default
i will reduce that to 10

Copy link
Member Author

Choose a reason for hiding this comment

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

done

we need to check what is the value we are setting on the nginx side (client_header_timeout)

Copy link
Contributor

@haydentherapper haydentherapper left a comment

Choose a reason for hiding this comment

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

thanks!

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.

3 participants