Skip to content
This repository has been archived by the owner on Sep 15, 2023. It is now read-only.

Commit

Permalink
Merge pull request #19 from admin-ch/feature/timestamp-validation
Browse files Browse the repository at this point in the history 
explicit http status code for invalid timestamp (signaturePayload)
  • Loading branch information
@ubhaller
ubhaller authored Sep 17, 2021
2 parents 7b0dde8 + 984a117 commit f48a41e
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 55 deletions.
7 changes: 5 additions & 2 deletions .../src/main/java/ch/admin/bag/covidcertificate/backend/delivery/ws/config/WsBaseConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
45 changes: 19 additions & 26 deletions ...main/java/ch/admin/bag/covidcertificate/backend/delivery/ws/controller/AppController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -93,7 +93,7 @@ public ResponseEntity<Void> 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(
Expand Down Expand Up @@ -123,7 +123,8 @@ public ResponseEntity<Void> registerForDelivery(
public ResponseEntity<CovidCertDelivery> 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);
Expand All @@ -146,7 +147,8 @@ public ResponseEntity<CovidCertDelivery> getCovidCertDelivery(
public ResponseEntity<Void> 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);
Expand Down Expand Up @@ -191,18 +193,10 @@ public ResponseEntity<Void> 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<String> 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<String> codeAlreadyExists() {
return ResponseEntity.status(HttpStatus.CONFLICT).body("code already in use");
}

@ExceptionHandler({CodeNotFoundException.class})
Expand All @@ -217,23 +211,22 @@ public ResponseEntity<String> invalidSignature() {
return ResponseEntity.status(HttpStatus.FORBIDDEN).body("invalid signature");
}

@ExceptionHandler({InvalidTimestampException.class})
@ResponseStatus(HttpStatus.TOO_EARLY)
public ResponseEntity<String> 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<String> 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<String> invalidPublicKey() {
return ResponseEntity.status(HttpStatus.BAD_REQUEST).body("I | KEY");
}

@ExceptionHandler({InvalidActionException.class})
@ResponseStatus(HttpStatus.METHOD_NOT_ALLOWED)
public ResponseEntity<String> 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<String> invalidAction() {
return ResponseEntity.status(HttpStatus.METHOD_NOT_ALLOWED).body("invalid action");
}
}
22 changes: 14 additions & 8 deletions ...ch/admin/bag/covidcertificate/backend/delivery/ws/security/SignaturePayloadValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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);
}
}
}
28 changes: 28 additions & 0 deletions ...ag/covidcertificate/backend/delivery/ws/security/exception/InvalidTimestampException.java
View file
Original file line number Diff line number Diff line change
@@ -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
+ '}';
}
}
40 changes: 21 additions & 19 deletions .../bag/covidcertificate/backend/delivery/ws/controller/appcontroller/AppControllerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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)) {
Expand All @@ -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()));
}
}

Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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() {
Expand Down

0 comments on commit f48a41e

Please sign in to comment.