Skip to content

Commit

Permalink
feat: Bump certificate hashing algo SHA1 -> SHA256 except for globalc…
Browse files Browse the repository at this point in the history
…onf V2's authCertHash (#1873)

* feat: Bump certificate hashing algo SHA1 -> SHA256 except for globalconf V2's authCertHash

Refs: XRDDEV-445

* fix: Overflowing cert hash in the UI + CR suggestions

Refs: XRDDEV-445
  • Loading branch information
andresrosenthal committed Dec 1, 2023
1 parent c172d58 commit b266030
Show file tree
Hide file tree
Showing 40 changed files with 339 additions and 289 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,15 @@
<owner>id0</owner>
<serverCode>SS0</serverCode>
<address>ss0</address>
<authCertHash>5+C5Gr24Dh912x5haKGOyZuK2KI=</authCertHash>
<authCertHash>enRxKzBGb0q7sUJa8Hx0A5aHmQwCa1HzWNFrN3ZDDJw=</authCertHash>
<client>id1</client>
<client>id7</client>
</securityServer>
<securityServer>
<owner>id4</owner>
<serverCode>SS1</serverCode>
<address>ss1</address>
<authCertHash>03SfHhv+L5OJrJaod/sOZn6vp1c=</authCertHash>
<authCertHash>Q/wASEqban646kxZ0/uveKBv4h7U3FWnlKzsSJZU1f8=</authCertHash>
<client>id5</client>
<client>id6</client>
<client>id3</client>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ void toTarget() throws Exception {
assertThat(result.getValidFrom()).isEqualTo(VALID_FROM);
assertThat(result.getValidTo()).isEqualTo(VALID_TO);

assertThat(result.getCertificate().getHash()).isEqualTo("05A10EEBDB0CD9679E4C85A78848145EF1F00BEA");
assertThat(result.getCertificate().getHash()).isEqualTo("094D62D75ECC25D6BD9EA83C7B34678016BB72BB80118FF6EC7E4D383A678CD1");
assertThat(result.getCertificate().getIssuerCommonName()).isEqualTo("AdminCA1");
assertThat(result.getCertificate().getIssuerDistinguishedName()).isEqualTo("C=SE, O=EJBCA Sample, CN=AdminCA1");
assertThat(result.getCertificate().getKeyUsages()).isEqualTo(Set.of(NON_REPUDIATION));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,8 @@ void addCertificationServiceOcspResponder() throws Exception {
verify(auditDataHelper).put(CA_ID, mockOcspInfo.getCaInfo().getId());
verify(auditDataHelper).put(OCSP_ID, mockOcspInfo.getId());
verify(auditDataHelper).put(OCSP_URL, mockOcspInfo.getUrl());
verify(auditDataHelper).put(OCSP_CERT_HASH, "F5:1B:1F:9C:07:23:4C:DA:E6:4C:99:CB:FC:D8:EE:0E:C5:5F:A4:AF");
verify(auditDataHelper).put(OCSP_CERT_HASH,
"B9:CF:6E:A1:BC:98:24:6B:16:68:24:E3:9A:9F:CD:8E:51:B7:05:37:44:68:D4:96:50:D2:22:85:A7:FA:54:2B");
verify(auditDataHelper).put(OCSP_CERT_HASH_ALGORITHM, DEFAULT_CERT_HASH_ALGORITHM_ID);
}

Expand All @@ -210,7 +211,7 @@ void addIntermediateCa() {

final CertificateAuthority certificateAuthority = service.addIntermediateCa(ID, certificateBytes);

assertEquals("24AFDE09AA818A20D3EE7A4A2264BA247DA5C3F9", certificateAuthority.getCaCertificate().getHash());
assertEquals("D8FD191D4155864DE4DB7F8A5E099DAF70E57AF1B62A2A9B3B3B0C2B51788994", certificateAuthority.getCaCertificate().getHash());

ArgumentCaptor<CaInfoEntity> captor = ArgumentCaptor.forClass(CaInfoEntity.class);
verify(caInfoRepository).save(captor.capture());
Expand All @@ -220,7 +221,8 @@ void addIntermediateCa() {

verify(auditDataHelper).put(CA_ID, ID);
verify(auditDataHelper).put(INTERMEDIATE_CA_ID, 0);
verify(auditDataHelper).put(INTERMEDIATE_CA_CERT_HASH, "24:AF:DE:09:AA:81:8A:20:D3:EE:7A:4A:22:64:BA:24:7D:A5:C3:F9");
verify(auditDataHelper).put(INTERMEDIATE_CA_CERT_HASH,
"D8:FD:19:1D:41:55:86:4D:E4:DB:7F:8A:5E:09:9D:AF:70:E5:7A:F1:B6:2A:2A:9B:3B:3B:0C:2B:51:78:89:94");
verify(auditDataHelper).put(INTERMEDIATE_CA_CERT_HASH_ALGORITHM, DEFAULT_CERT_HASH_ALGORITHM_ID);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,8 @@ void addOcspResponder() {
verify(auditDataHelper).put(INTERMEDIATE_CA_ID, ID);
verify(auditDataHelper).put(OCSP_ID, NEW_ID);
verify(auditDataHelper).put(OCSP_URL, URL);
verify(auditDataHelper).put(OCSP_CERT_HASH, "F5:1B:1F:9C:07:23:4C:DA:E6:4C:99:CB:FC:D8:EE:0E:C5:5F:A4:AF");
verify(auditDataHelper).put(OCSP_CERT_HASH,
"B9:CF:6E:A1:BC:98:24:6B:16:68:24:E3:9A:9F:CD:8E:51:B7:05:37:44:68:D4:96:50:D2:22:85:A7:FA:54:2B");
verify(auditDataHelper).put(OCSP_CERT_HASH_ALGORITHM, DEFAULT_CERT_HASH_ALGORITHM_ID);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,8 @@ void add() throws Exception {
verify(auditDataHelper).put(TSA_ID, ID);
verify(auditDataHelper).put(TSA_NAME, NAME);
verify(auditDataHelper).put(TSA_URL, URL);
verify(auditDataHelper).put(TSA_CERT_HASH, "05:A1:0E:EB:DB:0C:D9:67:9E:4C:85:A7:88:48:14:5E:F1:F0:0B:EA");
verify(auditDataHelper).put(TSA_CERT_HASH,
"09:4D:62:D7:5E:CC:25:D6:BD:9E:A8:3C:7B:34:67:80:16:BB:72:BB:80:11:8F:F6:EC:7E:4D:38:3A:67:8C:D1");
verify(auditDataHelper).put(TSA_CERT_HASH_ALGORITHM, DEFAULT_CERT_HASH_ALGORITHM_ID);
}

Expand All @@ -139,7 +140,8 @@ void update() throws Exception {
verify(auditDataHelper).put(TSA_ID, ID);
verify(auditDataHelper).put(TSA_NAME, NAME);
verify(auditDataHelper).put(TSA_URL, request.getUrl());
verify(auditDataHelper).put(TSA_CERT_HASH, "05:A1:0E:EB:DB:0C:D9:67:9E:4C:85:A7:88:48:14:5E:F1:F0:0B:EA");
verify(auditDataHelper).put(TSA_CERT_HASH,
"09:4D:62:D7:5E:CC:25:D6:BD:9E:A8:3C:7B:34:67:80:16:BB:72:BB:80:11:8F:F6:EC:7E:4D:38:3A:67:8C:D1");
verify(auditDataHelper).put(TSA_CERT_HASH_ALGORITHM, DEFAULT_CERT_HASH_ALGORITHM_ID);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public static class SecurityServer {
private ClientId owner;
private String serverCode;
private String address;
private List<byte[]> authCertHashes;
private List<byte[]> authCerts;
private List<ClientId> clients;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,8 @@
package org.niis.xroad.cs.admin.globalconf.generator;

import ee.ria.xroad.common.identifier.ClientId;
import ee.ria.xroad.common.util.CryptoUtils;

import lombok.RequiredArgsConstructor;
import lombok.SneakyThrows;
import lombok.extern.slf4j.Slf4j;
import org.niis.xroad.cs.admin.api.domain.AuthCert;
import org.niis.xroad.cs.admin.api.domain.ConfigurationSigningKey;
Expand Down Expand Up @@ -91,7 +89,7 @@ SharedParameters load() {
private List<SharedParameters.ConfigurationSource> getSources() {
return configurationService.getNodeAddressesWithConfigurationSigningKeys().entrySet().stream()
.map(this::toSource)
.collect(toList());
.toList();
}

private SharedParameters.ConfigurationSource toSource(
Expand Down Expand Up @@ -121,7 +119,7 @@ private List<SharedParameters.ApprovedCA> getApprovedCAs() {
var approvedCas = certificationServicesService.findAll();
return approvedCas.stream()
.map(this::toApprovedCa)
.collect(toList());
.toList();
}

private SharedParameters.ApprovedCA toApprovedCa(CertificationService ca) {
Expand All @@ -137,13 +135,13 @@ private SharedParameters.ApprovedCA toApprovedCa(CertificationService ca) {
private List<SharedParameters.CaInfo> toCaInfos(List<CertificateAuthority> cas) {
return cas.stream()
.map(ca -> new SharedParameters.CaInfo(toOcspInfos(ca.getOcspResponders()), ca.getCaCertificate().getEncoded()))
.collect(toList());
.toList();
}

private List<SharedParameters.OcspInfo> toOcspInfos(List<OcspResponder> ocspResponders) {
return ocspResponders.stream()
.map(this::toOcspInfo)
.collect(toList());
.toList();
}

private SharedParameters.OcspInfo toOcspInfo(OcspResponder ocsp) {
Expand All @@ -153,7 +151,7 @@ private SharedParameters.OcspInfo toOcspInfo(OcspResponder ocsp) {
private List<SharedParameters.ApprovedTSA> getApprovedTSAs() {
return timestampingServicesService.getTimestampingServices().stream()
.map(tsa -> new SharedParameters.ApprovedTSA(tsa.getName(), tsa.getUrl(), tsa.getCertificate().getEncoded()))
.collect(toList());
.toList();
}

private List<SharedParameters.Member> getMembers() {
Expand All @@ -164,7 +162,7 @@ private List<SharedParameters.Member> getMembers() {
private List<SharedParameters.SecurityServer> getSecurityServers() {
return securityServerService.findAll().stream()
.map(this::toSecurityServer)
.collect(toList());
.toList();
}

private SharedParameters.SecurityServer toSecurityServer(SecurityServer ss) {
Expand All @@ -173,22 +171,16 @@ private SharedParameters.SecurityServer toSecurityServer(SecurityServer ss) {
result.setAddress(ss.getAddress());
result.setServerCode(ss.getServerCode());
result.setClients(getSecurityServerClients(ss.getId()));
result.setAuthCertHashes(ss.getAuthCerts().stream()
result.setAuthCerts(ss.getAuthCerts().stream()
.map(AuthCert::getCert)
.map(SharedParametersLoader::certHash)
.collect(toList()));
.toList());
return result;
}

private List<ClientId> getSecurityServerClients(int id) {
return clientService.find(new ClientService.SearchParameters().setSecurityServerId(id))
.stream().map(SharedParametersLoader::toClientId).collect(toList());

}
.stream().map(SharedParametersLoader::toClientId).toList();

@SneakyThrows
private static byte[] certHash(byte[] cert) {
return CryptoUtils.certHash(cert);
}

private static ClientId toClientId(FlattenedSecurityServerClientView client) {
Expand All @@ -201,7 +193,7 @@ private static ClientId toClientId(FlattenedSecurityServerClientView client) {
private List<SharedParameters.GlobalGroup> getGlobalGroups() {
return globalGroupService.findGlobalGroups().stream()
.map(this::getGlobalGroup)
.collect(toList());
.toList();
}

private SharedParameters.GlobalGroup getGlobalGroup(GlobalGroup globalGroup) {
Expand All @@ -220,7 +212,7 @@ private List<ClientId> getGroupMembers(String groupCode) {
private SharedParameters.GlobalSettings getGlobalSettings() {
var memberClasses = memberClassService.findAll().stream()
.map(memberClass -> new SharedParameters.MemberClass(memberClass.getCode(), memberClass.getDescription()))
.collect(toList());
.toList();

return new SharedParameters.GlobalSettings(memberClasses, systemParameterService.getOcspFreshnessSeconds());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,10 @@
import ee.ria.xroad.common.conf.globalconf.sharedparameters.v2.SharedParametersTypeV2;
import ee.ria.xroad.common.conf.globalconf.sharedparameters.v2.SubsystemType;
import ee.ria.xroad.common.identifier.ClientId;
import ee.ria.xroad.common.util.CryptoUtils;

import jakarta.xml.bind.JAXBElement;
import lombok.SneakyThrows;
import org.mapstruct.Context;
import org.mapstruct.Mapper;
import org.mapstruct.Mapping;
Expand All @@ -48,8 +50,6 @@
import java.util.List;
import java.util.Map;

import static java.util.stream.Collectors.toList;

@Mapper(uses = {ObjectFactory.class, MappingUtils.class}, unmappedTargetPolicy = ReportingPolicy.ERROR)
abstract class SharedParametersV2Converter {
public static final SharedParametersV2Converter INSTANCE = Mappers.getMapper(SharedParametersV2Converter.class);
Expand All @@ -73,7 +73,7 @@ SharedParametersTypeV2 convert(SharedParameters sharedParameters) {
@Mapping(source = "intermediateCAs", target = "intermediateCA")
abstract ApprovedCATypeV2 convert(SharedParameters.ApprovedCA approvedCa);

@Mapping(source = "authCertHashes", target = "authCertHash")
@Mapping(source = "authCerts", target = "authCertHash", qualifiedByName = "toAuthCertHashes")
@Mapping(source = "clients", target = "client", qualifiedByName = "clientsById")
@Mapping(target = "owner", qualifiedByName = "clientById")
abstract SecurityServerType convert(SharedParameters.SecurityServer securityServer, @Context Map<ClientId, Object> clientMap);
Expand Down Expand Up @@ -104,7 +104,19 @@ List<JAXBElement<Object>> xmlClientIds(List<ClientId> clientIds, @Context Map<Cl
}
return clientIds.stream()
.map(clientId -> OBJECT_FACTORY.createSecurityServerTypeClient(xmlClientId(clientId, clientMap)))
.collect(toList());
.toList();
}

@Named("toAuthCertHashes")
protected List<byte[]> toAuthCertHashes(List<byte[]> authCerts) {
return authCerts.stream()
.map(this::toAuthCertHash)
.toList();
}

@SneakyThrows
private byte[] toAuthCertHash(byte[] authCert) {
return CryptoUtils.certSha1Hash(authCert);
}

private Map<ClientId, Object> createClientIdMap(SharedParameters sharedParameters) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,10 @@
import ee.ria.xroad.common.conf.globalconf.sharedparameters.v3.SharedParametersTypeV3;
import ee.ria.xroad.common.conf.globalconf.sharedparameters.v3.SubsystemType;
import ee.ria.xroad.common.identifier.ClientId;
import ee.ria.xroad.common.util.CryptoUtils;

import jakarta.xml.bind.JAXBElement;
import lombok.SneakyThrows;
import org.mapstruct.Context;
import org.mapstruct.Mapper;
import org.mapstruct.Mapping;
Expand All @@ -50,8 +52,6 @@
import java.util.List;
import java.util.Map;

import static java.util.stream.Collectors.toList;

@Mapper(uses = {ObjectFactory.class, MappingUtils.class}, unmappedTargetPolicy = ReportingPolicy.ERROR)

abstract class SharedParametersV3Converter {
Expand Down Expand Up @@ -82,7 +82,7 @@ abstract SharedParametersTypeV3 convert(SharedParameters sharedParameters,
@Mapping(source = "intermediateCAs", target = "intermediateCA")
abstract ApprovedCATypeV3 convert(SharedParameters.ApprovedCA approvedCa);

@Mapping(source = "authCertHashes", target = "authCertHash")
@Mapping(source = "authCerts", target = "authCertHash", qualifiedByName = "toAuthCertHashes")
@Mapping(source = "clients", target = "client", qualifiedByName = "clientsById")
@Mapping(target = "owner", qualifiedByName = "clientById")
abstract SecurityServerType convert(SharedParameters.SecurityServer securityServer, @Context Map<ClientId, Object> clientMap);
Expand Down Expand Up @@ -113,7 +113,19 @@ List<JAXBElement<Object>> xmlClientIds(List<ClientId> clientIds, @Context Map<Cl
}
return clientIds.stream()
.map(clientId -> OBJECT_FACTORY.createSecurityServerTypeClient(xmlClientId(clientId, clientMap)))
.collect(toList());
.toList();
}

@Named("toAuthCertHashes")
protected List<byte[]> toAuthCertHashes(List<byte[]> authCerts) {
return authCerts.stream()
.map(this::toAuthCertHash)
.toList();
}

@SneakyThrows
private byte[] toAuthCertHash(byte[] authCert) {
return CryptoUtils.certHash(authCert);
}

private Map<ClientId, Object> createClientIdMap(SharedParameters sharedParameters) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
package org.niis.xroad.cs.admin.globalconf.generator;

import ee.ria.xroad.common.identifier.ClientId;
import ee.ria.xroad.common.util.CryptoUtils;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
Expand Down Expand Up @@ -174,7 +173,7 @@ private void assertSecurityServers(SharedParameters parameters) {
assertThat(ss.getServerCode()).isEqualTo(SECURITY_SERVER_CODE);
assertThat(ss.getClients()).singleElement()
.isEqualTo(ClientId.Conf.create(XROAD_INSTANCE, "CLASS", "M2", "S1"));
assertThat(ss.getAuthCertHashes()).singleElement().isEqualTo(CryptoUtils.certHash(SECURITY_SERVER_AUTH_CERT));
assertThat(ss.getAuthCerts()).singleElement().isEqualTo(SECURITY_SERVER_AUTH_CERT);
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import ee.ria.xroad.common.conf.globalconf.sharedparameters.v2.ObjectFactory;
import ee.ria.xroad.common.conf.globalconf.sharedparameters.v2.SharedParametersTypeV2;
import ee.ria.xroad.common.identifier.ClientId;
import ee.ria.xroad.common.util.CryptoUtils;

import jakarta.xml.bind.JAXBContext;
import jakarta.xml.bind.JAXBElement;
Expand All @@ -37,8 +38,10 @@
import lombok.extern.slf4j.Slf4j;
import org.assertj.core.api.recursive.comparison.ComparingNormalizedFields;
import org.assertj.core.api.recursive.comparison.RecursiveComparisonConfiguration;
import org.bouncycastle.operator.OperatorCreationException;
import org.junit.jupiter.api.Test;

import java.io.IOException;
import java.io.StringWriter;
import java.math.BigInteger;
import java.util.List;
Expand All @@ -52,6 +55,7 @@

@Slf4j
class SharedParametersV2ConverterTest {

private static final Map<String, String> FIELD_NAME_MAP = Map.ofEntries(
entry("securityServer", "securityServers"),
entry("approvedCA", "approvedCAs"),
Expand All @@ -62,19 +66,20 @@ class SharedParametersV2ConverterTest {
entry("subsystem", "subsystems"),
entry("client", "clients"),
entry("memberClass", "memberClasses"),
entry("authCertHash", "authCertHashes"),
entry("authCertHash", "authCerts"),
entry("groupMember", "groupMembers")
);

@Test
void shouldConvertAllFields() {
void shouldConvertAllFields() throws IOException, OperatorCreationException {
var sharedParameters = getSharedParameters();
var xmlType = SharedParametersV2Converter.INSTANCE.convert(sharedParameters);

var conf = RecursiveComparisonConfiguration.builder()
.withIntrospectionStrategy(compareRenamedFields())
.withIgnoredFields("securityServers.owner",
"securityServers.clients",
"securityServers.authCerts",
"members.id",
"members.subsystems.id",
"centralService"
Expand All @@ -95,6 +100,8 @@ void shouldConvertAllFields() {
.allFieldsSatisfy(Objects::nonNull);

assertIdReferences(xmlType);
assertThat(xmlType.getSecurityServer().get(0).getAuthCertHash().get(0))
.isEqualTo(CryptoUtils.certSha1Hash(sharedParameters.getSecurityServers().get(0).getAuthCerts().get(0)));
}

@Test
Expand Down Expand Up @@ -191,7 +198,7 @@ private static SharedParameters.SecurityServer getSecurityServer() {
securityServer.setServerCode("security-server-code");
securityServer.setAddress("security-server-address");
securityServer.setClients(List.of(subsystemId(memberId(), "SUB1")));
securityServer.setAuthCertHashes(List.of("ss-auth-cert".getBytes(UTF_8)));
securityServer.setAuthCerts(List.of("ss-auth-cert".getBytes(UTF_8)));
return securityServer;
}

Expand Down
Loading

0 comments on commit b266030

Please sign in to comment.