Skip to content

Commit

Permalink
feat(keys): add functionality to extract public key to KeyParsers
Browse files Browse the repository at this point in the history
  • Loading branch information
paullatzelsperger committed Jul 12, 2024
1 parent 38728e0 commit a885770
Show file tree
Hide file tree
Showing 10 changed files with 165 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ void localPublicKeyService_withValueConfig(LocalPublicKeyDefaultExtension extens
"key1.id", "key1",
"key1.value", "value");

when(keyParserRegistry.parse("value")).thenReturn(Result.success(mock(PublicKey.class)));
when(keyParserRegistry.parsePublic("value")).thenReturn(Result.success(mock(PublicKey.class)));
when(context.getConfig(EDC_PUBLIC_KEYS_PREFIX)).thenReturn(ConfigFactory.fromMap(keys));
var localPublicKeyService = extension.localPublicKeyService();
extension.initialize(context);
Expand All @@ -75,7 +75,7 @@ void localPublicKeyService_withPathConfig(LocalPublicKeyDefaultExtension extensi
"key1.id", "key1",
"key1.path", path.getPath());

when(keyParserRegistry.parse(value)).thenReturn(Result.success(mock(PublicKey.class)));
when(keyParserRegistry.parsePublic(value)).thenReturn(Result.success(mock(PublicKey.class)));
when(context.getConfig(EDC_PUBLIC_KEYS_PREFIX)).thenReturn(ConfigFactory.fromMap(keys));
var localPublicKeyService = extension.localPublicKeyService();
extension.initialize(context);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,12 @@ public Result<Key> parse(String encoded) {
.map(kp -> kp.parse(encoded))
.orElseGet(() -> Result.failure("No parser found that can handle that format."));
}

@Override
public Result<Key> parsePublic(String encoded) {
return parsers.stream().filter(kp -> kp.canHandle(encoded))
.findFirst()
.map(kp -> kp.parsePublic(encoded))
.orElseGet(() -> Result.failure("No parser found that can handle that format."));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ public Result<PublicKey> resolveKey(String id) {
.orElseGet(() -> Result.failure("No public key could be resolved for key-ID '%s'".formatted(id)));
}

public Result<Void> addRawKey(String id, String rawKey) {
return parseKey(rawKey).onSuccess((pk) -> cachedKeys.put(id, pk)).mapEmpty();
}

private Optional<String> resolveFromVault(String id) {
return Optional.ofNullable(vault.resolveSecret(id));
}
Expand All @@ -57,16 +61,12 @@ private Optional<PublicKey> resolveFromCache(String id) {
}

private Result<PublicKey> parseKey(String encodedKey) {
return registry.parse(encodedKey).compose(pk -> {
return registry.parsePublic(encodedKey).compose(pk -> {
if (pk instanceof PublicKey publicKey) {
return Result.success(publicKey);
} else {
return Result.failure("The specified resource did not contain public key material.");
}
});
}

public Result<Void> addRawKey(String id, String rawKey) {
return parseKey(rawKey).onSuccess((pk) -> cachedKeys.put(id, pk)).mapTo();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,9 @@ public boolean canHandle(String encoded) {
@Override
public Result<Key> parse(String encoded) {
try {
var jwk = JWK.parse(encoded);


var jwk = JWK.parse(encoded);
// OctetKeyPairs (OKP) need special handling, as they can't be easily converted to a Java PrivateKey
if (jwk instanceof OctetKeyPair okp) {
return parseOctetKeyPair(okp).map(key -> key);
Expand All @@ -107,6 +108,26 @@ public Result<Key> parse(String encoded) {
}
}

@Override
public Result<Key> parsePublic(String encoded) {
try {
var jwk = JWK.parse(encoded).toPublicJWK();
// OctetKeyPairs (OKP) need special handling, as they can't be easily converted to a Java PrivateKey
if (jwk instanceof OctetKeyPair okp) {
return parseOctetKeyPair(okp.toPublicJWK()).map(key -> key);
}
var list = KeyConverter.toJavaKeys(List.of(jwk)); // contains an entry each for public and private key

return list.stream()
.findFirst()
.map(Result::success)
.orElse(Result.failure(ERROR_NO_KEY));
} catch (ParseException | NoSuchAlgorithmException | IOException | InvalidKeySpecException e) {
monitor.warning("Parser error", e);
return Result.failure("Parser error: " + e.getMessage());
}
}

private Result<? extends Key> parseOctetKeyPair(OctetKeyPair okp) throws NoSuchAlgorithmException, IOException, InvalidKeySpecException {
var d = okp.getDecodedD();
var x = okp.getDecodedX();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,7 @@ public boolean canHandle(String encoded) {
@Override
public Result<Key> parse(String encoded) {

var matcher = PEM_FORMAT_REGEX.matcher(encoded);
if (!matcher.find()) {
return Result.failure("The given input is not valid PEM.");
}
var keypair = parseKeys(encoded);
var keypair = parsePem(encoded);

if (keypair.succeeded()) {

Expand All @@ -83,7 +79,38 @@ public Result<Key> parse(String encoded) {
.orElseGet(() -> Result.failure("PEM-encoded structure did not contain a private key."));
}

return keypair.mapTo();
return keypair.mapEmpty();
}

@Override
public Result<Key> parsePublic(String encoded) {

var keypair = parsePem(encoded);
if (keypair.succeeded()) {

var keyPairList = keypair.getContent();
if (keyPairList.size() > 1) {
monitor.warning("PEM expected to contain exactly 1 key(-pair), but contained %s. Will take the first one. Please consider re-structuring your PEM document.".formatted(keyPairList.size()));
}
return keyPairList
.stream()
.filter(Objects::nonNull) // PEM strings that only contain public keys would get eliminated here
.map(keyPair -> (Key) keyPair.getPublic())
.filter(Objects::nonNull)
.findFirst()
.map(Result::success)
.orElseGet(() -> Result.failure("PEM-encoded structure did not contain a public key."));
}

return keypair.mapEmpty();
}

private Result<List<KeyPair>> parsePem(String pemEncoded) {
var matcher = PEM_FORMAT_REGEX.matcher(pemEncoded);
if (!matcher.find()) {
return Result.failure("The given input is not valid PEM.");
}
return parseKeys(pemEncoded);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@

class LocalPublicKeyServiceImplTest {

private LocalPublicKeyServiceImpl localPublicKeyService;
private final Vault vault = mock();
private final KeyParserRegistry keyParserRegistry = mock();
private LocalPublicKeyServiceImpl localPublicKeyService;

@BeforeEach
void setup() {
Expand All @@ -42,7 +42,7 @@ void setup() {

@Test
void resolve_withCache() {
when(keyParserRegistry.parse("value")).thenReturn(Result.success(mock(PublicKey.class)));
when(keyParserRegistry.parsePublic("value")).thenReturn(Result.success(mock(PublicKey.class)));
localPublicKeyService.addRawKey("id", "value");
AbstractResultAssert.assertThat(localPublicKeyService.resolveKey("id")).isSucceeded();
Mockito.verifyNoInteractions(vault);
Expand All @@ -51,7 +51,7 @@ void resolve_withCache() {
@Test
void resolve_withVault() {
when(vault.resolveSecret("id")).thenReturn("value");
when(keyParserRegistry.parse("value")).thenReturn(Result.success(mock(PublicKey.class)));
when(keyParserRegistry.parsePublic("value")).thenReturn(Result.success(mock(PublicKey.class)));
AbstractResultAssert.assertThat(localPublicKeyService.resolveKey("id")).isSucceeded();

verify(vault).resolveSecret("id");
Expand All @@ -66,7 +66,7 @@ void resolve_notFound() {
@Test
void resolve_wrongKeyType() {
when(vault.resolveSecret("id")).thenReturn("value");
when(keyParserRegistry.parse("value")).thenReturn(Result.success(mock(PrivateKey.class)));
when(keyParserRegistry.parsePublic("value")).thenReturn(Result.success(mock(PrivateKey.class)));

AbstractResultAssert.assertThat(localPublicKeyService.resolveKey("id")).isFailed();
verify(vault).resolveSecret("id");
Expand All @@ -75,7 +75,7 @@ void resolve_wrongKeyType() {
@Test
void resolve_wrongKeyFormat() {
when(vault.resolveSecret("id")).thenReturn("value");
when(keyParserRegistry.parse("value")).thenReturn(Result.failure("failure"));
when(keyParserRegistry.parsePublic("value")).thenReturn(Result.failure("failure"));

AbstractResultAssert.assertThat(localPublicKeyService.resolveKey("id")).isFailed();
verify(vault).resolveSecret("id");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,23 @@ void parse_publicKey(JWK jwk) {
assertThat(result).isSucceeded().isInstanceOf(PublicKey.class);
}


@ParameterizedTest
@ArgumentsSource(KeyProvider.class)
void parsePublic_withPublicKey(JWK jwk) {
var publickey = jwk.toPublicJWK();
var result = parser.parsePublic(publickey.toJSONString());
assertThat(result).isSucceeded().isInstanceOf(PublicKey.class);
}


@ParameterizedTest
@ArgumentsSource(KeyProvider.class)
void parsePublic_withPrivateKey(JWK jwk) {
var result = parser.parsePublic(jwk.toJSONString());
assertThat(result).isSucceeded().isInstanceOf(PublicKey.class);
}

private static class KeyProvider implements ArgumentsProvider {
@Override
public Stream<? extends Arguments> provideArguments(ExtensionContext context) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.assertj.core.api.Assertions;
import org.eclipse.edc.junit.testfixtures.TestUtils;
import org.junit.jupiter.api.Named;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
Expand Down Expand Up @@ -74,6 +75,38 @@ void parse_publicKey(String pem) {
.isInstanceOf(PublicKey.class);
}


@ParameterizedTest
@ArgumentsSource(PemConvertiblePrivateKeyProvider.class)
void parsePublic_withPrivateKey(String pem) {
var result = parser.parsePublic(pem);
assertThat(result)
.isSucceeded()
.isNotNull()
.isInstanceOf(PublicKey.class);

}

@Test
void parsePublic_withPrivateKey_whenEd25519_shouldFail() {
var pem = TestUtils.getResourceFileContentAsString("ed25519.pem");
var result = parser.parsePublic(pem);
assertThat(result)
.isFailed()
.detail().isEqualTo("PEM-encoded structure did not contain a public key.");
}


@ParameterizedTest
@ArgumentsSource(PemPublicKeyProvider.class)
void parsePublic_withPublicKey(String pem) {

assertThat(parser.parsePublic(pem))
.isSucceeded()
.isNotNull()
.isInstanceOf(PublicKey.class);
}

private static class PemPrivateKeyProvider implements ArgumentsProvider {

@Override
Expand All @@ -88,6 +121,19 @@ public Stream<? extends Arguments> provideArguments(ExtensionContext context) {
}
}

private static class PemConvertiblePrivateKeyProvider implements ArgumentsProvider {

@Override
public Stream<? extends Arguments> provideArguments(ExtensionContext context) {
return Stream.of(
Arguments.of(Named.named("RSA PrivateKey", TestUtils.getResourceFileContentAsString("rsa_2048.pem"))),
Arguments.of(Named.named("EC PrivateKey (P256)", TestUtils.getResourceFileContentAsString("ec_p256.pem"))),
Arguments.of(Named.named("EC PrivateKey (P384)", TestUtils.getResourceFileContentAsString("ec_p384.pem"))),
Arguments.of(Named.named("EC PrivateKey (P512)", TestUtils.getResourceFileContentAsString("ec_p512.pem")))
);
}
}

private static class PemPublicKeyProvider implements ArgumentsProvider {

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,20 @@ public interface KeyParser {
* @return Either a {@link java.security.PrivateKey}, a {@link java.security.PublicKey} or a failure.
*/
Result<Key> parse(String encoded);

/**
* Parses the encoded key as public key. If the encoded string is invalid, or the parser can't handle the input,
* it must return a {@link Result#failure(String)}, it must never throw an exception.
* <p>
* If the given key material contains public and private key data, the parser attempts to remove the private key data,
* returning only the public part of the key as {@link java.security.PublicKey}.
* If the given key material does not contain private key data, just public key data, returns a {@link java.security.PublicKey}. In all
* other cases, a {@link Result#failure(String)} is returned, for example, when a private key cannot be converted into a public key.
*
* @param encoded serialized/encoded key material.
* @return Either a {@link java.security.PublicKey} or a failure.
*/
default Result<Key> parsePublic(String encoded) {
return parse(encoded);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import org.eclipse.edc.spi.result.Result;

import java.security.Key;
import java.security.PrivateKey;

/**
* Registry that holds multiple {@link KeyParser} instances that are used to deserialize a private key from their
Expand All @@ -30,11 +29,21 @@ public interface KeyParserRegistry {
void register(KeyParser parser);

/**
* Attempts to parse the String representation of a private key into a {@link PrivateKey}. If no parser can handle
* Attempts to parse the String representation of a private key into a {@link Key}. If no parser can handle
* the encoded format, or it is corrupt etc. then a failure is returned.
*
* @param encoded The private key in encoded format (PEM, OpenSSH, JWK, PKCS8,...)
* @return a success result containing the private key, a failure if the encoded private key could not be deserialized.
*/
Result<Key> parse(String encoded);

/**
* Attempts to parse the String representation of a public or private key into a {@link Key}. If no parser can handle
* the encoded format, if the encoded format contains a private key that cannot be converted to a public key,
* or if the input is corrupt etc. then a failure is returned.
*
* @param encoded The private key in encoded format (PEM, OpenSSH, JWK, PKCS8,...)
* @return a success result containing the public key, a failure if the encoded public key could not be deserialized.
*/
Result<Key> parsePublic(String encoded);
}

0 comments on commit a885770

Please sign in to comment.