Conversation
사용할 aes 대칭 키값도 임시로 등록 (나중에 .env파일에 등록 필요)
WalkthroughQuizTestController에 QuizPageService 의존성이 추가되고 생성자 시그니처가 변경되었습니다. QuizPageService는 AesUtil을 주입 받아 다중선택·서술형 퀴즈 응답의 정답·해설 필드를 AES/CBC로 암호화하도록 변경되었습니다. 새로운 AesUtil 클래스와 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as 클라이언트
participant QPS as QuizPageService
participant AES as AesUtil
C->>QPS: getMultipleQuiz()/getDescriptiveQuiz(...)
activate QPS
QPS->>QPS: DTO 필드 구성(정답/해설 포함)
QPS->>AES: encrypt(answer / answerNumber / commentary)
activate AES
AES-->>QPS: Base64(IV + ciphertext)
deactivate AES
QPS-->>C: 암호화된 answer/commentary가 포함된 DTO 반환
deactivate QPS
Note over QPS,AES: 변경점: 응답 전 민감 필드 암호화 추가
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
Ksr-ccb
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
cs25-service/src/main/java/com/example/cs25service/domain/quiz/controller/QuizTestController.java (1)
4-19: 불필요한 서비스 주입 및 import 정리 제안현재
QuizPageService,TodayQuizResponseDto,PathVariable,RequestParam모두 주석 처리된 엔드포인트에서만 쓰이며 실제 코드 경로에서는 사용되지 않아 경고(또는 스타일 검사 실패)의 원인이 됩니다. 테스트 엔드포인트를 당장 노출하지 않을 계획이라면, 관련 의존성과 필드를 제거해 두는 편이 안전합니다. 추후 활성화 시 다시 추가하면 됩니다.적용 가능한 변경 예시는 아래와 같습니다:
-import com.example.cs25service.domain.quiz.dto.TodayQuizResponseDto; import com.example.cs25service.domain.quiz.service.QuizAccuracyCalculateService; -import com.example.cs25service.domain.quiz.service.QuizPageService; import lombok.RequiredArgsConstructor; import org.springframework.web.bind.annotation.GetMapping; -import org.springframework.web.bind.annotation.PathVariable; -import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.RestController; @@ - private final QuizPageService quizPageService;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
cs25-service/src/main/java/com/example/cs25service/domain/quiz/controller/QuizTestController.java(2 hunks)cs25-service/src/main/java/com/example/cs25service/domain/quiz/service/QuizPageService.java(4 hunks)cs25-service/src/main/java/com/example/cs25service/domain/quiz/util/AesUtil.java(1 hunks)cs25-service/src/main/resources/application.properties(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
cs25-service/src/main/java/com/example/cs25service/domain/quiz/controller/QuizTestController.java (1)
cs25-service/src/main/java/com/example/cs25service/domain/quiz/controller/QuizPageController.java (1)
RestController(11-28)
🪛 ast-grep (0.39.5)
cs25-service/src/main/java/com/example/cs25service/domain/quiz/util/AesUtil.java
[warning] 32-32: Triple DES (3DES or DESede) is considered deprecated. AES is the recommended cipher. Upgrade to use AES.
Context: Cipher.getInstance(ALGORITHM)
Note: [CWE-326]: Inadequate Encryption Strength [OWASP A03:2017]: Sensitive Data Exposure [OWASP A02:2021]: Cryptographic Failures [REFERENCES]
- https://find-sec-bugs.github.io/bugs.htm#TDES_USAGE
- https://csrc.nist.gov/News/2017/Update-to-Current-Use-and-Deprecation-of-TDEA
(desede-is-deprecated-java)
[warning] 62-62: Triple DES (3DES or DESede) is considered deprecated. AES is the recommended cipher. Upgrade to use AES.
Context: Cipher.getInstance(ALGORITHM)
Note: [CWE-326]: Inadequate Encryption Strength [OWASP A03:2017]: Sensitive Data Exposure [OWASP A02:2021]: Cryptographic Failures [REFERENCES]
- https://find-sec-bugs.github.io/bugs.htm#TDES_USAGE
- https://csrc.nist.gov/News/2017/Update-to-Current-Use-and-Deprecation-of-TDEA
(desede-is-deprecated-java)
[warning] 32-32: Use of AES with ECB mode detected. ECB doesn't provide message confidentiality and is not semantically secure so should not be used. Instead, use a strong, secure cipher: Cipher.getInstance("AES/CBC/PKCS7PADDING"). See https://owasp.org/www-community/Using_the_Java_Cryptographic_Extensions for more information.
Context: Cipher.getInstance(ALGORITHM)
Note: [CWE-327]: Use of a Broken or Risky Cryptographic Algorithm [OWASP A03:2017]: Sensitive Data Exposure [OWASP A02:2021]: Cryptographic Failures [REFERENCES]
- https://owasp.org/Top10/A02_2021-Cryptographic_Failures
- https://googleprojectzero.blogspot.com/2022/10/rc4-is-still-considered-harmful.html
(use-of-aes-ecb-java)
[warning] 62-62: Use of AES with ECB mode detected. ECB doesn't provide message confidentiality and is not semantically secure so should not be used. Instead, use a strong, secure cipher: Cipher.getInstance("AES/CBC/PKCS7PADDING"). See https://owasp.org/www-community/Using_the_Java_Cryptographic_Extensions for more information.
Context: Cipher.getInstance(ALGORITHM)
Note: [CWE-327]: Use of a Broken or Risky Cryptographic Algorithm [OWASP A03:2017]: Sensitive Data Exposure [OWASP A02:2021]: Cryptographic Failures [REFERENCES]
- https://owasp.org/Top10/A02_2021-Cryptographic_Failures
- https://googleprojectzero.blogspot.com/2022/10/rc4-is-still-considered-harmful.html
(use-of-aes-ecb-java)
| .answerNumber(aesUtil.encrypt(answerNumber)) | ||
| .commentary(aesUtil.encrypt(quiz.getCommentary())) | ||
| .quizType(quiz.getType().name()) |
There was a problem hiding this comment.
암호화로 인해 기존 응답 스키마가 깨집니다
Line 61과 Line 79에서 바로 암호화된 값을 내려보내면 /todayQuiz 실제 엔드포인트를 소비하는 기존 프런트가 즉시 깨집니다(현재 배포본은 복호화 로직이 없음). 프런트 적용이 완료될 때까지는 최소한 기능 플래그나 엔드포인트 버전 분리로 기존 소비자에게 평문을 유지해야 합니다.
Also applies to: 79-80
| private final String secretKey; | ||
|
|
||
| public AesUtil(@Value("${aes.secret.key}") String secretKey) { | ||
| this.secretKey = secretKey; | ||
| } | ||
|
|
||
| private final String ALGORITHM = "AES/CBC/PKCS5Padding"; | ||
|
|
||
| private SecretKey getKey() { | ||
| return new SecretKeySpec(secretKey.getBytes(StandardCharsets.UTF_8), "AES"); | ||
| } |
There was a problem hiding this comment.
비정상 AES 키 길이 허용으로 인한 런타임 실패 위험
현재 secretKey를 그대로 보관했다가 SecretKeySpec을 매번 만들고 있습니다. 프로퍼티 값이 16·24·32바이트가 아니거나 UTF-8 멀티바이트 문자를 포함하면 Cipher.init() 단계에서 InvalidKeyException이 발생해 암·복호화가 모두 실패합니다. PR 설명에도 48바이트 키 사용 가능하다고 적혀 있어 실제로 그렇게 설정하면 바로 장애가 납니다. 생성자에서 키 길이를 검증하고, 한 번 생성한 SecretKey를 재사용하도록 수정해 주세요.
- private final String secretKey;
-
- public AesUtil(@Value("${aes.secret.key}") String secretKey) {
- this.secretKey = secretKey;
- }
-
- private SecretKey getKey() {
- return new SecretKeySpec(secretKey.getBytes(StandardCharsets.UTF_8), "AES");
- }
+ private final SecretKey secretKey;
+
+ public AesUtil(@Value("${aes.secret.key}") String secretKey) {
+ byte[] keyBytes = secretKey.getBytes(StandardCharsets.UTF_8);
+ if (keyBytes.length != 16 && keyBytes.length != 24 && keyBytes.length != 32) {
+ throw new IllegalArgumentException("aes.secret.key는 16, 24 또는 32바이트여야 합니다.");
+ }
+ this.secretKey = new SecretKeySpec(keyBytes, "AES");
+ }
+
+ private SecretKey getKey() {
+ return secretKey;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private final String secretKey; | |
| public AesUtil(@Value("${aes.secret.key}") String secretKey) { | |
| this.secretKey = secretKey; | |
| } | |
| private final String ALGORITHM = "AES/CBC/PKCS5Padding"; | |
| private SecretKey getKey() { | |
| return new SecretKeySpec(secretKey.getBytes(StandardCharsets.UTF_8), "AES"); | |
| } | |
| // Replace the String-backed key with a pre-validated, cached SecretKey | |
| private final SecretKey secretKey; | |
| public AesUtil(@Value("${aes.secret.key}") String secretKey) { | |
| byte[] keyBytes = secretKey.getBytes(StandardCharsets.UTF_8); | |
| if (keyBytes.length != 16 && keyBytes.length != 24 && keyBytes.length != 32) { | |
| throw new IllegalArgumentException("aes.secret.key는 16, 24 또는 32바이트여야 합니다."); | |
| } | |
| this.secretKey = new SecretKeySpec(keyBytes, "AES"); | |
| } | |
| private final String ALGORITHM = "AES/CBC/PKCS5Padding"; | |
| private SecretKey getKey() { | |
| return secretKey; | |
| } |
Ksr-ccb
changed the title
1 participant
🔎 작업 내용
🛠️ 변경 사항
－ 임시 ａｐｉ 만들어서 적용해봤음。현재는 주석처리 되어있음.
📌 참고 사항
🙏 코드 리뷰 전 확인 체크리스트
type :)Summary by CodeRabbit