From ed9b957016483be31f2751c73dfb87f55aefff76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Dvo=C5=99=C3=A1k?= Date: Mon, 15 Jul 2024 15:21:28 +0200 Subject: [PATCH] Fix #279: Validate challenge format to be Base64 encoded bytes (#280) * Fix #279: Validate challenge format to be Base64 encoded bytes * Fix minor localization issues * Add HttpHeadersTest --------- Co-authored-by: Lubos Racansky --- .../ExceptionHandlingControllerAdvice.java | 6 +- .../InvalidChallengeHeaderException.java | 2 +- .../rest/http/HttpHeaders.java | 33 ++++++++- .../rest/http/HttpHeadersTest.java | 73 +++++++++++++++++++ 4 files changed, 107 insertions(+), 7 deletions(-) create mode 100644 src/test/java/com/wultra/app/mobileutilityserver/rest/http/HttpHeadersTest.java diff --git a/src/main/java/com/wultra/app/mobileutilityserver/rest/errorhandling/ExceptionHandlingControllerAdvice.java b/src/main/java/com/wultra/app/mobileutilityserver/rest/errorhandling/ExceptionHandlingControllerAdvice.java index bc91d971..7b705a1e 100644 --- a/src/main/java/com/wultra/app/mobileutilityserver/rest/errorhandling/ExceptionHandlingControllerAdvice.java +++ b/src/main/java/com/wultra/app/mobileutilityserver/rest/errorhandling/ExceptionHandlingControllerAdvice.java @@ -92,9 +92,9 @@ public class ExceptionHandlingControllerAdvice { @ExceptionHandler(InvalidChallengeHeaderException.class) @ResponseStatus(HttpStatus.FORBIDDEN) public @ResponseBody ErrorResponse handleInvalidChallengeHeaderException(InvalidChallengeHeaderException ex) { - final String code = "INSUFFICIENT_CHALLENGE"; - final String message = "Request does not contain sufficiently strong challenge header, 16B is required at least."; - logger.error("Request does not contain sufficiently strong challenge header, 16B is required at least: {}", ex.getMessage()); + final String code = "INVALID_CHALLENGE"; + final String message = "Request does not contain correct challenge header, a random Base64 encoded challenge with 16B - 32B raw length is required."; + logger.error("Request does not contain correct challenge header, a random Base64 encoded challenge with 16B - 32B raw length is required: {}", ex.getMessage()); logger.debug("Exception detail: ", ex); return new ErrorResponse(code, message); } diff --git a/src/main/java/com/wultra/app/mobileutilityserver/rest/errorhandling/InvalidChallengeHeaderException.java b/src/main/java/com/wultra/app/mobileutilityserver/rest/errorhandling/InvalidChallengeHeaderException.java index 59aca63f..8dce0151 100644 --- a/src/main/java/com/wultra/app/mobileutilityserver/rest/errorhandling/InvalidChallengeHeaderException.java +++ b/src/main/java/com/wultra/app/mobileutilityserver/rest/errorhandling/InvalidChallengeHeaderException.java @@ -19,7 +19,7 @@ package com.wultra.app.mobileutilityserver.rest.errorhandling; /** - * Exception thrown in the case challenge header is not present or is not sufficiently complex. + * Exception thrown in the case challenge header is not present or is not adequately complex. * * @author Petr Dvorak, petr@wultra.com */ diff --git a/src/main/java/com/wultra/app/mobileutilityserver/rest/http/HttpHeaders.java b/src/main/java/com/wultra/app/mobileutilityserver/rest/http/HttpHeaders.java index e7783df3..8fc5b330 100644 --- a/src/main/java/com/wultra/app/mobileutilityserver/rest/http/HttpHeaders.java +++ b/src/main/java/com/wultra/app/mobileutilityserver/rest/http/HttpHeaders.java @@ -17,22 +17,49 @@ */ package com.wultra.app.mobileutilityserver.rest.http; +import java.util.Base64; + +import org.apache.commons.lang3.StringUtils; + +import lombok.extern.slf4j.Slf4j; + /** * Class with constants for HTTP request / response headers. * * @author Petr Dvorak, petr@wultra.com */ +@Slf4j public class HttpHeaders { public static final int MIN_CHALLENGE_HEADER_LENGTH = 16; + public static final int MAX_CHALLENGE_HEADER_LENGTH = 32; public static final String REQUEST_CHALLENGE = "X-Cert-Pinning-Challenge"; public static final String RESPONSE_SIGNATURE = "X-Cert-Pinning-Signature"; + private HttpHeaders() { + throw new IllegalStateException("Should not be instantiated."); + } + public static boolean validChallengeHeader(String challengeHeader) { - return challengeHeader != null - && challengeHeader.length() >= HttpHeaders.MIN_CHALLENGE_HEADER_LENGTH - && !challengeHeader.isBlank(); + try { + if (StringUtils.isEmpty(challengeHeader)) { + logger.warn("Missing or blank challenge header: {}", challengeHeader); + return false; + } + final byte[] challengeBytes = Base64.getDecoder().decode(challengeHeader); + final int challengeLength = challengeBytes.length; + if (challengeLength >= MIN_CHALLENGE_HEADER_LENGTH && challengeLength <= MAX_CHALLENGE_HEADER_LENGTH) { + return true; + } else { + logger.warn("Invalid challenge size, must be between {} and {}, was: {}", MIN_CHALLENGE_HEADER_LENGTH, MAX_CHALLENGE_HEADER_LENGTH, challengeLength); + return false; + } + } catch (IllegalArgumentException ex) { + logger.warn("Invalid Base64 value received in the header: {}", challengeHeader); + logger.debug("Exception detail: {}", ex.getMessage(), ex); + return false; + } } } diff --git a/src/test/java/com/wultra/app/mobileutilityserver/rest/http/HttpHeadersTest.java b/src/test/java/com/wultra/app/mobileutilityserver/rest/http/HttpHeadersTest.java new file mode 100644 index 00000000..0555ab5b --- /dev/null +++ b/src/test/java/com/wultra/app/mobileutilityserver/rest/http/HttpHeadersTest.java @@ -0,0 +1,73 @@ +/* + * Wultra Mobile Utility Server + * Copyright (C) 2024 Wultra s.r.o. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +package com.wultra.app.mobileutilityserver.rest.http; + +import static org.junit.jupiter.api.Assertions.*; + +import java.util.Base64; + +import org.junit.jupiter.api.Test; + +/** + * Test for {@link HttpHeaders}. + * + * @author Lubos Racansky, lubos.racansky@wultra.com + */ +class HttpHeadersTest { + + @Test + void testValidChallengeHeader_success() { + final String input = Base64.getEncoder().encodeToString(new byte[20]); + + final boolean result = HttpHeaders.validChallengeHeader(input); + + assertTrue(result); + } + + @Test + void testValidChallengeHeader_empty() { + final boolean result = HttpHeaders.validChallengeHeader(""); + + assertFalse(result); + } + + @Test + void testValidChallengeHeader_tooShort() { + final String input = Base64.getEncoder().encodeToString(new byte[14]); + + final boolean result = HttpHeaders.validChallengeHeader(input); + + assertFalse(result); + } + + @Test + void testValidChallengeHeader_tooLong() { + final String input = Base64.getEncoder().encodeToString(new byte[33]); + + final boolean result = HttpHeaders.validChallengeHeader(input); + + assertFalse(result); + } + + @Test + void testValidChallengeHeader_notBase64() { + final boolean result = HttpHeaders.validChallengeHeader("&-+"); + + assertFalse(result); + } +}