Skip to content

Commit

Permalink
Fix remove deleted certificates from response (#173)
Browse files Browse the repository at this point in the history
* Include deleted SignerInformation only if isModifiedSince is set, part 2

* Update pom.xml

* Update dependencies, explicitly add jackson-databind for CVE-2020-36518

Co-authored-by: Felix Dittrich <[email protected]>
  • Loading branch information
bergmann-dierk and f11h committed Mar 28, 2022
1 parent fbdff82 commit 38f2a53
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 16 deletions.
11 changes: 9 additions & 2 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<parent>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-parent</artifactId>
<version>2.6.4</version>
<version>2.6.5</version>
<relativePath/>
</parent>

Expand Down Expand Up @@ -57,7 +57,8 @@
<spring.cloud.version>2021.0.1</spring.cloud.version>
<h2.version>2.1.210</h2.version>
<hibernate.version>5.6.5.Final</hibernate.version>
<dgc.lib.version>1.3.0</dgc.lib.version>
<dgc.lib.version>1.3.1</dgc.lib.version>
<jackson.databind.version>2.13.2.1</jackson.databind.version>
<!-- plugins -->
<plugin.maven-assembly.version>3.3.0</plugin.maven-assembly.version>
<plugin.checkstyle.version>3.1.2</plugin.checkstyle.version>
Expand Down Expand Up @@ -266,6 +267,12 @@
<artifactId>shedlock-spring</artifactId>
<version>${shedlock.version}</version>
</dependency>
<!-- Explicitly needed due to https://nvd.nist.gov/vuln/detail/CVE-2020-36518 until core is updated -->
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-databind</artifactId>
<version>${jackson.databind.version}</version>
</dependency>
</dependencies>

<build>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,16 @@ public interface SignerInformationRepository extends JpaRepository<SignerInforma
@Query("DELETE FROM SignerInformationEntity s WHERE s.deletedAt < :threshold")
int deleteDeletedSignerInformationOlderThan(@Param("threshold") ZonedDateTime threshold);

List<SignerInformationEntity> getByCertificateType(SignerInformationEntity.CertificateType type,
List<SignerInformationEntity> getByCertificateTypeAndDeletedAtIsNull(SignerInformationEntity.CertificateType type,
Pageable pageable);

List<SignerInformationEntity> getByCertificateType(SignerInformationEntity.CertificateType type);
List<SignerInformationEntity> getByCertificateTypeAndDeletedAtIsNull(SignerInformationEntity.CertificateType type);

List<SignerInformationEntity> getByCertificateTypeAndCountry(
List<SignerInformationEntity> getByCertificateTypeAndCountryAndDeletedAtIsNull(
SignerInformationEntity.CertificateType type, String countryCode,
Pageable pageable);

List<SignerInformationEntity> getByCertificateTypeAndCountry(
List<SignerInformationEntity> getByCertificateTypeAndCountryAndDeletedAtIsNull(
SignerInformationEntity.CertificateType type, String countryCode);

@Query(SELECT_SINCE)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public List<SignerInformationEntity> getSignerInformation() {
* @return List of SignerInformation
*/
public List<SignerInformationEntity> getSignerInformation(SignerInformationEntity.CertificateType type) {
return signerInformationRepository.getByCertificateType(type);
return signerInformationRepository.getByCertificateTypeAndDeletedAtIsNull(type);
}

/**
Expand All @@ -92,7 +92,7 @@ public List<SignerInformationEntity> getSignerInformation(SignerInformationEntit
public List<SignerInformationEntity> getSignerInformation(
String countryCode,
SignerInformationEntity.CertificateType type) {
return signerInformationRepository.getByCertificateTypeAndCountry(type, countryCode);
return signerInformationRepository.getByCertificateTypeAndCountryAndDeletedAtIsNull(type, countryCode);
}

/**
Expand Down Expand Up @@ -136,10 +136,10 @@ public List<SignerInformationEntity> getSignerInformation(SignerInformationEntit
} else if (ifModifiedSince != null) {
return signerInformationRepository.getByCertificateTypeIsSince(type, ifModifiedSince);
} else if (page != null && size != null) {
return signerInformationRepository.getByCertificateType(type,
return signerInformationRepository.getByCertificateTypeAndDeletedAtIsNull(type,
PageRequest.of(page, size));
} else {
return signerInformationRepository.getByCertificateType(type);
return signerInformationRepository.getByCertificateTypeAndDeletedAtIsNull(type);
}
}

Expand All @@ -166,10 +166,10 @@ public List<SignerInformationEntity> getSignerInformation(
return signerInformationRepository.getByCertificateTypeAndCountryIsSince(type, countryCode,
ifModifiedSince);
} else if (page != null && size != null) {
return signerInformationRepository.getByCertificateTypeAndCountry(type, countryCode,
return signerInformationRepository.getByCertificateTypeAndCountryAndDeletedAtIsNull(type, countryCode,
PageRequest.of(page, size));
} else {
return signerInformationRepository.getByCertificateTypeAndCountry(type, countryCode);
return signerInformationRepository.getByCertificateTypeAndCountryAndDeletedAtIsNull(type, countryCode);
}
}

Expand Down Expand Up @@ -308,7 +308,7 @@ public void deleteSignerCertificate(
*/
public List<CmsPackageDto> getCmsPackage(String country) {
return signerInformationRepository
.getByCertificateTypeAndCountry(SignerInformationEntity.CertificateType.DSC, country)
.getByCertificateTypeAndCountryAndDeletedAtIsNull(SignerInformationEntity.CertificateType.DSC, country)
.stream()
.map(this::addCertificateToSignaturePayload)
.map(it -> new CmsPackageDto(it.getSignature(), it.getId(), CmsPackageDto.CmsPackageTypeDto.DSC))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ void testSuccessfulGetSignerInformationByTypeAndCountryIsSincePageable() throws
List<SignerInformationEntity> signerInformationEntities =
signerInformationService.getSignerInformation(SignerInformationEntity.CertificateType.DSC,
null, null, null);
Assertions.assertEquals(7, signerInformationEntities.size());
Assertions.assertEquals(6, signerInformationEntities.size());

List<SignerInformationEntity> signerInformationEntities2 = signerInformationService.getSignerInformation(
SignerInformationEntity.CertificateType.DSC,
Expand Down Expand Up @@ -244,9 +244,16 @@ void testSuccessfulAddingNewSignerInformationAndDelete() throws Exception {
countryCode
);

List<SignerInformationEntity> entities =
signerInformationRepository.getByCertificateType(SignerInformationEntity.CertificateType.DSC);
// Deleted certificate should not be returned in queries without isSince
List<SignerInformationEntity> entitiesByCertificateType =
signerInformationRepository.getByCertificateTypeAndDeletedAtIsNull(SignerInformationEntity.CertificateType.DSC);
Assertions.assertTrue(entitiesByCertificateType.isEmpty());
List<SignerInformationEntity> entitiesByCertificateTypeAndCountry =
signerInformationRepository.getByCertificateTypeAndCountryAndDeletedAtIsNull(
SignerInformationEntity.CertificateType.DSC, countryCode);
Assertions.assertTrue(entitiesByCertificateTypeAndCountry.isEmpty());

List<SignerInformationEntity> entities = signerInformationRepository.findAll();
Assertions.assertFalse(entities.isEmpty());
SignerInformationEntity deletedSignerInformationEntity = entities.get(0);
Assertions.assertEquals(createdSignerInformationEntity.get().getThumbprint(), deletedSignerInformationEntity.getThumbprint());
Expand Down

0 comments on commit 38f2a53

Please sign in to comment.