Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
208 changes: 0 additions & 208 deletions MiniKms/logs/controller.log

This file was deleted.

4 changes: 0 additions & 4 deletions MiniKms/logs/entity.log

This file was deleted.

6 changes: 6 additions & 0 deletions MiniKms/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,12 @@
<artifactId>mapstruct</artifactId>
<version>1.6.0</version>
</dependency>
<dependency>
<groupId>net.logstash.logback</groupId>
<artifactId>logstash-logback-encoder</artifactId>
<version>8.1</version>
</dependency>

</dependencies>

<build>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
.requestMatchers(HttpMethod.OPTIONS, "/**").permitAll()
.requestMatchers("/api/v1/auth/**").permitAll()
.requestMatchers("/api/v1/test/**").permitAll()
.requestMatchers("/api/v1/crypto/**").permitAll()
.requestMatchers("/api/v1/signatures/**").permitAll()
.requestMatchers(HttpMethod.GET, "/api/v1/keys/**").authenticated() // Allow all roles to GET
.requestMatchers("/api/v1/keys/**").hasRole("MANAGER")
.anyRequest().authenticated()
Expand Down
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());

Check warning

Code scanning / CodeQL

Information exposure through an error message Medium

Error information
can be exposed to an external user.

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.java for both the sign and verify endpoints (lines 32 and 50). No new dependencies are required, as standard logging is available in Java (e.g., using java.util.logging.Logger), but we may need to add a logger field to the class. Specifically:

  • Add a private static final Logger to the controller.
  • Log the exception inside the catch blocks.
  • Change the response to use a generic error message, e.g., "Invalid request".
Suggested changeset 1
MiniKms/src/main/java/ftn/security/minikms/controller/SignatureController.java

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/MiniKms/src/main/java/ftn/security/minikms/controller/SignatureController.java b/MiniKms/src/main/java/ftn/security/minikms/controller/SignatureController.java
--- a/MiniKms/src/main/java/ftn/security/minikms/controller/SignatureController.java
+++ b/MiniKms/src/main/java/ftn/security/minikms/controller/SignatureController.java
@@ -10,6 +10,7 @@
 import org.springframework.web.bind.annotation.*;
 
 import java.security.GeneralSecurityException;
+import java.util.logging.Logger;
 import java.security.Principal;
 import java.util.Base64;
 import java.util.UUID;
@@ -18,6 +19,8 @@
 @RequestMapping("/api/v1/signatures")
 public class SignatureController {
 
+    private static final Logger LOGGER = Logger.getLogger(SignatureController.class.getName());
+
     @Autowired
     private SignatureService signatureService;
 
@@ -29,7 +32,8 @@
             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());
+            LOGGER.warning("Sign operation failed: " + e.getMessage());
+            return ResponseEntity.status(HttpStatus.BAD_REQUEST).body("Invalid request");
         }
     }
 
@@ -47,7 +51,8 @@
             );
             return ResponseEntity.ok(valid ? "VALID" : "INVALID");
         } catch (GeneralSecurityException | IllegalArgumentException e) {
-            return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(e.getMessage());
+            LOGGER.warning("Verify operation failed: " + e.getMessage());
+            return ResponseEntity.status(HttpStatus.BAD_REQUEST).body("Invalid request");
         }
     }
 }
EOF
@@ -10,6 +10,7 @@
import org.springframework.web.bind.annotation.*;

import java.security.GeneralSecurityException;
import java.util.logging.Logger;
import java.security.Principal;
import java.util.Base64;
import java.util.UUID;
@@ -18,6 +19,8 @@
@RequestMapping("/api/v1/signatures")
public class SignatureController {

private static final Logger LOGGER = Logger.getLogger(SignatureController.class.getName());

@Autowired
private SignatureService signatureService;

@@ -29,7 +32,8 @@
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());
LOGGER.warning("Sign operation failed: " + e.getMessage());
return ResponseEntity.status(HttpStatus.BAD_REQUEST).body("Invalid request");
}
}

@@ -47,7 +51,8 @@
);
return ResponseEntity.ok(valid ? "VALID" : "INVALID");
} catch (GeneralSecurityException | IllegalArgumentException e) {
return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(e.getMessage());
LOGGER.warning("Verify operation failed: " + e.getMessage());
return ResponseEntity.status(HttpStatus.BAD_REQUEST).body("Invalid request");
}
}
}
Copilot is powered by AI and may make mistakes. Always verify output.
}
}

@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 warning

Code scanning / CodeQL

Information exposure through an error message Medium

Error information
can be exposed to an external user.

Copilot Autofix

AI 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 SignatureController.java, lines 32 and 50 need updating so that instead of returning e.getMessage() in the response body, a generic string is returned, and the details of the exception are logged using a standard logger. To accomplish this, we should introduce a logger in the controller (e.g., with org.slf4j.Logger and LoggerFactory).

Required steps:

  • Import or define a logger in the controller class.
  • Replace all usages of e.getMessage() in response bodies with a generic message.
  • Log the exception details to the server log using the logger.
Suggested changeset 1
MiniKms/src/main/java/ftn/security/minikms/controller/SignatureController.java

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/MiniKms/src/main/java/ftn/security/minikms/controller/SignatureController.java b/MiniKms/src/main/java/ftn/security/minikms/controller/SignatureController.java
--- a/MiniKms/src/main/java/ftn/security/minikms/controller/SignatureController.java
+++ b/MiniKms/src/main/java/ftn/security/minikms/controller/SignatureController.java
@@ -9,6 +9,8 @@
 import org.springframework.transaction.annotation.Transactional;
 import org.springframework.web.bind.annotation.*;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import java.security.GeneralSecurityException;
 import java.security.Principal;
 import java.util.Base64;
@@ -18,6 +20,8 @@
 @RequestMapping("/api/v1/signatures")
 public class SignatureController {
 
+    private static final Logger logger = LoggerFactory.getLogger(SignatureController.class);
+
     @Autowired
     private SignatureService signatureService;
 
@@ -29,7 +33,8 @@
             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());
+            logger.error("Error during signing", e);
+            return ResponseEntity.status(HttpStatus.BAD_REQUEST).body("Invalid input");
         }
     }
 
@@ -47,7 +52,8 @@
             );
             return ResponseEntity.ok(valid ? "VALID" : "INVALID");
         } catch (GeneralSecurityException | IllegalArgumentException e) {
-            return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(e.getMessage());
+            logger.error("Error during verification", e);
+            return ResponseEntity.status(HttpStatus.BAD_REQUEST).body("Invalid input");
         }
     }
 }
EOF
@@ -9,6 +9,8 @@
import org.springframework.transaction.annotation.Transactional;
import org.springframework.web.bind.annotation.*;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.security.GeneralSecurityException;
import java.security.Principal;
import java.util.Base64;
@@ -18,6 +20,8 @@
@RequestMapping("/api/v1/signatures")
public class SignatureController {

private static final Logger logger = LoggerFactory.getLogger(SignatureController.class);

@Autowired
private SignatureService signatureService;

@@ -29,7 +33,8 @@
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());
logger.error("Error during signing", e);
return ResponseEntity.status(HttpStatus.BAD_REQUEST).body("Invalid input");
}
}

@@ -47,7 +52,8 @@
);
return ResponseEntity.ok(valid ? "VALID" : "INVALID");
} catch (GeneralSecurityException | IllegalArgumentException e) {
return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(e.getMessage());
logger.error("Error during verification", e);
return ResponseEntity.status(HttpStatus.BAD_REQUEST).body("Invalid input");
}
}
}
Copilot is powered by AI and may make mistakes. Always verify output.
}
}
}
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
Expand Up @@ -2,37 +2,41 @@

import jakarta.persistence.*;
import lombok.extern.slf4j.Slf4j;
import org.springframework.beans.factory.annotation.Value;

import java.util.Map;

@Slf4j
public class EntityLogger {

@PrePersist
public void prePersist(Object entity) {
log.info("Creating: {}", entity);
@Value("${logging.entity.enabled:true}")
private boolean loggingEnabled;

private void logEntity(String action, String phase, Object entity) {
if (!loggingEnabled) return;

log.info("{}", Map.of(
"action", action,
"phase", phase,
"entity", entity.getClass().getSimpleName()
));
}

@PrePersist
public void prePersist(Object entity) { logEntity("create", "pre", entity); }

@PostPersist
public void postPersist(Object entity) {
log.info("Created: {}", entity);
}
public void postPersist(Object entity) { logEntity("create", "post", entity); }

@PreUpdate
public void preUpdate(Object entity) {
log.info("Updating: {}", entity);
}
public void preUpdate(Object entity) { logEntity("update", "pre", entity); }

@PostUpdate
public void postUpdate(Object entity) {
log.info("Updated: {}", entity);
}
public void postUpdate(Object entity) { logEntity("update", "post", entity); }

@PreRemove
public void preRemove(Object entity) {
log.info("Deleting: {}", entity);
}
public void preRemove(Object entity) { logEntity("delete", "pre", entity); }

@PostRemove
public void postRemove(Object entity) {
log.info("Deleted: {}", entity);
}
public void postRemove(Object entity) { logEntity("delete", "post", entity); }
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,57 +5,56 @@
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import lombok.extern.slf4j.Slf4j;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Component;
import org.springframework.web.filter.OncePerRequestFilter;
import org.springframework.web.util.ContentCachingRequestWrapper;
import org.springframework.web.util.ContentCachingResponseWrapper;

import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.Map;
import java.util.UUID;

@Slf4j
@Component
public class RequestResponseLoggingFilter extends OncePerRequestFilter {

@Value("${logging.controller.enabled:true}")
private boolean loggingEnabled;

@Override
protected void doFilterInternal(HttpServletRequest request,
HttpServletResponse response,
FilterChain filterChain) throws ServletException, IOException {

ContentCachingRequestWrapper requestWrapper = new ContentCachingRequestWrapper(request);
ContentCachingResponseWrapper responseWrapper = new ContentCachingResponseWrapper(response);
if (!loggingEnabled) {
filterChain.doFilter(request, response);
return;
}

String requestId = request.getHeader("X-Request-ID");
if (requestId == null || requestId.isBlank()) {
requestId = UUID.randomUUID().toString();
}
response.setHeader("X-Request-ID", requestId);

responseWrapper.setHeader("X-Request-ID", requestId);
String username = request.getUserPrincipal() != null
? request.getUserPrincipal().getName()
: "anonymous";

long start = System.currentTimeMillis();
try {
filterChain.doFilter(requestWrapper, responseWrapper);
filterChain.doFilter(request, response);
} finally {
long duration = System.currentTimeMillis() - start;

String requestBody = new String(requestWrapper.getContentAsByteArray(), StandardCharsets.UTF_8);
log.info("[{}] REQUEST {} {} | Body={}",
requestId,
request.getMethod(),
request.getRequestURI(),
requestBody);

String responseBody = new String(responseWrapper.getContentAsByteArray(), StandardCharsets.UTF_8);
log.info("[{}] RESPONSE {} {} | Status={} | Duration={}ms | Body={}",
requestId,
request.getMethod(),
request.getRequestURI(),
responseWrapper.getStatus(),
duration,
responseBody);

responseWrapper.copyBodyToResponse();
log.info("{}", Map.of(
"message", "HTTP request completed",
"requestId", requestId,
"username", username,
"method", request.getMethod(),
"uri", request.getRequestURI(),
"status", response.getStatus(),
"durationMs", duration
));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import ftn.security.minikms.entity.KeyMetadata;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Query;
import org.springframework.data.repository.query.Param;

import java.util.UUID;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,14 @@
import ftn.security.minikms.entity.KeyMaterial;
import ftn.security.minikms.repository.KeyMetadataRepository;
import ftn.security.minikms.repository.WrappedKeyRepository;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import javax.crypto.BadPaddingException;
import javax.crypto.IllegalBlockSizeException;
import javax.crypto.NoSuchPaddingException;
import java.security.InvalidAlgorithmParameterException;
import java.security.InvalidKeyException;
import java.security.InvalidParameterException;
import java.security.NoSuchAlgorithmException;
import java.security.*;
import java.security.spec.InvalidKeySpecException;
import java.util.UUID;

Expand All @@ -25,6 +23,9 @@ public class KeyComputeService {
private final RSAService rsaService;
private final HMACService hmacService;

@Autowired
private RootKeyManager rootKeyManager;

public KeyComputeService(KeyMetadataRepository metadataRepository, WrappedKeyRepository keyRepository) {
this.metadataRepository = metadataRepository;
this.keyRepository = keyRepository;
Expand Down Expand Up @@ -76,4 +77,32 @@ public KeyMaterial getKey(UUID keyId, Integer version) {

return wrappedKey.getWrappedMaterial();
}

public KeyMaterial getKeySig(UUID keyId, Integer version) {
var metadata = metadataRepository.findById(keyId)
.orElseThrow(() -> new InvalidParameterException("Key with given id does not exist"));

if (version == null) version = metadata.getPrimaryVersion();
var wrappedKey = keyRepository.findByMetadataIdAndVersion(keyId, version)
.orElseThrow(() -> new InvalidParameterException("Key with given id and version does not exist"));

var wrappedMaterial = wrappedKey.getWrappedMaterial();

try {
// Unwrap the private key
byte[] unwrappedPrivate = rootKeyManager.unwrap(
wrappedMaterial.getKey(), keyId, version
);

// For asymmetric keys: also store public key
KeyMaterial material = new KeyMaterial();
material.setKey(unwrappedPrivate);
material.setPublicKey(wrappedMaterial.getPublicKey());

return material;
} catch (GeneralSecurityException e) {
throw new RuntimeException("Failed to unwrap key material", e);
}
}

}
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);
}
}
10 changes: 7 additions & 3 deletions MiniKms/src/main/resources/application.properties
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,13 @@ server.ssl.key-alias=minikms

# Debugging
#spring.jpa.show-sql=true
#logging.level.org.springframework.security=DEBUG
#logging.level.io.jsonwebtoken=DEBUG
logging.level.org.springframework.security=DEBUG
logging.level.io.jsonwebtoken=DEBUG

# 1 hour
jwt.expiration=3600000
jwt.secret=Dubt4z4Lba9fc82KES/2uRcxOR9LcTTwxh7UuxE4f9Q=
jwt.secret=Dubt4z4Lba9fc82KES/2uRcxOR9LcTTwxh7UuxE4f9Q=

# Enable/disable logging
logging.controller.enabled=true
logging.entity.enabled=true
Loading
Loading