Skip to content

Commit

Permalink
Fix unsafe implementation to generate cert aliases
Browse files Browse the repository at this point in the history
Was based on an assumption that a path string obtained from a classpath URL can always be converted
to a Path instance, which is not the case, esp. on Windows.

Doing e.g. Paths.get("file:/C:/path/WEB-INF/lib/lib.jar!/file/in/jar") will make
sun.nio.fs.WindowsPathParser throw an InvalidPathException.
  • Loading branch information
runeflobakk committed Jun 28, 2022
1 parent 2a52e35 commit b5681e0
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,10 @@
import java.security.cert.CertificateException;
import java.security.cert.CertificateFactory;
import java.security.cert.X509Certificate;
import java.util.UUID;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.stream.Stream;

import static java.nio.file.Files.isDirectory;
import static java.util.stream.Collectors.joining;
import static java.util.stream.StreamSupport.stream;

public class TrustStoreLoader {

Expand Down Expand Up @@ -86,7 +84,7 @@ public void forEachFile(ForFile forEachFile) {
}

try (InputStream inputStream = resourceUrl.openStream()) {
forEachFile.call(generateAlias(Paths.get(resourceUrl.getPath())), inputStream);
forEachFile.call(generateAlias(resourceName), inputStream);
} catch (Exception e) {
throw new ConfigurationException("Unable to load certificate from classpath: " + resourceName, e);
}
Expand Down Expand Up @@ -130,12 +128,25 @@ private interface ForFile {
void call(String fileName, InputStream contents) throws IOException, GeneralSecurityException;
}

private static String generateAlias(Path location) {
return stream(location.normalize().spliterator(), false)
.reduce((e1, e2) -> e1.getFileName().resolve(e2))
.map(nameBase -> stream(nameBase.spliterator(), false).map(Path::toString).collect(joining(":")))
.orElseGet(() -> UUID.randomUUID().toString());
static String generateAlias(Path location) {
return generateAlias(location.toString());
}

private static final AtomicInteger aliasSequence = new AtomicInteger();

static String generateAlias(String resourceName) {
if (resourceName == null || resourceName.trim().isEmpty()) {
return "certificate-alias-" + aliasSequence.getAndIncrement();
}
String[] splitOnSlashes = resourceName.split("/");
int size = splitOnSlashes.length;
if (size == 1) {
return splitOnSlashes[0];
} else {
return splitOnSlashes[size - 2] + ":" + splitOnSlashes[size - 1];
}
}


}

Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,33 @@
import org.hamcrest.Matcher;
import org.hamcrest.TypeSafeDiagnosingMatcher;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;

import java.nio.file.Paths;
import java.security.KeyStore;
import java.security.KeyStoreException;
import java.util.List;
import java.util.stream.IntStream;
import java.util.stream.Stream;

import static java.util.Collections.list;
import static java.util.stream.Collectors.toList;
import static no.digipost.signature.client.Certificates.PRODUCTION;
import static no.digipost.signature.client.Certificates.TEST;
import static no.digipost.signature.client.core.internal.security.TrustStoreLoader.generateAlias;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.everyItem;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.matchesRegex;
import static org.hamcrest.Matchers.notNullValue;
import static org.junit.jupiter.api.Assertions.assertAll;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.quicktheories.QuickTheory.qt;
import static org.quicktheories.generators.SourceDSL.strings;
import static uk.co.probablyfine.matchers.Java8Matchers.where;

class TrustStoreLoaderTest {

Expand Down Expand Up @@ -78,6 +92,50 @@ void loads_certificates_from_file_location() throws KeyStoreException {
assertThat(trustStore, containsExactlyTheAliases("certificatetest:commfides_test_ca.cer"));
}

@Nested
class GenerateAlias {
@Test
void generateAliasFromUnixPath() {
assertThat(generateAlias(Paths.get("/blah/blah/funny/env/MyCert.cer")), is("env:MyCert.cer"));
}

@Test
void generateAliasFromWindowsPath() {
assertThat(generateAlias(Paths.get("C:/blah/blah/funny/env/MyCert.cer")), is("env:MyCert.cer"));
}

@Test
void generateAliasFromUnixFileInJarUrlString() {
assertThat(generateAlias("/blah/fun/prod/WEB-INF/lib/mylib.jar!/certificates/env/MyCert.cer"), is("env:MyCert.cer"));
}

@Test
void generateAliasFromWindowsFileInJarUrlString() {
assertThat(generateAlias("file:/C:/blah/fun/prod/WEB-INF/lib/mylib.jar!/certificates/env/MyCert.cer"), is("env:MyCert.cer"));
}

@Test
void generateUniqueDefaultAliasesForNullsAndEmptyStrings() {
List<String> defaultAliases = Stream.<String>of(null, "", " ", " \n ").map(s -> TrustStoreLoader.generateAlias(s)).collect(toList());
int aliasCount = defaultAliases.size();
assertAll("default aliases",
() -> assertThat(defaultAliases, everyItem(matchesRegex("certificate-alias-\\d+"))),
() -> assertAll(IntStream.range(0, aliasCount).mapToObj(defaultAliases::get).map(alias -> () -> {
List<String> otherAliases = defaultAliases.stream().filter(a -> !alias.equals(a)).collect(toList());
assertThat("other alises than '" + alias + "'", otherAliases, hasSize(aliasCount - 1));
})));
}

@Test
void alwaysGeneratesAnAlias() {
qt()
.forAll(strings().allPossible().ofLengthBetween(0, 100))
.checkAssert(s -> assertThat(s, where(TrustStoreLoader::generateAlias, notNullValue())));
}


}


private static Matcher<KeyStore> containsExactlyTheAliases(String ... certAliases) {
return new TypeSafeDiagnosingMatcher<KeyStore>() {
Expand Down

0 comments on commit b5681e0

Please sign in to comment.