mirror of
https://github.com/getnora-io/nora.git
synced 2026-04-12 16:10:31 +00:00
fix(rate-limit): add NORA_RATE_LIMIT_ENABLED flag and SmartIpKeyExtractor
- Add enabled field to RateLimitConfig (default: true, env: NORA_RATE_LIMIT_ENABLED) - Skip rate limiter layers entirely when disabled - Replace PeerIpKeyExtractor with SmartIpKeyExtractor for upload/general routes to correctly identify clients behind reverse proxies and Docker bridge networks - Keep PeerIpKeyExtractor for auth routes (stricter brute-force protection) Root cause: PeerIpKeyExtractor saw all Docker bridge traffic as single IP (172.17.0.1), exhausting GCRA bucket for all clients simultaneously. With burst=1M, recovery time reached 84000+ seconds.
This commit is contained in:
6
Cargo.lock
generated
6
Cargo.lock
generated
@@ -1201,7 +1201,7 @@ checksum = "38bf9645c8b145698bb0b18a4637dcacbc421ea49bef2317e4fd8065a387cf21"
|
|||||||
|
|
||||||
[[package]]
|
[[package]]
|
||||||
name = "nora-cli"
|
name = "nora-cli"
|
||||||
version = "0.2.22"
|
version = "0.2.24"
|
||||||
dependencies = [
|
dependencies = [
|
||||||
"clap",
|
"clap",
|
||||||
"flate2",
|
"flate2",
|
||||||
@@ -1215,7 +1215,7 @@ dependencies = [
|
|||||||
|
|
||||||
[[package]]
|
[[package]]
|
||||||
name = "nora-registry"
|
name = "nora-registry"
|
||||||
version = "0.2.22"
|
version = "0.2.24"
|
||||||
dependencies = [
|
dependencies = [
|
||||||
"async-trait",
|
"async-trait",
|
||||||
"axum",
|
"axum",
|
||||||
@@ -1253,7 +1253,7 @@ dependencies = [
|
|||||||
|
|
||||||
[[package]]
|
[[package]]
|
||||||
name = "nora-storage"
|
name = "nora-storage"
|
||||||
version = "0.2.22"
|
version = "0.2.24"
|
||||||
dependencies = [
|
dependencies = [
|
||||||
"axum",
|
"axum",
|
||||||
"base64",
|
"base64",
|
||||||
|
|||||||
@@ -249,6 +249,8 @@ impl Default for AuthConfig {
|
|||||||
/// - `NORA_RATE_LIMIT_GENERAL_BURST` - General burst size
|
/// - `NORA_RATE_LIMIT_GENERAL_BURST` - General burst size
|
||||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||||
pub struct RateLimitConfig {
|
pub struct RateLimitConfig {
|
||||||
|
#[serde(default = "default_rate_limit_enabled")]
|
||||||
|
pub enabled: bool,
|
||||||
#[serde(default = "default_auth_rps")]
|
#[serde(default = "default_auth_rps")]
|
||||||
pub auth_rps: u64,
|
pub auth_rps: u64,
|
||||||
#[serde(default = "default_auth_burst")]
|
#[serde(default = "default_auth_burst")]
|
||||||
@@ -263,6 +265,9 @@ pub struct RateLimitConfig {
|
|||||||
pub general_burst: u32,
|
pub general_burst: u32,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn default_rate_limit_enabled() -> bool {
|
||||||
|
true
|
||||||
|
}
|
||||||
fn default_auth_rps() -> u64 {
|
fn default_auth_rps() -> u64 {
|
||||||
1
|
1
|
||||||
}
|
}
|
||||||
@@ -285,6 +290,7 @@ fn default_general_burst() -> u32 {
|
|||||||
impl Default for RateLimitConfig {
|
impl Default for RateLimitConfig {
|
||||||
fn default() -> Self {
|
fn default() -> Self {
|
||||||
Self {
|
Self {
|
||||||
|
enabled: default_rate_limit_enabled(),
|
||||||
auth_rps: default_auth_rps(),
|
auth_rps: default_auth_rps(),
|
||||||
auth_burst: default_auth_burst(),
|
auth_burst: default_auth_burst(),
|
||||||
upload_rps: default_upload_rps(),
|
upload_rps: default_upload_rps(),
|
||||||
@@ -426,6 +432,9 @@ impl Config {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Rate limit config
|
// Rate limit config
|
||||||
|
if let Ok(val) = env::var("NORA_RATE_LIMIT_ENABLED") {
|
||||||
|
self.rate_limit.enabled = val.to_lowercase() == "true" || val == "1";
|
||||||
|
}
|
||||||
if let Ok(val) = env::var("NORA_RATE_LIMIT_AUTH_RPS") {
|
if let Ok(val) = env::var("NORA_RATE_LIMIT_AUTH_RPS") {
|
||||||
if let Ok(v) = val.parse::<u64>() {
|
if let Ok(v) = val.parse::<u64>() {
|
||||||
self.rate_limit.auth_rps = v;
|
self.rate_limit.auth_rps = v;
|
||||||
|
|||||||
@@ -210,6 +210,7 @@ async fn run_server(config: Config, storage: Storage) {
|
|||||||
|
|
||||||
// Log rate limiting configuration
|
// Log rate limiting configuration
|
||||||
info!(
|
info!(
|
||||||
|
enabled = config.rate_limit.enabled,
|
||||||
auth_rps = config.rate_limit.auth_rps,
|
auth_rps = config.rate_limit.auth_rps,
|
||||||
auth_burst = config.rate_limit.auth_burst,
|
auth_burst = config.rate_limit.auth_burst,
|
||||||
upload_rps = config.rate_limit.upload_rps,
|
upload_rps = config.rate_limit.upload_rps,
|
||||||
@@ -264,16 +265,49 @@ async fn run_server(config: Config, storage: Storage) {
|
|||||||
None
|
None
|
||||||
};
|
};
|
||||||
|
|
||||||
// Create rate limiters before moving config to state
|
let rate_limit_enabled = config.rate_limit.enabled;
|
||||||
let auth_limiter = rate_limit::auth_rate_limiter(&config.rate_limit);
|
|
||||||
let upload_limiter = rate_limit::upload_rate_limiter(&config.rate_limit);
|
|
||||||
let general_limiter = rate_limit::general_rate_limiter(&config.rate_limit);
|
|
||||||
|
|
||||||
// Initialize Docker auth with proxy timeout
|
// Initialize Docker auth with proxy timeout
|
||||||
let docker_auth = registry::DockerAuth::new(config.docker.proxy_timeout);
|
let docker_auth = registry::DockerAuth::new(config.docker.proxy_timeout);
|
||||||
|
|
||||||
let http_client = reqwest::Client::new();
|
let http_client = reqwest::Client::new();
|
||||||
|
|
||||||
|
// Registry routes (shared between rate-limited and non-limited paths)
|
||||||
|
let registry_routes = Router::new()
|
||||||
|
.merge(registry::docker_routes())
|
||||||
|
.merge(registry::maven_routes())
|
||||||
|
.merge(registry::npm_routes())
|
||||||
|
.merge(registry::cargo_routes())
|
||||||
|
.merge(registry::pypi_routes())
|
||||||
|
.merge(registry::raw_routes());
|
||||||
|
|
||||||
|
// Routes WITHOUT rate limiting (health, metrics, UI)
|
||||||
|
let public_routes = Router::new()
|
||||||
|
.merge(health::routes())
|
||||||
|
.merge(metrics::routes())
|
||||||
|
.merge(ui::routes())
|
||||||
|
.merge(openapi::routes());
|
||||||
|
|
||||||
|
let app_routes = if rate_limit_enabled {
|
||||||
|
// Create rate limiters before moving config to state
|
||||||
|
let auth_limiter = rate_limit::auth_rate_limiter(&config.rate_limit);
|
||||||
|
let upload_limiter = rate_limit::upload_rate_limiter(&config.rate_limit);
|
||||||
|
let general_limiter = rate_limit::general_rate_limiter(&config.rate_limit);
|
||||||
|
|
||||||
|
let auth_routes = auth::token_routes().layer(auth_limiter);
|
||||||
|
let limited_registry = registry_routes.layer(upload_limiter);
|
||||||
|
|
||||||
|
Router::new()
|
||||||
|
.merge(auth_routes)
|
||||||
|
.merge(limited_registry)
|
||||||
|
.layer(general_limiter)
|
||||||
|
} else {
|
||||||
|
info!("Rate limiting DISABLED");
|
||||||
|
Router::new()
|
||||||
|
.merge(auth::token_routes())
|
||||||
|
.merge(registry_routes)
|
||||||
|
};
|
||||||
|
|
||||||
let state = Arc::new(AppState {
|
let state = Arc::new(AppState {
|
||||||
storage,
|
storage,
|
||||||
config,
|
config,
|
||||||
@@ -287,35 +321,9 @@ async fn run_server(config: Config, storage: Storage) {
|
|||||||
http_client,
|
http_client,
|
||||||
});
|
});
|
||||||
|
|
||||||
// Token routes with strict rate limiting (brute-force protection)
|
|
||||||
let auth_routes = auth::token_routes().layer(auth_limiter);
|
|
||||||
|
|
||||||
// Registry routes with upload rate limiting
|
|
||||||
let registry_routes = Router::new()
|
|
||||||
.merge(registry::docker_routes())
|
|
||||||
.merge(registry::maven_routes())
|
|
||||||
.merge(registry::npm_routes())
|
|
||||||
.merge(registry::cargo_routes())
|
|
||||||
.merge(registry::pypi_routes())
|
|
||||||
.merge(registry::raw_routes())
|
|
||||||
.layer(upload_limiter);
|
|
||||||
|
|
||||||
// Routes WITHOUT rate limiting (health, metrics, UI)
|
|
||||||
let public_routes = Router::new()
|
|
||||||
.merge(health::routes())
|
|
||||||
.merge(metrics::routes())
|
|
||||||
.merge(ui::routes())
|
|
||||||
.merge(openapi::routes());
|
|
||||||
|
|
||||||
// Routes WITH rate limiting
|
|
||||||
let rate_limited_routes = Router::new()
|
|
||||||
.merge(auth_routes)
|
|
||||||
.merge(registry_routes)
|
|
||||||
.layer(general_limiter);
|
|
||||||
|
|
||||||
let app = Router::new()
|
let app = Router::new()
|
||||||
.merge(public_routes)
|
.merge(public_routes)
|
||||||
.merge(rate_limited_routes)
|
.merge(app_routes)
|
||||||
.layer(DefaultBodyLimit::max(100 * 1024 * 1024)) // 100MB default body limit
|
.layer(DefaultBodyLimit::max(100 * 1024 * 1024)) // 100MB default body limit
|
||||||
.layer(middleware::from_fn(request_id::request_id_middleware))
|
.layer(middleware::from_fn(request_id::request_id_middleware))
|
||||||
.layer(middleware::from_fn(metrics::metrics_middleware))
|
.layer(middleware::from_fn(metrics::metrics_middleware))
|
||||||
|
|||||||
@@ -10,6 +10,7 @@
|
|||||||
|
|
||||||
use crate::config::RateLimitConfig;
|
use crate::config::RateLimitConfig;
|
||||||
use tower_governor::governor::GovernorConfigBuilder;
|
use tower_governor::governor::GovernorConfigBuilder;
|
||||||
|
use tower_governor::key_extractor::SmartIpKeyExtractor;
|
||||||
|
|
||||||
/// Create rate limiter layer for auth endpoints (strict protection against brute-force)
|
/// Create rate limiter layer for auth endpoints (strict protection against brute-force)
|
||||||
pub fn auth_rate_limiter(
|
pub fn auth_rate_limiter(
|
||||||
@@ -35,11 +36,12 @@ pub fn auth_rate_limiter(
|
|||||||
pub fn upload_rate_limiter(
|
pub fn upload_rate_limiter(
|
||||||
config: &RateLimitConfig,
|
config: &RateLimitConfig,
|
||||||
) -> tower_governor::GovernorLayer<
|
) -> tower_governor::GovernorLayer<
|
||||||
tower_governor::key_extractor::PeerIpKeyExtractor,
|
SmartIpKeyExtractor,
|
||||||
governor::middleware::StateInformationMiddleware,
|
governor::middleware::StateInformationMiddleware,
|
||||||
axum::body::Body,
|
axum::body::Body,
|
||||||
> {
|
> {
|
||||||
let gov_config = GovernorConfigBuilder::default()
|
let gov_config = GovernorConfigBuilder::default()
|
||||||
|
.key_extractor(SmartIpKeyExtractor)
|
||||||
.per_second(config.upload_rps)
|
.per_second(config.upload_rps)
|
||||||
.burst_size(config.upload_burst)
|
.burst_size(config.upload_burst)
|
||||||
.use_headers()
|
.use_headers()
|
||||||
@@ -53,11 +55,12 @@ pub fn upload_rate_limiter(
|
|||||||
pub fn general_rate_limiter(
|
pub fn general_rate_limiter(
|
||||||
config: &RateLimitConfig,
|
config: &RateLimitConfig,
|
||||||
) -> tower_governor::GovernorLayer<
|
) -> tower_governor::GovernorLayer<
|
||||||
tower_governor::key_extractor::PeerIpKeyExtractor,
|
SmartIpKeyExtractor,
|
||||||
governor::middleware::StateInformationMiddleware,
|
governor::middleware::StateInformationMiddleware,
|
||||||
axum::body::Body,
|
axum::body::Body,
|
||||||
> {
|
> {
|
||||||
let gov_config = GovernorConfigBuilder::default()
|
let gov_config = GovernorConfigBuilder::default()
|
||||||
|
.key_extractor(SmartIpKeyExtractor)
|
||||||
.per_second(config.general_rps)
|
.per_second(config.general_rps)
|
||||||
.burst_size(config.general_burst)
|
.burst_size(config.general_burst)
|
||||||
.use_headers()
|
.use_headers()
|
||||||
@@ -102,6 +105,7 @@ mod tests {
|
|||||||
#[test]
|
#[test]
|
||||||
fn test_custom_config() {
|
fn test_custom_config() {
|
||||||
let config = RateLimitConfig {
|
let config = RateLimitConfig {
|
||||||
|
enabled: true,
|
||||||
auth_rps: 10,
|
auth_rps: 10,
|
||||||
auth_burst: 20,
|
auth_burst: 20,
|
||||||
upload_rps: 500,
|
upload_rps: 500,
|
||||||
|
|||||||
Reference in New Issue
Block a user