-
Notifications
You must be signed in to change notification settings - Fork 0
Key Sign/Verify and logging system restructuring #12
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
Changes from all commits
47965a1
6425202
2c0d603
7d788d0
17d2e45
cf66285
d42d47f
7de80f3
9dd5f01
b00d275
f58af3e
8c8da2a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
This file was deleted.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,53 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| package ftn.security.minikms.controller; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import ftn.security.minikms.dto.SignRequestDTO; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import ftn.security.minikms.dto.VerifyRequestDTO; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import ftn.security.minikms.service.SignatureService; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.springframework.beans.factory.annotation.Autowired; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.springframework.http.HttpStatus; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.springframework.http.ResponseEntity; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.springframework.transaction.annotation.Transactional; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.springframework.web.bind.annotation.*; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.security.GeneralSecurityException; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.security.Principal; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.Base64; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.UUID; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @RestController | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @RequestMapping("/api/v1/signatures") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public class SignatureController { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @Autowired | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private SignatureService signatureService; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @PostMapping("/sign") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @Transactional(readOnly = true) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public ResponseEntity<String> sign(@RequestParam UUID keyId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @RequestBody SignRequestDTO request) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| byte[] signature = signatureService.sign(keyId, request.getMessage(), request.getVersion()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ResponseEntity.ok(Base64.getEncoder().encodeToString(signature)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (GeneralSecurityException | IllegalArgumentException e) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(e.getMessage()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @PostMapping("/verify") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @Transactional(readOnly = true) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public ResponseEntity<String> verify(@RequestParam UUID keyId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @RequestParam(required = false) Integer version, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @RequestBody VerifyRequestDTO req) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| boolean valid = signatureService.verify( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| keyId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| req.getMessage(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Base64.getDecoder().decode(req.getSignature()), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| version | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ResponseEntity.ok(valid ? "VALID" : "INVALID"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (GeneralSecurityException | IllegalArgumentException e) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(e.getMessage()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Check warningCode scanning / CodeQL Information exposure through an error message Medium Error information Error loading related location Loading
Copilot AutofixAI 4 months ago To fix the problem, the code should not directly send exception messages back to the API caller. Instead, it should respond with a generic, non-informative message, such as "Invalid input" or "An error has occurred." The actual exception message should be logged on the server for debugging purposes. Specifically, in Required steps:
Suggested changeset
1
MiniKms/src/main/java/ftn/security/minikms/controller/SignatureController.java
Copilot is powered by AI and may make mistakes. Always verify output.
Refresh and try again.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| package ftn.security.minikms.dto; | ||
|
|
||
| import lombok.Data; | ||
|
|
||
| @Data | ||
| public class SignRequestDTO { | ||
| private String message; | ||
| private Integer version; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| package ftn.security.minikms.dto; | ||
|
|
||
| import lombok.Data; | ||
|
|
||
| @Data | ||
| public class VerifyRequestDTO { | ||
| private String message; | ||
| private String signature; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| package ftn.security.minikms.service; | ||
|
|
||
| import ftn.security.minikms.entity.KeyMaterial; | ||
| import ftn.security.minikms.entity.KeyMetadata; | ||
| import ftn.security.minikms.entity.WrappedKey; | ||
| import ftn.security.minikms.enumeration.KeyType; | ||
| import org.springframework.beans.factory.annotation.Autowired; | ||
| import org.springframework.stereotype.Service; | ||
|
|
||
| import java.nio.charset.StandardCharsets; | ||
| import java.security.*; | ||
| import java.security.spec.PKCS8EncodedKeySpec; | ||
| import java.security.spec.X509EncodedKeySpec; | ||
| import java.util.Arrays; | ||
| import java.util.UUID; | ||
|
|
||
| @Service | ||
| public class SignatureService { | ||
|
|
||
| @Autowired | ||
| private KeyComputeService keyComputeService; | ||
|
|
||
| public SignatureService() { | ||
| } | ||
|
|
||
| public byte[] sign(UUID keyId, String message, Integer version) throws GeneralSecurityException { | ||
| KeyMaterial keyMaterial = keyComputeService.getKeySig(keyId, version); | ||
|
|
||
| if (keyMaterial.getPublicKey() == null) { | ||
| throw new IllegalArgumentException("Key is not asymmetric and cannot be used for signing"); | ||
| } | ||
|
|
||
|
|
||
| PrivateKey privateKey = KeyFactory.getInstance("RSA") | ||
| .generatePrivate(new PKCS8EncodedKeySpec(keyMaterial.getKey())); | ||
|
|
||
| Signature signature = Signature.getInstance("SHA256withRSA"); | ||
| signature.initSign(privateKey); | ||
| signature.update(message.getBytes(java.nio.charset.StandardCharsets.UTF_8)); | ||
|
|
||
| return signature.sign(); | ||
| } | ||
|
|
||
| public boolean verify(UUID keyId, String message, byte[] signatureBytes, Integer version) throws GeneralSecurityException { | ||
| KeyMaterial keyMaterial = keyComputeService.getKeySig(keyId, version); | ||
|
|
||
| if (keyMaterial.getPublicKey() == null) { | ||
| throw new IllegalArgumentException("Key is not asymmetric and cannot be used for verification"); | ||
| } | ||
|
|
||
| PublicKey publicKey = KeyFactory.getInstance("RSA") | ||
| .generatePublic(new X509EncodedKeySpec(keyMaterial.getPublicKey())); | ||
|
|
||
| Signature signature = Signature.getInstance("SHA256withRSA"); | ||
| signature.initVerify(publicKey); | ||
| signature.update(message.getBytes(StandardCharsets.UTF_8)); | ||
|
|
||
| return signature.verify(signatureBytes); | ||
| } | ||
| } |
Check warning
Code scanning / CodeQL
Information exposure through an error message Medium
Copilot Autofix
AI 4 months ago
To fix the problem, modify the controller methods so that, instead of returning the exception's message in the HTTP response, they return a generic error message (e.g., "Invalid input" or "Bad request"). Server-side logging for the exception can be added so that error details are available for diagnostics, but not exposed to the client. The changes should be made in
MiniKms/src/main/java/ftn/security/minikms/controller/SignatureController.javafor both thesignandverifyendpoints (lines 32 and 50). No new dependencies are required, as standard logging is available in Java (e.g., usingjava.util.logging.Logger), but we may need to add a logger field to the class. Specifically:private static final Loggerto the controller.catchblocks."Invalid request".