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.

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);
   }

smp-angular/src/app/common/model/keystore-result.model.ts

@@ -5,4 +5,6 @@ export interface KeystoreResult {
   errorMessage?: string;
 
   addedCertificates?: CertificateRo[];
+
+  ignoredAliases?: String[];
 }

smp-angular/src/app/system-settings/admin-keystore/keystore-import-dialog/keystore-import-dialog.component.spec.ts

-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();
-  });
-});

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);
       }
-    )
+    })
   }
 }

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;
+    }
 }

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
      *

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 {

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);
+    }
 }

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());

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,

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