diff --git a/ch-covidcertificate-backend-delivery/ch-covidcertificate-backend-delivery-ws/src/main/java/ch/admin/bag/covidcertificate/backend/delivery/ws/config/WsBaseConfig.java b/ch-covidcertificate-backend-delivery/ch-covidcertificate-backend-delivery-ws/src/main/java/ch/admin/bag/covidcertificate/backend/delivery/ws/config/WsBaseConfig.java index 53f4021..e9514a3 100644 --- a/ch-covidcertificate-backend-delivery/ch-covidcertificate-backend-delivery-ws/src/main/java/ch/admin/bag/covidcertificate/backend/delivery/ws/config/WsBaseConfig.java +++ b/ch-covidcertificate-backend-delivery/ch-covidcertificate-backend-delivery-ws/src/main/java/ch/admin/bag/covidcertificate/backend/delivery/ws/config/WsBaseConfig.java @@ -28,6 +28,7 @@ import java.security.NoSuchAlgorithmException; import java.security.UnrecoverableKeyException; import java.security.cert.CertificateException; +import java.time.Duration; import java.util.Base64; import java.util.List; import java.util.Map; @@ -111,8 +112,10 @@ public LockProvider lockProvider(DataSource dataSource) { } @Bean - public SignaturePayloadValidator signatureValidator() { - return new SignaturePayloadValidator(); + public SignaturePayloadValidator signatureValidator( + @Value("${ws.signature.validation.timestamp.halfWindowDuration:PT60M}") + Duration halfWindow) { + return new SignaturePayloadValidator(halfWindow); } @Bean diff --git a/ch-covidcertificate-backend-delivery/ch-covidcertificate-backend-delivery-ws/src/main/java/ch/admin/bag/covidcertificate/backend/delivery/ws/controller/AppController.java b/ch-covidcertificate-backend-delivery/ch-covidcertificate-backend-delivery-ws/src/main/java/ch/admin/bag/covidcertificate/backend/delivery/ws/controller/AppController.java index a9bc49d..9efd9dc 100644 --- a/ch-covidcertificate-backend-delivery/ch-covidcertificate-backend-delivery-ws/src/main/java/ch/admin/bag/covidcertificate/backend/delivery/ws/controller/AppController.java +++ b/ch-covidcertificate-backend-delivery/ch-covidcertificate-backend-delivery-ws/src/main/java/ch/admin/bag/covidcertificate/backend/delivery/ws/controller/AppController.java @@ -27,11 +27,11 @@ import ch.admin.bag.covidcertificate.backend.delivery.ws.security.exception.InvalidPublicKeyException; import ch.admin.bag.covidcertificate.backend.delivery.ws.security.exception.InvalidSignatureException; import ch.admin.bag.covidcertificate.backend.delivery.ws.security.exception.InvalidSignaturePayloadException; +import ch.admin.bag.covidcertificate.backend.delivery.ws.security.exception.InvalidTimestampException; import ch.admin.bag.covidcertificate.backend.delivery.ws.utils.HashUtil; import ch.ubique.openapi.docannotations.Documentation; import java.security.NoSuchAlgorithmException; import java.util.List; -import javax.servlet.http.HttpServletRequest; import javax.validation.Valid; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -93,7 +93,7 @@ public ResponseEntity registerForDelivery( @Valid @RequestBody DeliveryRegistration registration) throws CodeAlreadyExistsException, InvalidSignatureException, InvalidActionException, InvalidSignaturePayloadException, InvalidPublicKeyException, - NoSuchAlgorithmException { + NoSuchAlgorithmException, InvalidTimestampException { String code = registration.getCode(); logger.info("registration for transfer code {} requested", code); validateSignature( @@ -123,7 +123,8 @@ public ResponseEntity registerForDelivery( public ResponseEntity getCovidCertDelivery( @Valid @RequestBody RequestDeliveryPayload payload) throws CodeNotFoundException, InvalidSignatureException, InvalidActionException, - InvalidSignaturePayloadException, InvalidPublicKeyException { + InvalidSignaturePayloadException, InvalidPublicKeyException, + InvalidTimestampException { String code = payload.getCode(); validateSignature(code, payload.getSignaturePayload(), payload.getSignature()); signaturePayloadValidator.validate(payload.getSignaturePayload(), Action.GET, code); @@ -146,7 +147,8 @@ public ResponseEntity getCovidCertDelivery( public ResponseEntity covidCertDeliveryComplete( @Valid @RequestBody RequestDeliveryPayload payload) throws InvalidPublicKeyException, InvalidSignatureException, CodeNotFoundException, - InvalidActionException, InvalidSignaturePayloadException { + InvalidActionException, InvalidSignaturePayloadException, + InvalidTimestampException { String code = payload.getCode(); validateSignature(code, payload.getSignaturePayload(), payload.getSignature()); signaturePayloadValidator.validate(payload.getSignaturePayload(), Action.DELETE, code); @@ -191,18 +193,10 @@ public ResponseEntity registerForPush(@Valid @RequestBody PushRegistration return ResponseEntity.ok().build(); } - private boolean isRegisterRequest(HttpServletRequest req) { - return req.getRequestURL().toString().endsWith("/app/delivery/v1/covidcert/register"); - } - @ExceptionHandler({CodeAlreadyExistsException.class}) @ResponseStatus(HttpStatus.CONFLICT) - public ResponseEntity codeAlreadyExists(HttpServletRequest req) { - if (isRegisterRequest(req)) { - return ResponseEntity.status(HttpStatus.CONFLICT).body("code already in use"); - } else { - return ResponseEntity.status(HttpStatus.FORBIDDEN).body("access denied"); - } + public ResponseEntity codeAlreadyExists() { + return ResponseEntity.status(HttpStatus.CONFLICT).body("code already in use"); } @ExceptionHandler({CodeNotFoundException.class}) @@ -217,23 +211,22 @@ public ResponseEntity invalidSignature() { return ResponseEntity.status(HttpStatus.FORBIDDEN).body("invalid signature"); } + @ExceptionHandler({InvalidTimestampException.class}) + @ResponseStatus(HttpStatus.TOO_EARLY) + public ResponseEntity invalidTimestamp(InvalidTimestampException e) { + logger.error("received invalid timestamp. {}", e.toString()); + return ResponseEntity.status(HttpStatus.TOO_EARLY).body("I | TIME"); + } + @ExceptionHandler({InvalidPublicKeyException.class}) @ResponseStatus(HttpStatus.BAD_REQUEST) - public ResponseEntity invalidPublicKey(HttpServletRequest req) { - if (isRegisterRequest(req)) { - return ResponseEntity.status(HttpStatus.BAD_REQUEST).body("invalid public key"); - } else { - return ResponseEntity.status(HttpStatus.FORBIDDEN).body("access denied"); - } + public ResponseEntity invalidPublicKey() { + return ResponseEntity.status(HttpStatus.BAD_REQUEST).body("I | KEY"); } @ExceptionHandler({InvalidActionException.class}) @ResponseStatus(HttpStatus.METHOD_NOT_ALLOWED) - public ResponseEntity invalidAction(HttpServletRequest req) { - if (isRegisterRequest(req)) { - return ResponseEntity.status(HttpStatus.METHOD_NOT_ALLOWED).body("invalid action"); - } else { - return ResponseEntity.status(HttpStatus.FORBIDDEN).body("access denied"); - } + public ResponseEntity invalidAction() { + return ResponseEntity.status(HttpStatus.METHOD_NOT_ALLOWED).body("invalid action"); } } diff --git a/ch-covidcertificate-backend-delivery/ch-covidcertificate-backend-delivery-ws/src/main/java/ch/admin/bag/covidcertificate/backend/delivery/ws/security/SignaturePayloadValidator.java b/ch-covidcertificate-backend-delivery/ch-covidcertificate-backend-delivery-ws/src/main/java/ch/admin/bag/covidcertificate/backend/delivery/ws/security/SignaturePayloadValidator.java index 9af07b7..2b3f67c 100644 --- a/ch-covidcertificate-backend-delivery/ch-covidcertificate-backend-delivery-ws/src/main/java/ch/admin/bag/covidcertificate/backend/delivery/ws/security/SignaturePayloadValidator.java +++ b/ch-covidcertificate-backend-delivery/ch-covidcertificate-backend-delivery-ws/src/main/java/ch/admin/bag/covidcertificate/backend/delivery/ws/security/SignaturePayloadValidator.java @@ -3,22 +3,27 @@ import ch.admin.bag.covidcertificate.backend.delivery.model.util.CodeHelper; import ch.admin.bag.covidcertificate.backend.delivery.ws.security.exception.InvalidActionException; import ch.admin.bag.covidcertificate.backend.delivery.ws.security.exception.InvalidSignaturePayloadException; +import ch.admin.bag.covidcertificate.backend.delivery.ws.security.exception.InvalidTimestampException; +import java.time.Duration; import java.time.Instant; -import java.time.temporal.ChronoUnit; public class SignaturePayloadValidator { + private final Duration halfWindow; + + public SignaturePayloadValidator(Duration halfWindow) { + this.halfWindow = halfWindow; + } public void validate(String signaturePayload, Action expectedAction, String code) - throws InvalidSignaturePayloadException, InvalidActionException { + throws InvalidSignaturePayloadException, InvalidActionException, + InvalidTimestampException { String[] signaturePayloadSplit = signaturePayload.split(":"); if (signaturePayloadSplit.length != 3) { throw new InvalidSignaturePayloadException(); } - if (!isCurrentTimestamp(Long.valueOf(signaturePayloadSplit[2]))) { - throw new InvalidSignaturePayloadException(); - } + validateTimestamp(Long.valueOf(signaturePayloadSplit[2])); if (!code.equals(CodeHelper.getSanitizedCode(signaturePayloadSplit[1]))) { throw new InvalidSignaturePayloadException(); @@ -29,10 +34,11 @@ public void validate(String signaturePayload, Action expectedAction, String code } } - private boolean isCurrentTimestamp(Long epochMilli) { + private void validateTimestamp(Long epochMilli) throws InvalidTimestampException { Instant now = Instant.now(); Instant timestamp = Instant.ofEpochMilli(epochMilli); - return timestamp.isAfter(now.minus(5, ChronoUnit.MINUTES)) - && timestamp.isBefore(now.plus(5, ChronoUnit.MINUTES)); + if (timestamp.isBefore(now.minus(halfWindow)) || timestamp.isAfter(now.plus(halfWindow))) { + throw new InvalidTimestampException(now, halfWindow, timestamp); + } } } diff --git a/ch-covidcertificate-backend-delivery/ch-covidcertificate-backend-delivery-ws/src/main/java/ch/admin/bag/covidcertificate/backend/delivery/ws/security/exception/InvalidTimestampException.java b/ch-covidcertificate-backend-delivery/ch-covidcertificate-backend-delivery-ws/src/main/java/ch/admin/bag/covidcertificate/backend/delivery/ws/security/exception/InvalidTimestampException.java new file mode 100644 index 0000000..22f07e5 --- /dev/null +++ b/ch-covidcertificate-backend-delivery/ch-covidcertificate-backend-delivery-ws/src/main/java/ch/admin/bag/covidcertificate/backend/delivery/ws/security/exception/InvalidTimestampException.java @@ -0,0 +1,28 @@ +package ch.admin.bag.covidcertificate.backend.delivery.ws.security.exception; + +import java.time.Duration; +import java.time.Instant; + +public class InvalidTimestampException extends Exception { + private final Instant acceptedFrom; + private final Instant acceptedTo; + private final Instant actual; + + public InvalidTimestampException(Instant now, Duration halfWindow, Instant timestamp) { + this.acceptedFrom = now.minus(halfWindow); + this.acceptedTo = now.plus(halfWindow); + this.actual = timestamp; + } + + @Override + public String toString() { + return "InvalidTimestampException{" + + "acceptedFrom=" + + acceptedFrom + + ", acceptedTo=" + + acceptedTo + + ", actual=" + + actual + + '}'; + } +} diff --git a/ch-covidcertificate-backend-delivery/ch-covidcertificate-backend-delivery-ws/src/test/java/ch/admin/bag/covidcertificate/backend/delivery/ws/controller/appcontroller/AppControllerTest.java b/ch-covidcertificate-backend-delivery/ch-covidcertificate-backend-delivery-ws/src/test/java/ch/admin/bag/covidcertificate/backend/delivery/ws/controller/appcontroller/AppControllerTest.java index 22e4e35..f101cbc 100644 --- a/ch-covidcertificate-backend-delivery/ch-covidcertificate-backend-delivery-ws/src/test/java/ch/admin/bag/covidcertificate/backend/delivery/ws/controller/appcontroller/AppControllerTest.java +++ b/ch-covidcertificate-backend-delivery/ch-covidcertificate-backend-delivery-ws/src/test/java/ch/admin/bag/covidcertificate/backend/delivery/ws/controller/appcontroller/AppControllerTest.java @@ -28,8 +28,8 @@ import java.security.KeyPair; import java.security.NoSuchAlgorithmException; import java.sql.SQLException; +import java.time.Duration; import java.time.Instant; -import java.time.temporal.ChronoUnit; import java.util.Base64; import java.util.List; import org.junit.jupiter.api.BeforeAll; @@ -46,6 +46,8 @@ public abstract class AppControllerTest extends BaseControllerTest { private static final String COMPLETE_ENDPOINT = BASE_URL + "/covidcert/complete"; private static final String PUSH_REGISTER_ENDPOINT = BASE_URL + "/push/register"; // TODO + private static final Duration halfTimestampValidityWindow = Duration.ofHours(1); + @BeforeAll public void setup() throws NoSuchAlgorithmException, SQLException { super.setup(); @@ -210,10 +212,7 @@ public void deliveryFlowTest() throws Exception { // bad complete transfer request completeTransfer( getRequestDeliveryPayload( - Action.DELETE, - code, - Instant.now().minus(6, ChronoUnit.MINUTES), - this.algorithm)); + Action.DELETE, code, getInvalidTimestamp(), this.algorithm)); // verify covid cert is still in db assertEquals(1, deliveryDataService.findCovidCerts(code).size()); @@ -266,23 +265,22 @@ private MockHttpServletResponse getCovidCert(String code) throws Exception { } @Test - public void forbiddenTest() throws Exception { + public void invalidSignatureOrSignaturePayloadTest() throws Exception { final String code = CodeGenerator.generateCode(); // register registerForDelivery( getDeliveryRegistration(Action.REGISTER, code, Instant.now(), this.algorithm)); - // forbidden tests for get - internalForbiddenTest(code, Action.GET, GET_COVID_CERT_ENDPOINT, HttpStatus.FORBIDDEN); + // tests for get + internalInvalidSignatureOrSignaturePayloadTest(code, Action.GET, GET_COVID_CERT_ENDPOINT); - // forbidden tests for delete (since best effort, it always returns OK) - internalForbiddenTest(code, Action.DELETE, COMPLETE_ENDPOINT, HttpStatus.FORBIDDEN); + // tests for delete + internalInvalidSignatureOrSignaturePayloadTest(code, Action.DELETE, COMPLETE_ENDPOINT); } - private void internalForbiddenTest( - String code, Action expectedAction, String endpoint, HttpStatus expectedStatusCode) - throws Exception { + private void internalInvalidSignatureOrSignaturePayloadTest( + String code, Action expectedAction, String endpoint) throws Exception { // invalid signature payload (action) for (Action action : Action.values()) { if (!expectedAction.equals(action)) { @@ -297,7 +295,7 @@ private void internalForbiddenTest( this.algorithm))) .contentType(MediaType.APPLICATION_JSON) .accept(acceptMediaType)) - .andExpect(status().is(expectedStatusCode.value())); + .andExpect(status().is(HttpStatus.METHOD_NOT_ALLOWED.value())); } } @@ -315,7 +313,7 @@ private void internalForbiddenTest( .content(asJsonString(payload)) .contentType(MediaType.APPLICATION_JSON) .accept(acceptMediaType)) - .andExpect(status().is(expectedStatusCode.value())); + .andExpect(status().is(HttpStatus.FORBIDDEN.value())); // invalid signature payload (timestamp) mockMvc.perform( @@ -325,11 +323,11 @@ private void internalForbiddenTest( getRequestDeliveryPayload( expectedAction, code, - Instant.now().minus(6, ChronoUnit.MINUTES), + getInvalidTimestamp(), this.algorithm))) .contentType(MediaType.APPLICATION_JSON) .accept(acceptMediaType)) - .andExpect(status().is(expectedStatusCode.value())); + .andExpect(status().is(HttpStatus.TOO_EARLY.value())); // invalid signature (encrypted with wrong private key) mockMvc.perform( @@ -339,11 +337,15 @@ private void internalForbiddenTest( getRequestDeliveryPayload( expectedAction, code, - Instant.now().minus(6, ChronoUnit.MINUTES), + Instant.now(), getWrongAlgorithm()))) .contentType(MediaType.APPLICATION_JSON) .accept(acceptMediaType)) - .andExpect(status().is(expectedStatusCode.value())); + .andExpect(status().is(HttpStatus.FORBIDDEN.value())); + } + + private Instant getInvalidTimestamp() { + return Instant.now().minus(halfTimestampValidityWindow.multipliedBy(3)); } private Algorithm getWrongAlgorithm() {