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

Auth is handled before the ratelimiter middleware #325

Open
shaunco opened this issue Aug 26, 2024 · 2 comments
Open

Auth is handled before the ratelimiter middleware #325

shaunco opened this issue Aug 26, 2024 · 2 comments
Assignees
Labels
enhancement New feature or request

Comments

@shaunco
Copy link

shaunco commented Aug 26, 2024

Describe the bug
Rate limiting (fail2ban) is usually used to prevent brute force auth attacks against a server. Wish offers both authentication and ratelimiting, but seems to call the auth handler first and then only calls the ratelimiter middleware if auth succeeds. Calling the rate limiter first produces the desired effect of preventing brute force auth attacks.

To Reproduce

s, err := wish.NewServer(
	wish.WithAddress(":2222"),
	wish.WithHostKeyPath(myHostKeyPath),
	wish.WithPasswordAuth(func(ctx ssh.Context, password string) bool {
		return password == "password"
	}),
	wish.WithMiddleware(
		ratelimiter.Middleware(ratelimiter.NewRateLimiter(1, 2, 1000)),
	),
)

In the above server, auth is always called - regardless of rate limits, and the ratelimiter Middleware is only called if auth succeeds (returns true).

Expected behavior
Rate limiting should happen before auth, perhaps via s.ConnCallback instead of middleware.

@caarlos0 caarlos0 self-assigned this Aug 29, 2024
@caarlos0 caarlos0 added the enhancement New feature or request label Aug 29, 2024
@pushand
Copy link

pushand commented Sep 21, 2024

does the order matter? move middleware before WithPasswardAuth?

@shaunco
Copy link
Author

shaunco commented Sep 21, 2024

The order doesn't matter. The WithMiddleware writes to an array, but WithPasswordAuth (and the other auth methods) each write to a different struct variable inside the Server struct, which are processed before the Middleware.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants
@caarlos0 @shaunco @pushand and others