Skip to content

Commit 1aa4b1d

Browse files
committed
Auto merge of rust-lang#1596 - sgrif:sg-realip, r=sgrif
Rate limit the crate publish endpoint This adds a restriction to the number of crates that can be published from a single IP address in a single time period. This is done per IP instead of per-user, since that's what we can do with nginx alone. We may want to do additional rate limiting per token to force spammers to register additional accounts, and not just change their IP. This will use what's called the "leaky bucket" strategy for rate limiting. Basically every user has a bucket of tokens they use to publish crates. They start with 10 tokens available. Every time they hit this endpoint, they use one of the tokens. We give them a new token each minute. What this means is that you can upload 1 crate per minute, but we allow you to upload up to 10 in a short period of time before we start enforcing the rate limit. When someone does hit the rate limit, they will receive a 429 response. We could also allow it to instead just slow down the requests, refusing to process them until a token is available (queueing a max of 10 requests). This reserves 10 megabyte of memory for the IP table, which means we can hold about 80000 IPs. When the table is full, it will attempt to drop the oldest record, and if that doesn't give enough space, it'll give a 503. Keep in mind this is all in memory, not shared between our servers. This means that it is possible (but not guaranteed) that someone can upload 20 crates, and then send 2 requests per minute. On Heroku and any system that involves proxies, the remote_addr of the request becomes useless, as it will be the proxy forwarding your request rather than the client itself. Most proxies will set or append an `X-Forwarded-For` header, putting whatever remote_addr it received at the end of the the header. So if there are 3 proxies, the header will look like real_ip, proxy1, proxy2 However, if we're not careful this makes us vulnerable to IP spoofing. A lot of implementations just look at the first value in the header. All the proxies just append the header though (they don't know if they're getting traffic from the origin or a proxy), so you end up with spoofed_ip, real_ip, proxy1, proxy2 nginx, Rails, and many other implementations avoid this by requiring an allowlist of trusted IPs. With this configuration, the realip only kicks in if remote_addr is a trusted proxy, and then it will replace it with the last non-trusted IP from the header.
2 parents cfafa89 + ac4d090 commit 1aa4b1d

File tree

5 files changed

+33
-12
lines changed

5 files changed

+33
-12
lines changed

.buildpacks

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
https://github.com/Starkast/heroku-buildpack-cmake#a243c67
22
https://github.com/emk/heroku-buildpack-rust#578d630
33
https://codon-buildpacks.s3.amazonaws.com/buildpacks/heroku/emberjs.tgz
4-
https://github.com/travis-ci/nginx-buildpack.git#2fbde35
4+
https://github.com/heroku/heroku-buildpack-nginx.git#fbc49cd
55
https://github.com/sgrif/heroku-buildpack-diesel#f605edd

app/templates/policies.hbs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,12 @@ them. If necessary, the team may reach out to inactive maintainers and help
2525
mediate the process of ownership transfer.
2626
</p>
2727

28+
<p>
29+
Using an automated tool to claim ownership of a large number of package names
30+
is not permitted. We reserve the right to block traffic or revoke ownership
31+
of any package we determine to have been claimed by an automated tool.
32+
</p>
33+
2834
<h2 id='removal'><a href='#removal'>Removal</a></h2>
2935

3036
<p>

config/nginx.conf.erb

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@ events {
99
}
1010

1111
http {
12+
set_real_ip_from 10.0.0.0/8;
13+
set_real_ip_from 127.0.0.0/24;
14+
real_ip_header X-Forwarded-For;
15+
real_ip_recursive on;
16+
1217
gzip on;
1318
gzip_comp_level 2;
1419
gzip_proxied any;
@@ -28,6 +33,8 @@ http {
2833
client_body_timeout 30;
2934
client_max_body_size 50m;
3035

36+
limit_req_zone $remote_addr zone=publish:10m rate=1r/m;
37+
3138
upstream app_server {
3239
server localhost:8888 fail_timeout=0;
3340
}
@@ -38,22 +45,30 @@ http {
3845
keepalive_timeout 5;
3946

4047
location ~ ^/assets/ {
41-
add_header Strict-Transport-Security "max-age=31536000" always;
4248
add_header X-Content-Type-Options nosniff;
4349
add_header Cache-Control public;
4450
root dist;
4551
expires max;
4652
}
4753

54+
add_header Strict-Transport-Security "max-age=31536000" always;
55+
add_header Vary 'Accept, Accept-Encoding, Cookie';
56+
proxy_set_header Host $http_host;
57+
proxy_set_header X-Real-Ip $remote_addr;
58+
proxy_redirect off;
59+
if ($http_x_forwarded_proto != 'https') {
60+
rewrite ^ https://$host$request_uri? permanent;
61+
}
62+
4863
location / {
49-
add_header Strict-Transport-Security "max-age=31536000" always;
50-
add_header Vary 'Accept, Accept-Encoding, Cookie';
51-
proxy_set_header Host $http_host;
52-
proxy_redirect off;
53-
if ($http_x_forwarded_proto != 'https') {
54-
rewrite ^ https://$host$request_uri? permanent;
55-
}
5664
proxy_pass http://app_server;
5765
}
66+
67+
location ~ ^/api/v./crates/new$ {
68+
proxy_pass http://app_server;
69+
70+
limit_req zone=publish burst=10 nodelay;
71+
limit_req_status 429;
72+
}
5873
}
5974
}

src/middleware/block_ips.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,10 @@ impl Handler for BlockIps {
2929
fn call(&self, req: &mut dyn Request) -> Result<Response, Box<dyn Error + Send>> {
3030
let has_blocked_ip = req
3131
.headers()
32-
.find("X-Forwarded-For")
32+
.find("X-Real-Ip")
3333
.unwrap()
3434
.iter()
35-
.any(|v| v.split(", ").any(|ip| self.ips.iter().any(|x| x == ip)));
35+
.any(|ip| self.ips.iter().any(|v| v == ip));
3636
if has_blocked_ip {
3737
let body = format!(
3838
"We are unable to process your request at this time. \

src/middleware/log_request.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ impl Handler for LogRequests {
3838
level = level,
3939
method = req.method(),
4040
path = FullPath(req),
41-
ip = request_header(req, "X-Forwarded-For"),
41+
ip = request_header(req, "X-Real-Ip"),
4242
time_ms = response_time,
4343
user_agent = request_header(req, "User-Agent"),
4444
referer = request_header(req, "Referer"), // sic

0 commit comments

Comments
 (0)