From fd09884856c8619a0a6291a7c56dab1558146d4b Mon Sep 17 00:00:00 2001
From: Sebastian-Ion TINCU <Sebastian-Ion.TINCU@ext.ec.europa.eu>
Date: Fri, 15 Mar 2024 10:37:53 +0100
Subject: [PATCH] EDELIVERY-11391 SMP - Trustore - System admin is able to
import duplicated certificates
Verify if certificates being uploaded already exist in the current keystore or truststore.
---
.../alert-message/alert-message.service.ts | 4 +++
.../app/common/model/keystore-result.model.ts | 2 ++
.../keystore-import-dialog.component.spec.ts | 25 ---------------
.../keystore-import-dialog.component.ts | 15 +++++----
.../smp/data/ui/KeystoreImportResult.java | 9 ++++++
.../smp/services/ui/UIKeystoreService.java | 31 ++++++++++++++++++-
.../smp/services/ui/UITruststoreService.java | 11 +++++--
.../services/ui/UIKeystoreServiceTest.java | 16 +++++++++-
.../services/ui/UITruststoreServiceTest.java | 23 +++++++++++++-
.../ui/internal/KeystoreAdminController.java | 17 ++++++++++
.../internal/KeystoreAdminControllerIT.java | 17 +++++++---
11 files changed, 127 insertions(+), 43 deletions(-)
delete mode 100644 smp-angular/src/app/system-settings/admin-keystore/keystore-import-dialog/keystore-import-dialog.component.spec.ts
diff --git a/smp-angular/src/app/common/alert-message/alert-message.service.ts b/smp-angular/src/app/common/alert-message/alert-message.service.ts
index 09e66f237..95f7ddbb8 100644
--- a/smp-angular/src/app/common/alert-message/alert-message.service.ts
+++ b/smp-angular/src/app/common/alert-message/alert-message.service.ts
@@ -128,6 +128,10 @@ export class AlertMessageService {
this.showMessage(message, 'success', keepAfterNavigationChange, timeoutInSeconds);
}
+ warning(message: string, keepAfterNavigationChange = false, timeoutInSeconds: number = null) {
+ this.showMessage(message, 'warning', keepAfterNavigationChange, timeoutInSeconds);
+ }
+
error(message: any, keepAfterNavigationChange = false, timeoutInSeconds: number = null) {
this.showMessage(message, 'error', keepAfterNavigationChange, timeoutInSeconds);
}
diff --git a/smp-angular/src/app/common/model/keystore-result.model.ts b/smp-angular/src/app/common/model/keystore-result.model.ts
index 352592c4f..1f67bd075 100644
--- a/smp-angular/src/app/common/model/keystore-result.model.ts
+++ b/smp-angular/src/app/common/model/keystore-result.model.ts
@@ -5,4 +5,6 @@ export interface KeystoreResult {
errorMessage?: string;
addedCertificates?: CertificateRo[];
+
+ ignoredAliases?: String[];
}
diff --git a/smp-angular/src/app/system-settings/admin-keystore/keystore-import-dialog/keystore-import-dialog.component.spec.ts b/smp-angular/src/app/system-settings/admin-keystore/keystore-import-dialog/keystore-import-dialog.component.spec.ts
deleted file mode 100644
index 0e77656dd..000000000
--- a/smp-angular/src/app/system-settings/admin-keystore/keystore-import-dialog/keystore-import-dialog.component.spec.ts
+++ /dev/null
@@ -1,25 +0,0 @@
-import { async, ComponentFixture, TestBed } from '@angular/core/testing';
-
-import {KeystoreImportDialogComponent} from "./keystore-import-dialog.component";
-
-describe('KeystoreImportDialogComponent', () => {
- let component: KeystoreImportDialogComponent;
- let fixture: ComponentFixture<KeystoreImportDialogComponent>;
-
- beforeEach(async(() => {
- TestBed.configureTestingModule({
- declarations: [ KeystoreImportDialogComponent ]
- })
- .compileComponents();
- }));
-
- beforeEach(() => {
- fixture = TestBed.createComponent(KeystoreImportDialogComponent);
- component = fixture.componentInstance;
- fixture.detectChanges();
- });
-
- it('should be created', () => {
- expect(component).toBeTruthy();
- });
-});
diff --git a/smp-angular/src/app/system-settings/admin-keystore/keystore-import-dialog/keystore-import-dialog.component.ts b/smp-angular/src/app/system-settings/admin-keystore/keystore-import-dialog/keystore-import-dialog.component.ts
index 9e872cb6b..44d3b8910 100644
--- a/smp-angular/src/app/system-settings/admin-keystore/keystore-import-dialog/keystore-import-dialog.component.ts
+++ b/smp-angular/src/app/system-settings/admin-keystore/keystore-import-dialog/keystore-import-dialog.component.ts
@@ -2,8 +2,6 @@ import {Component, Inject} from '@angular/core';
import {MAT_DIALOG_DATA, MatDialogRef} from '@angular/material/dialog';
import {FormControl, FormGroup, UntypedFormBuilder, Validators} from "@angular/forms";
import {AlertMessageService} from "../../../common/alert-message/alert-message.service";
-import {HttpClient} from "@angular/common/http";
-import {SecurityService} from "../../../security/security.service";
import {AdminKeystoreService} from "../admin-keystore.service";
import {KeystoreResult} from "../../../common/model/keystore-result.model";
@@ -18,8 +16,6 @@ export class KeystoreImportDialogComponent {
selectedFile: File;
constructor(private keystoreService: AdminKeystoreService,
- private securityService: SecurityService,
- private http: HttpClient,
private dialogRef: MatDialogRef<KeystoreImportDialogComponent>,
private alertService: AlertMessageService,
@Inject(MAT_DIALOG_DATA) public data: any,
@@ -27,7 +23,7 @@ export class KeystoreImportDialogComponent {
this.formTitle = "Keystore import dialog";
- this.dialogForm = fb.group({
+ this.dialogForm = this.fb.group({
'file': new FormControl({value: '', readonly: false}, [Validators.required]),
'keystoreType': new FormControl({value: '', readonly: false}, [Validators.required]),
'password': new FormControl({value: '', readonly: false}, [Validators.required]),
@@ -44,11 +40,14 @@ export class KeystoreImportDialogComponent {
importKeystore() {
this.keystoreService.uploadKeystore(this.selectedFile, this.dialogForm.controls['keystoreType'].value,
- this.dialogForm.controls['password'].value).subscribe((res: KeystoreResult) => {
+ this.dialogForm.controls['password'].value).subscribe({next: (res: KeystoreResult) => {
if (res) {
if (res.errorMessage) {
this.alertService.exception("Error occurred while importing keystore:" + this.selectedFile.name, res.errorMessage, false);
} else {
+ if (res.ignoredAliases) {
+ this.alertService.warning("The following aliases have been ignored because they were already present in the current keystore: " + res.ignoredAliases.join(","), false);
+ }
this.keystoreService.notifyKeystoreEntriesUpdated(res.addedCertificates);
this.dialogRef.close();
}
@@ -56,9 +55,9 @@ export class KeystoreImportDialogComponent {
this.alertService.exception("Error occurred while reading keystore.", "Check if uploaded file has valid keystore type.", false);
}
},
- err => {
+ error: (err) => {
this.alertService.exception('Error uploading keystore file ' + this.selectedFile.name, err.error?.errorDescription);
}
- )
+ })
}
}
diff --git a/smp-server-library/src/main/java/eu/europa/ec/edelivery/smp/data/ui/KeystoreImportResult.java b/smp-server-library/src/main/java/eu/europa/ec/edelivery/smp/data/ui/KeystoreImportResult.java
index 51ce5863f..18c0412b5 100644
--- a/smp-server-library/src/main/java/eu/europa/ec/edelivery/smp/data/ui/KeystoreImportResult.java
+++ b/smp-server-library/src/main/java/eu/europa/ec/edelivery/smp/data/ui/KeystoreImportResult.java
@@ -20,10 +20,15 @@ package eu.europa.ec.edelivery.smp.data.ui;
import java.util.ArrayList;
import java.util.List;
+import java.util.Set;
+import java.util.TreeSet;
public class KeystoreImportResult {
String errorMessage;
+
+ Set<String> ignoredAliases = new TreeSet<>();
+
List<CertificateRO> addedCertificates = new ArrayList<>();
public String getErrorMessage() {
@@ -37,4 +42,8 @@ public class KeystoreImportResult {
public List<CertificateRO> getAddedCertificates() {
return addedCertificates;
}
+
+ public Set<String> getIgnoredAliases() {
+ return ignoredAliases;
+ }
}
diff --git a/smp-server-library/src/main/java/eu/europa/ec/edelivery/smp/services/ui/UIKeystoreService.java b/smp-server-library/src/main/java/eu/europa/ec/edelivery/smp/services/ui/UIKeystoreService.java
index 45db040bc..7ddd7fc2d 100644
--- a/smp-server-library/src/main/java/eu/europa/ec/edelivery/smp/services/ui/UIKeystoreService.java
+++ b/smp-server-library/src/main/java/eu/europa/ec/edelivery/smp/services/ui/UIKeystoreService.java
@@ -260,7 +260,6 @@ public class UIKeystoreService extends BasicKeystoreService {
* @param password password for new keystore file
*/
public List<CertificateRO> importKeys(KeyStore newKeystore, String password) throws UnrecoverableKeyException, NoSuchAlgorithmException, KeyStoreException, IOException, CertificateException {
-
String keystoreSecToken = configurationService.getKeystoreCredentialToken();
KeyStore keyStore = loadKeystore(configurationService.getKeystoreFile(), keystoreSecToken);
if (keyStore != null) {
@@ -274,6 +273,36 @@ public class UIKeystoreService extends BasicKeystoreService {
return Collections.emptyList();
}
+ /**
+ * Returns entries having certificates that are already present in the current keystore.
+ *
+ * @param newKeystore new keystore file to import
+ * @return the set of duplicate certificates
+ * @throws KeyStoreException when not able to read the keystore aliases
+ */
+ public Set<String> findDuplicateCertificates(KeyStore newKeystore) throws KeyStoreException {
+ LOG.debug("Searching for entries with duplicate certificates");
+
+ String keystoreSecToken = configurationService.getKeystoreCredentialToken();
+ KeyStore keyStore = loadKeystore(configurationService.getKeystoreFile(), keystoreSecToken);
+ if (keyStore != null) {
+ return list(newKeystore.aliases())
+ .stream()
+ .filter(alias -> containsCertificate(keyStore, newKeystore, alias))
+ .peek(alias -> LOG.debug("Found entry with duplicate certificate [{}]", alias))
+ .collect(Collectors.toSet());
+ }
+ return Collections.emptySet();
+ }
+
+ private boolean containsCertificate(KeyStore keyStore, KeyStore newKeystore, String alias) {
+ try {
+ return keyStore.getCertificateAlias(newKeystore.getCertificate(alias)) != null;
+ } catch (KeyStoreException e) {
+ throw new SMPRuntimeException(ErrorCode.CERTIFICATE_ERROR, "An error occurred while loading the entry " + alias, e);
+ }
+ }
+
/**
* Delete keys smp keystore
*
diff --git a/smp-server-library/src/main/java/eu/europa/ec/edelivery/smp/services/ui/UITruststoreService.java b/smp-server-library/src/main/java/eu/europa/ec/edelivery/smp/services/ui/UITruststoreService.java
index 667264482..88625ad60 100644
--- a/smp-server-library/src/main/java/eu/europa/ec/edelivery/smp/services/ui/UITruststoreService.java
+++ b/smp-server-library/src/main/java/eu/europa/ec/edelivery/smp/services/ui/UITruststoreService.java
@@ -25,6 +25,8 @@ import eu.europa.ec.edelivery.smp.data.model.user.DBUser;
import eu.europa.ec.edelivery.smp.data.ui.CertificateRO;
import eu.europa.ec.edelivery.smp.exceptions.CertificateAlreadyRegisteredException;
import eu.europa.ec.edelivery.smp.exceptions.CertificateNotTrustedException;
+import eu.europa.ec.edelivery.smp.exceptions.ErrorCode;
+import eu.europa.ec.edelivery.smp.exceptions.SMPRuntimeException;
import eu.europa.ec.edelivery.smp.logging.SMPLogger;
import eu.europa.ec.edelivery.smp.logging.SMPLoggerFactory;
import eu.europa.ec.edelivery.smp.services.CRLVerifierService;
@@ -502,9 +504,14 @@ public class UITruststoreService extends BasicKeystoreService {
}
public String addCertificate(String alias, X509Certificate certificate) throws NoSuchAlgorithmException, KeyStoreException, IOException, CertificateException {
-
KeyStore truststore = loadTruststore(getTruststoreFile());
if (truststore != null) {
+
+ String certificateAlias = truststore.getCertificateAlias(certificate);
+ if (certificateAlias != null) {
+ throw new SMPRuntimeException(ErrorCode.CERTIFICATE_ERROR, "duplicate", "The certificate you are trying to upload already exists under the [" + certificateAlias + "] entry");
+ }
+
String aliasPrivate = StringUtils.isBlank(alias) ? createAliasFromCert(certificate, truststore) : alias.trim();
if (truststore.containsAlias(aliasPrivate)) {
@@ -529,8 +536,6 @@ public class UITruststoreService extends BasicKeystoreService {
}
public String createAliasFromCert(X509Certificate x509cert, KeyStore truststore) {
-
-
String dn = x509cert.getSubjectX500Principal().getName();
String alias = null;
try {
diff --git a/smp-server-library/src/test/java/eu/europa/ec/edelivery/smp/services/ui/UIKeystoreServiceTest.java b/smp-server-library/src/test/java/eu/europa/ec/edelivery/smp/services/ui/UIKeystoreServiceTest.java
index 7fcf7ada5..720858369 100644
--- a/smp-server-library/src/test/java/eu/europa/ec/edelivery/smp/services/ui/UIKeystoreServiceTest.java
+++ b/smp-server-library/src/test/java/eu/europa/ec/edelivery/smp/services/ui/UIKeystoreServiceTest.java
@@ -36,14 +36,16 @@ import org.springframework.test.util.ReflectionTestUtils;
import javax.net.ssl.KeyManager;
import javax.security.auth.x500.X500Principal;
import java.io.File;
-import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
import java.security.*;
import java.security.cert.CertificateException;
import java.security.cert.X509Certificate;
+import java.util.Arrays;
+import java.util.HashSet;
import java.util.List;
+import java.util.Set;
import static org.junit.jupiter.api.Assertions.*;
@@ -254,5 +256,17 @@ public class UIKeystoreServiceTest extends AbstractServiceIntegrationTest {
assertNotEquals(km, testInstance.getKeyManagers()[0]);
}
+ @Test
+ void testFindDuplicateCertificate() throws Exception {
+ // given
+ KeyStore keyStore = loadKeystore("test-import.jks", "NewPassword1234", "JKS");
+ KeyStore sameKeyStore = keyStore;
+ testInstance.importKeys(keyStore, "NewPassword1234");
+
+ // when
+ Set<String> duplicateCertificates = testInstance.findDuplicateCertificates(keyStore);
+ // then
+ assertEquals(new HashSet<>(Arrays.asList("testcertificatea", "testcertificateb")), duplicateCertificates);
+ }
}
diff --git a/smp-server-library/src/test/java/eu/europa/ec/edelivery/smp/services/ui/UITruststoreServiceTest.java b/smp-server-library/src/test/java/eu/europa/ec/edelivery/smp/services/ui/UITruststoreServiceTest.java
index 48439490e..d8218b2e0 100644
--- a/smp-server-library/src/test/java/eu/europa/ec/edelivery/smp/services/ui/UITruststoreServiceTest.java
+++ b/smp-server-library/src/test/java/eu/europa/ec/edelivery/smp/services/ui/UITruststoreServiceTest.java
@@ -22,12 +22,14 @@ import eu.europa.ec.edelivery.smp.data.dao.UserDao;
import eu.europa.ec.edelivery.smp.data.model.user.DBUser;
import eu.europa.ec.edelivery.smp.data.ui.CertificateRO;
import eu.europa.ec.edelivery.smp.exceptions.CertificateNotTrustedException;
+import eu.europa.ec.edelivery.smp.exceptions.SMPRuntimeException;
import eu.europa.ec.edelivery.smp.services.CRLVerifierService;
import eu.europa.ec.edelivery.smp.services.ConfigurationService;
import eu.europa.ec.edelivery.smp.testutil.X509CertificateTestUtils;
import org.apache.commons.io.FileUtils;
import org.hamcrest.MatcherAssert;
import org.hamcrest.Matchers;
+import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.ArgumentCaptor;
@@ -401,6 +403,26 @@ class UITruststoreServiceTest {
assertEquals(count + 1, testInstance.getNormalizedTrustedList().size());
}
+ @Test
+ void testAddCertificate_DuplicateCertificate() throws Exception {
+ String alias = "duplicate";
+ String subject = "CN=Something,O=test,C=EU";
+ X509Certificate certificate = X509CertificateTestUtils.createX509CertificateForTest(subject);
+
+ doReturn(targetTruststore.toFile()).when(configurationService).getTruststoreFile();
+ doReturn(targetTruststore.toFile()).when(configurationService).getTruststoreFile();
+ doReturn(truststorePassword).when(configurationService).getTruststoreCredentialToken();
+ testInstance.refreshData();
+ int count = testInstance.getNormalizedTrustedList().size();
+
+ // when
+ testInstance.addCertificate(alias, certificate);
+ SMPRuntimeException smpRuntimeException = assertThrows(SMPRuntimeException.class, () -> testInstance.addCertificate(alias, certificate));
+
+ // then
+ Assertions.assertEquals("Certificate error [duplicate]. Error: The certificate you are trying to upload already exists under the [duplicate] entry!", smpRuntimeException.getMessage());
+ }
+
@Test
void testDeleteCertificate() throws Exception {
String subject = "CN=Something,O=test,C=EU";
@@ -418,7 +440,6 @@ class UITruststoreServiceTest {
assertEquals(count - 1, testInstance.getNormalizedTrustedList().size());
}
-
protected void resetKeystore() throws IOException {
FileUtils.deleteDirectory(targetDirectory.toFile());
FileUtils.copyDirectory(resourceDirectory.toFile(), targetDirectory.toFile());
diff --git a/smp-webapp/src/main/java/eu/europa/ec/edelivery/smp/ui/internal/KeystoreAdminController.java b/smp-webapp/src/main/java/eu/europa/ec/edelivery/smp/ui/internal/KeystoreAdminController.java
index a973ee80e..0c03e7d52 100644
--- a/smp-webapp/src/main/java/eu/europa/ec/edelivery/smp/ui/internal/KeystoreAdminController.java
+++ b/smp-webapp/src/main/java/eu/europa/ec/edelivery/smp/ui/internal/KeystoreAdminController.java
@@ -18,6 +18,7 @@
*/
package eu.europa.ec.edelivery.smp.ui.internal;
+import eu.europa.ec.edelivery.security.utils.KeystoreUtils;
import eu.europa.ec.edelivery.smp.data.ui.CertificateRO;
import eu.europa.ec.edelivery.smp.data.ui.KeystoreImportResult;
import eu.europa.ec.edelivery.smp.data.ui.enums.EntityROStatus;
@@ -38,7 +39,9 @@ import java.security.NoSuchAlgorithmException;
import java.security.UnrecoverableKeyException;
import java.security.cert.CertificateException;
import java.security.cert.X509Certificate;
+import java.util.HashSet;
import java.util.List;
+import java.util.Set;
import static eu.europa.ec.edelivery.smp.ui.ResourceConstants.*;
import static org.springframework.util.MimeTypeUtils.APPLICATION_JSON_VALUE;
@@ -90,9 +93,11 @@ public class KeystoreAdminController {
try {
KeyStore keyStore = KeyStore.getInstance(keystoreType);
keyStore.load(new ByteArrayInputStream(fileBytes), password.toCharArray());
+ Set<String> ignoredAliases = removeDuplicateCertificates(keyStore);
List<CertificateRO> certificateROList = uiKeystoreService.importKeys(keyStore, password);
certificateROList.forEach(cert -> cert.setStatus(EntityROStatus.NEW.getStatusNumber()));
keystoreImportResult.getAddedCertificates().addAll(certificateROList);
+ keystoreImportResult.getIgnoredAliases().addAll(ignoredAliases);
} catch (KeyStoreException | CertificateException | NoSuchAlgorithmException | IOException |
UnrecoverableKeyException e) {
String msg = e.getClass().getName() + " occurred while reading the keystore: " + e.getMessage();
@@ -102,6 +107,18 @@ public class KeystoreAdminController {
return keystoreImportResult;
}
+ private Set<String> removeDuplicateCertificates(KeyStore keyStore) throws KeyStoreException {
+ Set<String> duplicateAliases = new HashSet<>();
+
+ Set<String> duplicateCertificateAliases = uiKeystoreService.findDuplicateCertificates(keyStore);
+ for (String alias: duplicateCertificateAliases) {
+ KeystoreUtils.deleteCertificate(keyStore, alias);
+ duplicateAliases.add(alias);
+ }
+
+ return duplicateAliases;
+ }
+
@PreAuthorize("@smpAuthorizationService.systemAdministrator AND @smpAuthorizationService.isCurrentlyLoggedIn(#userEncId)")
@DeleteMapping(value = "/{user-id}/delete/{cert-alias}", produces = APPLICATION_JSON_VALUE)
public CertificateRO deleteCertificate(@PathVariable(PATH_PARAM_ENC_USER_ID) String userEncId,
diff --git a/smp-webapp/src/test/java/eu/europa/ec/edelivery/smp/ui/internal/KeystoreAdminControllerIT.java b/smp-webapp/src/test/java/eu/europa/ec/edelivery/smp/ui/internal/KeystoreAdminControllerIT.java
index 30742c1cf..9958cd26f 100644
--- a/smp-webapp/src/test/java/eu/europa/ec/edelivery/smp/ui/internal/KeystoreAdminControllerIT.java
+++ b/smp-webapp/src/test/java/eu/europa/ec/edelivery/smp/ui/internal/KeystoreAdminControllerIT.java
@@ -36,7 +36,10 @@ import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
+import java.util.Arrays;
+import java.util.HashSet;
import java.util.List;
+import java.util.stream.Collectors;
import static eu.europa.ec.edelivery.smp.test.testutils.MockMvcUtils.*;
import static eu.europa.ec.edelivery.smp.ui.ResourceConstants.CONTEXT_PATH_INTERNAL_KEYSTORE;
@@ -123,11 +126,13 @@ class KeystoreAdminControllerIT extends AbstractControllerTest {
}
@Test
- void uploadKeystoreOK() throws Exception {
-
+ void uploadKeystoreOK_removeDuplicateCertificates() throws Exception {
MockHttpSession session = loginWithSystemAdmin(mvc);
UserRO userRO = getLoggedUserData(mvc, session);
- int countStart = uiKeystoreService.getKeystoreEntriesList().size();
+ assertEquals(uiKeystoreService.getKeystoreEntriesList().stream()
+ .map(CertificateRO::getAlias)
+ .collect(Collectors.toSet()), new HashSet<>(Arrays.asList("second_domain_alias", "single_domain_key")));
+
// given when
MvcResult result = mvc.perform(post(PATH + "/" + userRO.getUserId() + "/upload/JKS/test123")
.session(session)
@@ -140,7 +145,11 @@ class KeystoreAdminControllerIT extends AbstractControllerTest {
assertNotNull(res);
assertNull(res.getErrorMessage());
- assertEquals(countStart + 1, uiKeystoreService.getKeystoreEntriesList().size());
+ assertTrue(res.getAddedCertificates().isEmpty());
+ assertEquals(res.getIgnoredAliases(), new HashSet(Arrays.asList("single_domain_key")));
+ assertEquals(uiKeystoreService.getKeystoreEntriesList().stream()
+ .map(CertificateRO::getAlias)
+ .collect(Collectors.toSet()), new HashSet<>(Arrays.asList("second_domain_alias", "single_domain_key")));
}
@Test
--
GitLab