-
Notifications
You must be signed in to change notification settings - Fork 1
[FEATURE] 동일 IP 중복접근 방지 필터구현 #340
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
Conversation
📝 WalkthroughSummary by CodeRabbit릴리스 노트
WalkthroughRedis 기반 IP별 레이트 리미트 필터(IPFilter)를 추가하고, SecurityConfig에 ObjectProvider로 주입하여 JWT 필터 앞에 조건부로 삽입하도록 보안 필터 체인을 변경했습니다. 필터는 분당 20회 초과 시 429 응답을 반환합니다. Changes
Sequence DiagramsequenceDiagram
participant Client
participant SecurityFilterChain
participant IPFilter
participant Redis
participant JWTFilter
participant Application
Client->>SecurityFilterChain: HTTP Request
SecurityFilterChain->>IPFilter: invoke (if available)
alt OPTIONS or infra endpoint
IPFilter-->>SecurityFilterChain: bypass (pass through)
SecurityFilterChain->>JWTFilter: continue
else normal request
IPFilter->>IPFilter: resolve client IP
IPFilter->>Redis: GET/INCR key(ip)
Redis-->>IPFilter: count
alt count < 20
IPFilter->>Redis: INCR + set TTL(if new)
Redis-->>IPFilter: ok
IPFilter-->>SecurityFilterChain: pass through
SecurityFilterChain->>JWTFilter: continue
JWTFilter->>Application: authenticate/process
Application-->>Client: 200/response
else count >= 20
IPFilter-->>Client: 429 (RETRY_GET_IMAGE JSON)
end
end
alt Redis error
IPFilter->>IPFilter: log warning (fail-open)
IPFilter-->>SecurityFilterChain: pass through
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Test Results215 tests 215 ✅ 12s ⏱️ Results for commit 23d8248. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/main/java/or/sopt/houme/global/filter/IPFilter.java (2)
44-44: ObjectMapper를 주입받는 방식으로 변경하세요.현재
new ObjectMapper()로 인스턴스를 직접 생성하고 있는데, 이는 Spring에서 관리하는 공유 ObjectMapper 설정(날짜 포맷, 모듈 등)을 활용하지 못합니다. 생성자 주입을 통해 Spring이 관리하는 ObjectMapper를 사용하면 일관된 JSON 직렬화 설정을 보장할 수 있습니다.다음과 같이 수정하세요:
private final StringRedisTemplate redisTemplate; - private final ObjectMapper objectMapper = new ObjectMapper(); + private final ObjectMapper objectMapper;
83-86: Fail-open 전략이 올바르게 구현되었습니다.Redis 장애 시 서비스 가용성을 우선시하는 fail-open 전략이 적절하게 구현되어 있습니다. 경고 로그를 남기면서 요청을 통과시키는 방식은 실용적인 선택입니다. 다만, 프로덕션 환경에서는 이러한 오류가 발생할 때 알림(alert)을 설정하여 Redis 문제를 신속하게 감지할 수 있도록 권장합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/or/sopt/houme/global/config/SecurityConfig.java(3 hunks)src/main/java/or/sopt/houme/global/filter/IPFilter.java(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-28T05:49:49.354Z
Learnt from: gdbs1107
Repo: TEAM-HOUME/HOUME-SERVER PR: 226
File: src/main/java/or/sopt/houme/domain/admin/controller/AdminSSRController.java:55-65
Timestamp: 2025-08-28T05:49:49.354Z
Learning: gdbs1107의 현재 JWTFilter는 Authorization 헤더에서만 Bearer 토큰을 추출하며, CustomUserDetailsService를 통해 사용자 인증을 처리함. Admin 전용 쿠키 인증 구현을 위해서는 JWTFilter 수정이 필요하지만, 현재는 localStorage/쿼리 파라미터 방식을 임시로 사용 중.
Applied to files:
src/main/java/or/sopt/houme/global/config/SecurityConfig.java
📚 Learning: 2025-08-28T05:49:49.354Z
Learnt from: gdbs1107
Repo: TEAM-HOUME/HOUME-SERVER PR: 226
File: src/main/java/or/sopt/houme/domain/admin/controller/AdminSSRController.java:55-65
Timestamp: 2025-08-28T05:49:49.354Z
Learning: gdbs1107은 기존 JWTFilter가 헤더에서만 토큰을 검증하고 있어 admin 전용 쿠키 검증 구현이 어렵다고 언급함. 아키텍처 호환성을 우선 고려하는 개발 스타일을 보임.
Applied to files:
src/main/java/or/sopt/houme/global/config/SecurityConfig.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (6)
src/main/java/or/sopt/houme/global/filter/IPFilter.java (3)
114-118: 프록시 환경에서 X-Forwarded-For 스푸핑 가능성을 검증하세요.현재 구현은
X-Forwarded-For헤더의 첫 번째 IP를 신뢰하고 있습니다. 만약 애플리케이션이 신뢰할 수 있는 프록시(예: AWS ALB, CloudFlare) 뒤에 있지 않거나, 클라이언트가 직접 접근 가능한 경우, 공격자가X-Forwarded-For헤더를 조작하여 rate limiting을 우회할 수 있습니다.다음 사항을 확인하세요:
- 애플리케이션이 신뢰할 수 있는 프록시(AWS WAF/ALB, CloudFlare 등) 뒤에서만 실행되는지
- 프록시가
X-Forwarded-For헤더를 올바르게 설정/덮어쓰는지- 직접 접근이 차단되어 있는지
인프라 설정을 확인하고, 필요시 다음을 고려하세요:
- Spring의
ForwardedHeaderFilter사용하여 프록시 헤더를 안전하게 처리- IP 검증 로직 추가 (올바른 IP 형식인지 확인)
- 문서화: 프록시 뒤에서만 운영되어야 함을 명시
35-35: 테스트 환경 비활성화 전략이 적절합니다.
@Profile("!test")설정과shouldNotFilter를 통한 제외 경로 설정이 잘 구현되어 있습니다. 테스트에 영향을 주지 않으면서도 모니터링 엔드포인트와 CORS preflight를 적절히 제외하고 있습니다.Also applies to: 47-62
96-96: Rate limiting에 부적합한 ErrorCode를 사용하고 있습니다.
ErrorCode.RETRY_GET_IMAGE는 이름상 이미지 조회 재시도를 위한 에러 코드로 보이는데, IP 기반 rate limiting에 사용되고 있습니다. 이는 코드 가독성과 유지보수성을 저해하며, 에러 메시지가 실제 상황과 맞지 않을 수 있습니다.다음 중 하나를 권장합니다:
- 새로운 ErrorCode 추가 (권장):
TOO_MANY_REQUESTS또는RATE_LIMIT_EXCEEDED같은 명확한 ErrorCode를 ErrorCode enum에 추가- 기존 ErrorCode 이름 변경:
RETRY_GET_IMAGE가 범용적인 rate limiting 용도라면 이름을TOO_MANY_REQUESTS로 변경예시:
// ErrorCode enum에 추가 TOO_MANY_REQUESTS(42900, "너무 많은 요청이 들어왔습니다. 잠시 후에 재요청하세요.", HttpStatus.TOO_MANY_REQUESTS)⛔ Skipped due to learnings
Learnt from: PBEM22 Repo: TEAM-HOUME/HOUME-SERVER PR: 234 File: src/main/java/or/sopt/houme/global/api/GlobalExceptionHandler.java:21-41 Timestamp: 2025-09-05T15:24:59.870Z Learning: PBEM22는 ImageFallbackException 핸들러에서 ErrorCode의 HTTP 상태와 관계없이 항상 HTTP 500을 반환하도록 의도적으로 설계했음. 이는 폴백 이미지 상황을 내부 서버 오류로 처리하는 비즈니스 로직임.src/main/java/or/sopt/houme/global/config/SecurityConfig.java (3)
9-9: IPFilter 의존성 추가가 올바르게 구현되었습니다.
@RequiredArgsConstructor를 통한 생성자 주입 방식으로 IPFilter를 추가한 것은 Spring의 권장 패턴을 따르고 있습니다. 기존 JWTFilter와 동일한 방식으로 일관성 있게 구현되었습니다.Also applies to: 37-37
119-122: 필터 체인 순서가 적절하게 설정되었습니다.IPFilter를 JWTFilter보다 먼저 실행하도록 배치한 것은 올바른 설계입니다:
- Rate limiting을 먼저 적용하여 과도한 요청을 조기에 차단
- JWT 검증 등 비용이 큰 작업을 차단된 요청에 대해 수행하지 않음
- PR 목적(WAF 보완을 위한 애플리케이션 레벨 즉시 차단)을 충족
주석도 명확하게 의도를 설명하고 있습니다.
37-37: test 프로파일에서 SecurityConfig의 IPFilter 의존성 주입 실패 가능성 검증됨검증 결과:
- IPFilter는
@Profile("!test")로 설정되어 test 프로파일에서 빈이 생성되지 않음- SecurityConfig는
@RequiredArgsConstructor로 IPFilter를 필수 의존성으로 요구함- test 프로파일을 사용하는 여러 테스트(
@ActiveProfiles("test")+@SpringBootTest)에서 SecurityConfig 빈 로드 시 IPFilter 주입 실패 가능성이 높음- test 설정 파일, TestConfiguration, 또는 모킹으로 이를 처리한 증거 없음
권장 조치:
- IPFilter를 Optional로 변경 (가장 권장):
private final Optional<IPFilter> ipFilter; // 필터 등록 시 ipFilter.ifPresent(filter -> http.addFilterBefore(filter, UsernamePasswordAuthenticationFilter.class) );
- 테스트용 Mock IPFilter 빈 생성:
@TestConfiguration public class TestSecurityConfig { @Bean @Profile("test") public IPFilter ipFilter() { return mock(IPFilter.class); } }
| Long count = redisTemplate.opsForValue().increment(key); | ||
| if (count != null && count == 1L) { | ||
| // 첫 요청: 키에 TTL(1분) 설정 | ||
| redisTemplate.expire(key, WINDOW); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redis 키 TTL 설정에 race condition이 존재합니다.
increment 호출 후 count == 1L일 때 expire를 호출하는 방식은 두 연산 사이에 간격이 있어 문제가 발생할 수 있습니다:
expire호출 실패 시 키가 영구적으로 남아 해당 IP가 영구 차단됩니다.- 분산 환경에서 동시 요청 시 TTL이 설정되지 않을 수 있습니다.
Redis의 SET 명령어에 EX 옵션을 사용하거나, Lua 스크립트로 원자적 연산을 구현하는 것이 더 안전합니다.
다음과 같이 수정을 권장합니다:
방법 1: SET with EX 옵션 사용 (단순하지만 INCR 대신 GET/SET 필요)
String countStr = redisTemplate.opsForValue().get(key);
Long count = (countStr != null) ? Long.parseLong(countStr) + 1 : 1L;
if (count == 1L) {
redisTemplate.opsForValue().set(key, String.valueOf(count), WINDOW);
} else {
redisTemplate.opsForValue().set(key, String.valueOf(count));
}방법 2: Lua 스크립트 사용 (권장 - 원자적 연산)
// 클래스 레벨에 스크립트 정의
private static final String LUA_SCRIPT =
"local count = redis.call('INCR', KEYS[1]) " +
"if count == 1 then " +
" redis.call('EXPIRE', KEYS[1], ARGV[1]) " +
"end " +
"return count";
// doFilterInternal에서 사용
DefaultRedisScript<Long> script = new DefaultRedisScript<>(LUA_SCRIPT, Long.class);
Long count = redisTemplate.execute(script,
Collections.singletonList(key),
String.valueOf(WINDOW.getSeconds()));🤖 Prompt for AI Agents
In src/main/java/or/sopt/houme/global/filter/IPFilter.java around lines 72 to
76, the current pattern calling increment() then expire() causes a race
condition (TTL may not be set) and can leave keys permanent; replace this with
an atomic Redis operation: implement and execute a Lua script (INCR +
conditional EXPIRE) via DefaultRedisScript to perform the increment and set TTL
in one atomic call, passing the key as KEYS[1] and the window seconds as
ARGV[1]; alternatively, if you must avoid Lua, use Redis SET with EX semantics
to create/update the counter atomically on first request and update the value
otherwise, but prefer the Lua approach for correctness in concurrent/distributed
scenarios.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/or/sopt/houme/global/filter/IPFilter.java (1)
74-78: Redis 키 TTL 설정의 race condition은 이미 지적되었습니다.
increment()후 별도로expire()를 호출하는 패턴은 두 연산 사이에 원자성이 보장되지 않아, TTL이 설정되지 않아 키가 영구적으로 남거나 분산 환경에서 문제가 발생할 수 있습니다. 이전 리뷰에서 Lua 스크립트를 사용한 원자적 연산으로 수정하도록 제안되었습니다.
🧹 Nitpick comments (2)
src/main/java/or/sopt/houme/global/filter/IPFilter.java (2)
40-46: ObjectMapper를 재사용하도록 리팩토링하세요.매 필터 인스턴스마다 새로운
ObjectMapper를 생성하고 있습니다.ObjectMapper는 thread-safe하고 생성 비용이 높으므로, Spring Bean으로 주입받거나static final로 선언하여 재사용하는 것이 권장됩니다.해결 방법: Spring Bean 주입 (권장)
+ private final ObjectMapper objectMapper; - private final ObjectMapper objectMapper = new ObjectMapper();
@RequiredArgsConstructor가 자동으로 생성자 주입을 처리하므로, SecurityConfig나 다른 설정에서 ObjectMapper 빈을 제공하면 됩니다.대안: static final 선언
- private final ObjectMapper objectMapper = new ObjectMapper(); + private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();그리고
objectMapper사용 부분을OBJECT_MAPPER로 변경하세요.
97-107: 에러 코드 명칭이 의미상 맞지 않을 수 있습니다.
ErrorCode.RETRY_GET_IMAGE를 레이트 리미팅 에러로 사용하고 있는데, 에러 코드 이름이 특정 기능("GET_IMAGE")에 한정되어 있어 범용적인 IP 레이트 리미팅 목적과 맞지 않아 보입니다.범용적인 레이트 리미팅 에러 코드(예:
ErrorCode.TOO_MANY_REQUESTS또는ErrorCode.RATE_LIMIT_EXCEEDED)를 추가하거나, 기존 코드가 이미 429 에러를 범용적으로 사용하는 것이라면 그대로 두셔도 됩니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/or/sopt/houme/global/config/SecurityConfig.java(3 hunks)src/main/java/or/sopt/houme/global/filter/IPFilter.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/or/sopt/houme/global/config/SecurityConfig.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-04T16:51:35.261Z
Learnt from: PBEM22
Repo: TEAM-HOUME/HOUME-SERVER PR: 231
File: src/main/java/or/sopt/houme/domain/credit/service/CreditServiceImpl.java:27-61
Timestamp: 2025-09-04T16:51:35.261Z
Learning: CreditServiceImpl.decreaseCreditAtomically에서 Redisson 락 해제를 finally가 아닌 TransactionSynchronizationManager.afterCompletion으로 처리하도록 PBEM22가 반영함. 향후 유사한 락 처리 논의 시 "커밋 이후 unlock" 패턴을 우선 제안한다.
Applied to files:
src/main/java/or/sopt/houme/global/filter/IPFilter.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (3)
src/main/java/or/sopt/houme/global/filter/IPFilter.java (3)
1-39: 클래스 구조와 조건부 활성화가 적절합니다.
@Profile("!test")와@ConditionalOnBean을 사용하여 테스트 환경과 Redis 미설정 환경에서 필터가 자동으로 비활성화되도록 구성한 점이 좋습니다.OncePerRequestFilter를 상속하여 요청당 한 번만 실행되도록 보장하고 있습니다.
48-64: 엔드포인트 제외 로직이 적절합니다.CORS Preflight와 인프라 모니터링 엔드포인트를 레이트 리미팅에서 제외하는 것은 합리적입니다. URI null 체크도 포함되어 있습니다.
85-88: Fail-open 전략이 적절하게 구현되었습니다.Redis 장애 시 서비스 가용성을 우선시하여 요청을 통과시키는 전략이 잘 구현되어 있습니다. 경고 로그도 적절하게 남기고 있습니다.
| private String resolveClientIp(HttpServletRequest request) { | ||
|
|
||
| // 프록시 환경 고려: X-Forwarded-For의 첫 IP → X-Real-IP → remoteAddr 순으로 사용 | ||
| String xff = request.getHeader("X-Forwarded-For"); | ||
| if (xff != null && !xff.isBlank()) { | ||
| String first = xff.split(",")[0].trim(); | ||
| if (!first.isBlank()) return first; | ||
| } | ||
| String xri = request.getHeader("X-Real-IP"); | ||
| return Optional.ofNullable(xri).filter(ip -> !ip.isBlank()).orElse(request.getRemoteAddr()); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
X-Forwarded-For 기반 IP 파싱에 스푸핑 취약점이 존재합니다.
X-Forwarded-For 헤더는 클라이언트가 임의로 조작할 수 있어, 신뢰할 수 없는 환경에서 레이트 리미팅을 우회할 수 있습니다. 예를 들어, 공격자가 X-Forwarded-For: 192.168.1.1을 매 요청마다 다르게 설정하면 레이트 리미팅을 무력화할 수 있습니다.
이 구현은 반드시 다음 조건에서만 안전합니다:
- 애플리케이션이 신뢰할 수 있는 리버스 프록시(ALB, CloudFront 등) 뒤에 위치
- 리버스 프록시가 클라이언트의
X-Forwarded-For헤더를 덮어쓰거나 검증 - 클라이언트가 애플리케이션에 직접 접근할 수 없음
검증 필요:
PR 설명에서 AWS WAF를 언급하고 있어 리버스 프록시가 있는 것으로 보이지만, 다음을 확인해주세요:
- 프로덕션 환경에서 클라이언트가 애플리케이션 서버에 직접 접근할 수 없는지
- ALB/CloudFront 등의 프록시가
X-Forwarded-For를 적절히 설정하고 있는지 - 보안 그룹/네트워크 정책으로 직접 접근이 차단되어 있는지
추가 권장사항:
배포 아키텍처에 대한 문서화를 추가하고, 만약 직접 접근 가능성이 있다면 request.getRemoteAddr()만 사용하거나, 신뢰할 수 있는 프록시 IP 범위를 검증하는 로직을 추가하세요.
// 예시: 신뢰할 수 있는 프록시에서만 X-Forwarded-For 사용
private static final Set<String> TRUSTED_PROXIES = Set.of("10.0.0.0/8", ...);
private String resolveClientIp(HttpServletRequest request) {
String remoteAddr = request.getRemoteAddr();
// 신뢰할 수 있는 프록시에서 온 요청만 X-Forwarded-For 사용
if (isTrustedProxy(remoteAddr)) {
String xff = request.getHeader("X-Forwarded-For");
if (xff != null && !xff.isBlank()) {
return xff.split(",")[0].trim();
}
}
return remoteAddr;
}
---
🏁 Script executed:
```shell
#!/bin/bash
# Find the IPFilter.java file and inspect context around lines 113-123
find . -name "IPFilter.java" -type f 2>/dev/null | head -20
Length of output: 57
🏁 Script executed:
#!/bin/bash
# Search for reverse proxy, proxy configuration, trusted proxy, WAF references
rg -i "reverse.proxy|trusted.proxy|x-forwarded|x-real-ip|aws.waf|alb|cloudfront" -C 5 --type java --type xml --type properties --type yml --type yaml -g '!**/test/**' 2>/dev/null | head -100Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look for deployment or architecture documentation
find . -type f \( -name "*deploy*" -o -name "*architecture*" -o -name "*README*" -o -name "*SECURITY*" \) 2>/dev/null | grep -v node_modules | head -30Length of output: 11
🏁 Script executed:
#!/bin/bash
# Check for application configuration files (application.yml, application.properties, etc.)
find . -type f \( -name "application*.yml" -o -name "application*.properties" \) 2>/dev/nullLength of output: 42
🏁 Script executed:
#!/bin/bash
# Search for IPFilter usage - where it's registered, how it's configured
rg -n "IPFilter" --type java -C 3 2>/dev/null | head -80Length of output: 3597
---
🏁 Script executed:
```shell
#!/bin/bash
# Read full IPFilter.java file
cat ./src/main/java/or/sopt/houme/global/filter/IPFilter.java
Length of output: 4399
🏁 Script executed:
#!/bin/bash
# Read README.md
cat ./README.mdLength of output: 1676
🏁 Script executed:
#!/bin/bash
# Read application-local.yml
cat ./src/main/resources/application-local.ymlLength of output: 2852
🏁 Script executed:
#!/bin/bash
# Read full SecurityConfig.java
cat ./src/main/java/or/sopt/houme/global/config/SecurityConfig.javaLength of output: 5536
🏁 Script executed:
#!/bin/bash
# Search for any network/security configuration, Spring security chain configuration
rg -n "addFilterBefore|addFilterAfter|securityFilterChain|httpSecurity" --type java -C 2 2>/dev/nullLength of output: 856
X-Forwarded-For 헤더 신뢰로 인한 IP 스푸핑 취약점 - 방어 계층 추가 필요
코드가 검증 없이 X-Forwarded-For 헤더를 신뢰하고 있습니다. 현재 코드베이스에는 신뢰할 수 있는 프록시 검증 로직이 없으며, 배포 환경에서 프록시/WAF 설정도 코드 수준에서 확인할 수 없습니다.
레이트 리미팅은 보안의 핵심이므로, 인프라 계층 보호에만 의존하지 말고 코드 수준에서도 방어해야 합니다. 다음 중 하나를 구현하세요:
Option 1: 신뢰할 수 있는 프록시 IP 범위 검증 (권장)
private static final Set<String> TRUSTED_PROXY_IPS = Set.of(
"10.0.0.0/8", // VPC CIDR
"172.31.0.0/16" // ALB/NLB 서브넷
);
private String resolveClientIp(HttpServletRequest request) {
String remoteAddr = request.getRemoteAddr();
// 신뢰할 수 있는 프록시에서만 X-Forwarded-For 사용
if (isTrustedProxyIp(remoteAddr)) {
String xff = request.getHeader("X-Forwarded-For");
if (xff != null && !xff.isBlank()) {
return xff.split(",")[0].trim();
}
}
return remoteAddr;
}Option 2: 프록시 환경이 확실하지 않으면 직접 주소만 사용
return request.getRemoteAddr();🤖 Prompt for AI Agents
In src/main/java/or/sopt/houme/global/filter/IPFilter.java around lines 113 to
123, the code currently trusts X-Forwarded-For unconditionally which allows IP
spoofing; update resolveClientIp to only use X-Forwarded-For when
request.getRemoteAddr() is a trusted proxy IP (implement an isTrustedProxyIp
that checks remoteAddr against a configured set of CIDR ranges/subnets),
otherwise return request.getRemoteAddr(); when using X-Forwarded-For, parse the
header safely by splitting on ',' and taking the first non-blank trimmed token
and validating its format before returning it.
📝 Code Coverage
|
kbt82883
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
와우 고생하셨습니다!
어찌보면 간단할 수 있지만 딱 필요한 곳에 필요한 만큼 검증하도록 작성해주신 것 같습니다.
| * */ | ||
| private String resolveClientIp(HttpServletRequest request) { | ||
|
|
||
| // 프록시 환경 고려: X-Forwarded-For의 첫 IP → X-Real-IP → remoteAddr 순으로 사용 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 고려사항 너무 좋네요! 하지만, 코드래빗이 말한 부분이 조금 우려스럽긴 합니다..
PBEM22
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네트워크단과 사용하는 도구들에 대해서 생각이 많아지는 부분이네요 ㅠ 고생하셨습니다. 코드래빗이 말한 부분들은 보안 취약점이 될 수도 있을 것 같아서 조금 수정하면 될것 같네용!
|
이때 필요했던 이유가 동일 IP에서 계속 이미지 생성 요청을 마운트? 하여 엄청 많은 요청이 대기큐에 쌓여있었어서 해당 PR이 올라왔던 것 같습니다. 그 사이에 동일 houseId로는 이미지 생성을 못하게 하는 애플리케이션단의 검증이 생기긴 했지만, 여전히 대기큐에 요청이 과다하게 몰리는 경우는 막지 못할 것 같습니다. 그렇기 떄문에 현재도 필요한 PR이라고 생각합니다! |
답변 감사드립니다!-! |
Application 단에서 막는 것은 코드 유지보수 비용과 자원 소모에 대해서 조금 더 생각을 해보느라 답변이 늦어졌습니다 ㅠ.ㅠ 그래서 전에 말씀드렸던 Nginx의 limit_req 모듈을 사용하여 인프라 레벨에서 처리하는게 어떤가 싶습니다.
만약 이 방식에 동의해주신다면, 제가 관련 설정을 Nginx에 적용해보도록 하겠습니다. 의견 부탁드립니다! |
결론부터 말씀드리자면 유지보수적인 부분이 커버가 가능하다면 nginx에서의 도입이 성능적으로 우수할 것으로 생각됩니다. 제가 구현했던 방법은 interceptor가 아닌 filter단에서 구현된 것으로 서블릿 컨테이너 내부로 진입하지 않기 때문에 인터셉터 대비 성능 오버헤드는 확실히 적을 것으로 예상됩니다만, 말씀하신대로 스레드가 할당되어 들어오는 것은 여전히 문제입니다. 제가 우려되는 부분은 두가지입니다.
그래서 결론을 정리해보자면
어떻게 생각하시는지 답변주시면 감사하겠습니다~! |
아 필터였죠!! 인터셉터로 이야기를 했네요.!
|
📣 Related Issue
📝 Summary
기능: 동일 IP 기준 1분당 20회까지 허용, 21번째 요청부터 429로 차단하는 IPFilter 추가
구현
배경(AWS WAF와의 2중 필터링)
🙏 Question & PR point
그러면 AWS WAF는 왜 필요한가요?
WAF가 없으면 10,000 개의 요청이 모두 인스턴스로 들어와 필터단에서 로직을 수행하게 됩니다. 그러면 결국 10,000 개의 요청을 처리하기 위해 필터단에서 부하가 발생하고 이는 곧 인스턴스의 부하 -> 서버가 터져버릴 수 있겠죠.
그래서 1차적으로 인스턴스에 들어오기 전에 WAF에서 요청을 필터링하고, 해당 요청들에 한해서 필터에서 2차적으로 요청을 처리하는 것이에요.
@ConditionalOnBean(StringRedisTemplate.class)
@ConditionalOnBean 은 Spring Boot의 조건부 빈 등록입니다. 이렇게 선언하면, StringRedisTemplate 이 이미 스프링 컨텍스트에 등록되어 있을 때만 IPFilter 클래스가 활성화됩니다.
도입 이유는 테스트 단에서는 레디스 관련 빈들을 등록하지 않기 때문에 필터단에서 모든 테스트를 위한 요청이 실패해버리는 문제를 해결하기 위함이었습니다.
📬 Postman