From 8692fe35dbc544690362d0aca86f091bfc10578b Mon Sep 17 00:00:00 2001 From: sharma1210 Date: Fri, 8 Aug 2025 18:41:14 +0000 Subject: [PATCH] Provide specific reason for invalid SSL certificate (#2792) * Fix: Robustly parse certs and provide specific errors * Add test for expired certificate failure * fixing indentation * fixing indentation * Update SecurityActionTest.java * Update SecurityActionTest.java for correcting the testcase * Fix: Provide indentation fix * Fixing Deduplication in test --- .../console/settings/SecurityAction.java | 2 +- .../console/settings/SecurityActionTest.java | 86 ++++++++++++++++--- 2 files changed, 73 insertions(+), 15 deletions(-) diff --git a/core/src/main/java/google/registry/ui/server/console/settings/SecurityAction.java b/core/src/main/java/google/registry/ui/server/console/settings/SecurityAction.java index 80341d567..1bb001467 100644 --- a/core/src/main/java/google/registry/ui/server/console/settings/SecurityAction.java +++ b/core/src/main/java/google/registry/ui/server/console/settings/SecurityAction.java @@ -120,7 +120,7 @@ public class SecurityAction extends ConsoleApiAction { } } } catch (InsecureCertificateException e) { - setFailedResponse("Invalid certificate in parameter", SC_BAD_REQUEST); + setFailedResponse(e.getMessage(), SC_BAD_REQUEST); return; } diff --git a/core/src/test/java/google/registry/ui/server/console/settings/SecurityActionTest.java b/core/src/test/java/google/registry/ui/server/console/settings/SecurityActionTest.java index 350a8e944..0e328dfb8 100644 --- a/core/src/test/java/google/registry/ui/server/console/settings/SecurityActionTest.java +++ b/core/src/test/java/google/registry/ui/server/console/settings/SecurityActionTest.java @@ -20,6 +20,7 @@ import static google.registry.testing.DatabaseHelper.loadRegistrar; import static google.registry.testing.DatabaseHelper.loadSingleton; import static google.registry.testing.SqlHelper.saveRegistrar; import static google.registry.util.DateTimeUtils.START_OF_TIME; +import static jakarta.servlet.http.HttpServletResponse.SC_BAD_REQUEST; import static jakarta.servlet.http.HttpServletResponse.SC_OK; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.when; @@ -54,30 +55,52 @@ class SecurityActionTest extends ConsoleActionBaseTestCase { SAMPLE_CERT2); private Registrar testRegistrar; + private static final String VALIDITY_TOO_LONG_CERT_PEM = + "-----BEGIN CERTIFICATE-----\n" + + "MIIDejCCAv+gAwIBAgIQHNcSEt4VENkSgtozEEoQLzAKBggqhkjOPQQDAzB8MQsw\n" + + "CQYDVQQGEwJVUzEOMAwGA1UECAwFVGV4YXMxEDAOBgNVBAcMB0hvdXN0b24xGDAW\n" + + "BgNVBAoMD1NTTCBDb3Jwb3JhdGlvbjExMC8GA1UEAwwoU1NMLmNvbSBSb290IENl\n" + + "cnRpZmljYXRpb24gQXV0aG9yaXR5IEVDQzAeFw0xOTAzMDcxOTQyNDJaFw0zNDAz\n" + + "MDMxOTQyNDJaMG8xCzAJBgNVBAYTAlVTMQ4wDAYDVQQIDAVUZXhhczEQMA4GA1UE\n" + + "BwwHSG91c3RvbjERMA8GA1UECgwIU1NMIENvcnAxKzApBgNVBAMMIlNTTC5jb20g\n" + + "U1NMIEludGVybWVkaWF0ZSBDQSBFQ0MgUjIwdjAQBgcqhkjOPQIBBgUrgQQAIgNi\n" + + "AASEOWn30uEYKDLFu4sCjFQ1VupFaeMtQjqVWyWSA7+KFljnsVaFQ2hgs4cQk1f/\n" + + "RQ2INSwdVCYU0i5qsbom20rigUhDh9dM/r6bEZ75eFE899kSCI14xqThYVLPdLEl\n" + + "+dyjggFRMIIBTTASBgNVHRMBAf8ECDAGAQH/AgEAMB8GA1UdIwQYMBaAFILRhXMw\n" + + "5zUE044CkvvlpNHEIejNMHgGCCsGAQUFBwEBBGwwajBGBggrBgEFBQcwAoY6aHR0\n" + + "cDovL3d3dy5zc2wuY29tL3JlcG9zaXRvcnkvU1NMY29tLVJvb3RDQS1FQ0MtMzg0\n" + + "LVIxLmNydDAgBggrBgEFBQcwAYYUaHR0cDovL29jc3BzLnNzbC5jb20wEQYDVR0g\n" + + "BAowCDAGBgRVHSAAMB0GA1UdJQQWMBQGCCsGAQUFBwMCBggrBgEFBQcDATA7BgNV\n" + + "HR8ENDAyMDCgLqAshipodHRwOi8vY3Jscy5zc2wuY29tL3NzbC5jb20tZWNjLVJv\n" + + "b3RDQS5jcmwwHQYDVR0OBBYEFA10Zgpen+Is7NXCXSUEf3Uyuv99MA4GA1UdDwEB\n" + + "/wQEAwIBhjAKBggqhkjOPQQDAwNpADBmAjEAxYt6Ylk/N8Fch/3fgKYKwI5A011Q\n" + + "MKW0h3F9JW/NX/F7oYtWrxljheH8n2BrkDybAjEAlCxkLE0vQTYcFzrR24oogyw6\n" + + "VkgTm92+jiqJTO5SSA9QUa092S5cTKiHkH2cOM6m\n" + + "-----END CERTIFICATE-----"; + private AuthenticatedRegistrarAccessor registrarAccessor = AuthenticatedRegistrarAccessor.createForTesting( ImmutableSetMultimap.of("registrarId", AuthenticatedRegistrarAccessor.Role.ADMIN)); - private CertificateChecker certificateChecker = - new CertificateChecker( - ImmutableSortedMap.of(START_OF_TIME, 20825, DateTime.parse("2020-09-01T00:00:00Z"), 398), - 30, - 15, - 2048, - ImmutableSet.of("secp256r1", "secp384r1"), - clock); @BeforeEach void beforeEach() { testRegistrar = saveRegistrar("registrarId"); } - @Test void testSuccess_postRegistrarInfo() throws IOException { + CertificateChecker lenientChecker = + new CertificateChecker( + ImmutableSortedMap.of( + START_OF_TIME, 20825, DateTime.parse("2020-09-01T00:00:00Z"), 398), + 30, + 15, + 2048, + ImmutableSet.of("secp256r1", "secp384r1"), + clock); + clock.setTo(DateTime.parse("2020-11-01T00:00:00Z")); - SecurityAction action = - createAction( - testRegistrar.getRegistrarId()); + SecurityAction action = createAction(testRegistrar.getRegistrarId(), lenientChecker); action.run(); assertThat(((FakeResponse) consoleApiParams.response()).getStatus()).isEqualTo(SC_OK); Registrar r = loadRegistrar(testRegistrar.getRegistrarId()); @@ -90,9 +113,39 @@ class SecurityActionTest extends ConsoleActionBaseTestCase { assertThat(history.getDescription()).hasValue("registrarId|IP_CHANGE,PRIMARY_SSL_CERT_CHANGE"); } - private SecurityAction createAction(String registrarId) throws IOException { + @Test + void testFailure_validityPeriodTooLong_returnsSpecificError() throws IOException { + CertificateChecker strictChecker = + new CertificateChecker( + ImmutableSortedMap.of(START_OF_TIME, 398), + 30, + 15, + 2048, + ImmutableSet.of("secp256r1", "secp384r1"), + clock); + + clock.setTo(DateTime.parse("2025-01-01T00:00:00Z")); + String escapedCert = VALIDITY_TOO_LONG_CERT_PEM.replace("\n", "\\n"); + String jsonWithBadCert = + String.format( + "{\"registrarId\": \"registrarId\", \"clientCertificate\": \"%s\"}", escapedCert); + + SecurityAction action = + createAction(testRegistrar.getRegistrarId(), jsonWithBadCert, strictChecker); + action.run(); + + String expectedError = + "Certificate validity period is too long; it must be less than or equal to 398 days."; + FakeResponse response = (FakeResponse) consoleApiParams.response(); + assertThat(response.getStatus()).isEqualTo(SC_BAD_REQUEST); + assertThat(response.getPayload()).isEqualTo(expectedError); + } + + private SecurityAction createAction( + String registrarId, String jsonBody, CertificateChecker certificateChecker) + throws IOException { when(consoleApiParams.request().getMethod()).thenReturn(Action.Method.POST.toString()); - doReturn(new BufferedReader(new StringReader(jsonRegistrar1))) + doReturn(new BufferedReader(new StringReader(jsonBody))) .when(consoleApiParams.request()) .getReader(); Optional maybeRegistrar = @@ -101,4 +154,9 @@ class SecurityActionTest extends ConsoleActionBaseTestCase { return new SecurityAction( consoleApiParams, certificateChecker, registrarAccessor, registrarId, maybeRegistrar); } + + private SecurityAction createAction(String registrarId, CertificateChecker certificateChecker) + throws IOException { + return createAction(registrarId, jsonRegistrar1, certificateChecker); + } }