Load algorithm parameters from PEM encoded 'EC PRIVATE KEY' files

Update `PrivateKeyParser` implementations so that algorithm parameters
for PEM encoded 'EC PRIVATE KEY' files are loaded from the incoming
data.

Prior to this commit, the algorithm parameter was hard-coded to
'secp384r1' which could result in a mismatch to the actual file
content.

Fixes gh-34232

spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/ssl/PrivateKeyParser.java

@@ -18,6 +18,7 @@ package org.springframework.boot.buildpack.platform.docker.ssl;
 
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
+import java.nio.ByteBuffer;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -33,6 +34,9 @@ import java.util.function.Function;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import org.springframework.boot.buildpack.platform.docker.ssl.PrivateKeyParser.DerElement.TagType;
+import org.springframework.boot.buildpack.platform.docker.ssl.PrivateKeyParser.DerElement.ValueType;
+import org.springframework.util.Assert;
 import org.springframework.util.Base64Utils;
 
 /**
@@ -90,7 +94,38 @@ final class PrivateKeyParser {
 	}
 
 	private static PKCS8EncodedKeySpec createKeySpecForEc(byte[] bytes) {
-		return createKeySpecForAlgorithm(bytes, EC_ALGORITHM, EC_PARAMETERS);
+		DerElement ecPrivateKey = DerElement.of(bytes);
+		Assert.state(ecPrivateKey.isType(ValueType.ENCODED, TagType.SEQUENCE),
+				"Key spec should be an ASN.1 encoded sequence");
+		DerElement version = DerElement.of(ecPrivateKey.getContents());
+		Assert.state(version != null && version.isType(ValueType.PRIMITIVE, TagType.INTEGER),
+				"Key spec should start with version");
+		Assert.state(version.getContents().remaining() == 1 && version.getContents().get() == 1,
+				"Key spec version must be 1");
+		DerElement privateKey = DerElement.of(ecPrivateKey.getContents());
+		Assert.state(privateKey != null && privateKey.isType(ValueType.PRIMITIVE, TagType.OCTET_STRING),
+				"Key spec should contain private key");
+		DerElement parameters = DerElement.of(ecPrivateKey.getContents());
+		return createKeySpecForAlgorithm(bytes, EC_ALGORITHM, getEcParameters(parameters));
+	}
+
+	private static int[] getEcParameters(DerElement parameters) {
+		if (parameters == null) {
+			return EC_PARAMETERS;
+		}
+		Assert.state(parameters.isType(ValueType.ENCODED), "Key spec should contain encoded parameters");
+		DerElement contents = DerElement.of(parameters.getContents());
+		Assert.state(contents.isType(ValueType.PRIMITIVE, TagType.OBJECT_IDENTIFIER),
+				"Key spec parameters should contain object identifier");
+		return getEcParameters(contents.getContents());
+	}
+
+	private static int[] getEcParameters(ByteBuffer bytes) {
+		int[] result = new int[bytes.remaining()];
+		for (int i = 0; i < result.length; i++) {
+			result[i] = bytes.get() & 0xFF;
+		}
+		return result;
 	}
 
 	private static PKCS8EncodedKeySpec createKeySpecForAlgorithm(byte[] bytes, int[] algorithm, int[] parameters) {
@@ -258,4 +293,102 @@ final class PrivateKeyParser {
 
 	}
 
+	/**
+	 * An ASN.1 DER encoded element.
+	 */
+	static final class DerElement {
+
+		private final ValueType valueType;
+
+		private final long tagType;
+
+		private ByteBuffer contents;
+
+		private DerElement(ByteBuffer bytes) {
+			byte b = bytes.get();
+			this.valueType = ((b & 0x20) == 0) ? ValueType.PRIMITIVE : ValueType.ENCODED;
+			this.tagType = decodeTagType(b, bytes);
+			int length = decodeLength(bytes);
+			bytes.limit(bytes.position() + length);
+			this.contents = bytes.slice();
+			bytes.limit(bytes.capacity());
+			bytes.position(bytes.position() + length);
+		}
+
+		private long decodeTagType(byte b, ByteBuffer bytes) {
+			long tagType = (b & 0x1F);
+			if (tagType != 0x1F) {
+				return tagType;
+			}
+			tagType = 0;
+			b = bytes.get();
+			while ((b & 0x80) != 0) {
+				tagType <<= 7;
+				tagType = tagType | (b & 0x7F);
+				b = bytes.get();
+			}
+			return tagType;
+		}
+
+		private int decodeLength(ByteBuffer bytes) {
+			byte b = bytes.get();
+			if ((b & 0x80) == 0) {
+				return b & 0x7F;
+			}
+			int numberOfLengthBytes = (b & 0x7F);
+			Assert.state(numberOfLengthBytes != 0, "Infinite length encoding is not supported");
+			Assert.state(numberOfLengthBytes != 0x7F, "Reserved length encoding is not supported");
+			Assert.state(numberOfLengthBytes <= 4, "Length overflow");
+			int length = 0;
+			for (int i = 0; i < numberOfLengthBytes; i++) {
+				length <<= 8;
+				length |= (bytes.get() & 0xFF);
+			}
+			return length;
+		}
+
+		boolean isType(ValueType valueType) {
+			return this.valueType == valueType;
+		}
+
+		boolean isType(ValueType valueType, TagType tagType) {
+			return this.valueType == valueType && this.tagType == tagType.getNumber();
+		}
+
+		ByteBuffer getContents() {
+			return this.contents;
+		}
+
+		static DerElement of(byte[] bytes) {
+			return of(ByteBuffer.wrap(bytes));
+		}
+
+		static DerElement of(ByteBuffer bytes) {
+			return (bytes.remaining() > 0) ? new DerElement(bytes) : null;
+		}
+
+		enum ValueType {
+
+			PRIMITIVE, ENCODED
+
+		}
+
+		enum TagType {
+
+			INTEGER(0x02), OCTET_STRING(0x04), OBJECT_IDENTIFIER(0x06), SEQUENCE(0x10);
+
+			private final int number;
+
+			TagType(int number) {
+				this.number = number;
+			}
+
+			int getNumber() {
+				return this.number;
+			}
+
+		}
+
+	}
+
 }

spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/ssl/PemFileWriter.java

@@ -82,7 +82,13 @@ public class PemFileWriter {
 			+ "wSKcVTdEdO2DhBlYddN0zG0rjq4vDMtdmldEl4BdldQ=\n" + "-----END RSA PRIVATE KEY-----\n";
 
 	public static final String PRIVATE_EC_KEY = EXAMPLE_SECRET_QUALIFIER + "-----BEGIN EC PRIVATE KEY-----\n"
-			+ "MHcCAQEEIIwZkO8Zjbggzi8wwrk5rzSPzUX31gqTRhBYw4AL6w44oAoGCCqGSM49\n"
+			+ "MIGkAgEBBDB21WGGOb1DokKW0MUHO7RQ6jZSUYXfO2iyfCbjmSJhyK8fSuq1V0N2\n"
+			+ "Bj7X+XYhS6ygBwYFK4EEACKhZANiAATsRaYri/tDMvrrB2NJlxWFOZ4YBLYdSM+a\n"
+			+ "FlGh1FuLjOHW9cx8w0iRHd1Hxn4sxqsa62KzGoCj63lGoaJgi67YNCF0lBa/zCLy\n"
+			+ "ktaMsQePDOR8UR0Cfi2J9bh+IjxXd+o=\n" + "-----END EC PRIVATE KEY-----";
+
+	public static final String PRIVATE_EC_KEY_PRIME_256_V1 = EXAMPLE_SECRET_QUALIFIER
+			+ "-----BEGIN EC PRIVATE KEY-----\n" + "MHcCAQEEIIwZkO8Zjbggzi8wwrk5rzSPzUX31gqTRhBYw4AL6w44oAoGCCqGSM49\n"
 			+ "AwEHoUQDQgAE8y28khug747bA68M90IAMCPHAYyen+RsN6i84LORpNDUhv00QZWd\n"
 			+ "hOhjWFCQjnewR98Y8pEb1fnORll4LhHPlQ==\n" + "-----END EC PRIVATE KEY-----";
 

spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/ssl/PrivateKeyParserTests.java

@@ -22,6 +22,7 @@ import java.net.URISyntaxException;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.security.PrivateKey;
+import java.security.interfaces.ECPrivateKey;
 
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
@@ -89,17 +90,27 @@ class PrivateKeyParserTests {
 		Path path = this.fileWriter.writeFile("key.pem", PemFileWriter.PRIVATE_RSA_KEY);
 		PrivateKey privateKey = PrivateKeyParser.parse(path);
 		assertThat(privateKey).isNotNull();
-		// keys in PKCS#1 format are converted to PKCS#8 for parsing
 		assertThat(privateKey.getFormat()).isEqualTo("PKCS#8");
 	}
 
 	@Test
-	void parsePkcs1EcKeyFile() throws IOException {
+	void parsePemEcKeyFile() throws IOException {
 		Path path = this.fileWriter.writeFile("key.pem", PemFileWriter.PRIVATE_EC_KEY);
-		PrivateKey privateKey = PrivateKeyParser.parse(path);
+		ECPrivateKey privateKey = (ECPrivateKey) PrivateKeyParser.parse(path);
 		assertThat(privateKey).isNotNull();
-		// keys in PKCS#1 format are converted to PKCS#8 for parsing
 		assertThat(privateKey.getFormat()).isEqualTo("PKCS#8");
+		assertThat(privateKey.getAlgorithm()).isEqualTo("EC");
+		assertThat(privateKey.getParams().toString()).contains("1.3.132.0.34").doesNotContain("prime256v1");
+	}
+
+	@Test
+	void parsePemEcKeyFilePrime256v1() throws IOException {
+		Path path = this.fileWriter.writeFile("key.pem", PemFileWriter.PRIVATE_EC_KEY_PRIME_256_V1);
+		ECPrivateKey privateKey = (ECPrivateKey) PrivateKeyParser.parse(path);
+		assertThat(privateKey).isNotNull();
+		assertThat(privateKey.getFormat()).isEqualTo("PKCS#8");
+		assertThat(privateKey.getAlgorithm()).isEqualTo("EC");
+		assertThat(privateKey.getParams().toString()).contains("prime256v1").doesNotContain("1.3.132.0.34");
 	}
 
 	@Test

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/server/PrivateKeyParser.java

@@ -21,6 +21,7 @@ import java.io.IOException;
 import java.io.InputStreamReader;
 import java.io.Reader;
 import java.net.URL;
+import java.nio.ByteBuffer;
 import java.security.GeneralSecurityException;
 import java.security.KeyFactory;
 import java.security.PrivateKey;
@@ -33,6 +34,9 @@ import java.util.function.Function;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import org.springframework.boot.web.server.PrivateKeyParser.DerElement.TagType;
+import org.springframework.boot.web.server.PrivateKeyParser.DerElement.ValueType;
+import org.springframework.util.Assert;
 import org.springframework.util.Base64Utils;
 import org.springframework.util.FileCopyUtils;
 import org.springframework.util.ResourceUtils;
@@ -92,7 +96,38 @@ final class PrivateKeyParser {
 	}
 
 	private static PKCS8EncodedKeySpec createKeySpecForEc(byte[] bytes) {
-		return createKeySpecForAlgorithm(bytes, EC_ALGORITHM, EC_PARAMETERS);
+		DerElement ecPrivateKey = DerElement.of(bytes);
+		Assert.state(ecPrivateKey.isType(ValueType.ENCODED, TagType.SEQUENCE),
+				"Key spec should be an ASN.1 encoded sequence");
+		DerElement version = DerElement.of(ecPrivateKey.getContents());
+		Assert.state(version != null && version.isType(ValueType.PRIMITIVE, TagType.INTEGER),
+				"Key spec should start with version");
+		Assert.state(version.getContents().remaining() == 1 && version.getContents().get() == 1,
+				"Key spec version must be 1");
+		DerElement privateKey = DerElement.of(ecPrivateKey.getContents());
+		Assert.state(privateKey != null && privateKey.isType(ValueType.PRIMITIVE, TagType.OCTET_STRING),
+				"Key spec should contain private key");
+		DerElement parameters = DerElement.of(ecPrivateKey.getContents());
+		return createKeySpecForAlgorithm(bytes, EC_ALGORITHM, getEcParameters(parameters));
+	}
+
+	private static int[] getEcParameters(DerElement parameters) {
+		if (parameters == null) {
+			return EC_PARAMETERS;
+		}
+		Assert.state(parameters.isType(ValueType.ENCODED), "Key spec should contain encoded parameters");
+		DerElement contents = DerElement.of(parameters.getContents());
+		Assert.state(contents.isType(ValueType.PRIMITIVE, TagType.OBJECT_IDENTIFIER),
+				"Key spec parameters should contain object identifier");
+		return getEcParameters(contents.getContents());
+	}
+
+	private static int[] getEcParameters(ByteBuffer bytes) {
+		int[] result = new int[bytes.remaining()];
+		for (int i = 0; i < result.length; i++) {
+			result[i] = bytes.get() & 0xFF;
+		}
+		return result;
 	}
 
 	private static PKCS8EncodedKeySpec createKeySpecForAlgorithm(byte[] bytes, int[] algorithm, int[] parameters) {
@@ -102,8 +137,7 @@ final class PrivateKeyParser {
 			DerEncoder algorithmIdentifier = new DerEncoder();
 			algorithmIdentifier.objectIdentifier(algorithm);
 			algorithmIdentifier.objectIdentifier(parameters);
-			byte[] byteArray = algorithmIdentifier.toByteArray();
+			encoder.sequence(algorithmIdentifier.toByteArray());
-			encoder.sequence(byteArray);
 			encoder.octetString(bytes);
 			return new PKCS8EncodedKeySpec(encoder.toSequence());
 		}
@@ -262,4 +296,102 @@ final class PrivateKeyParser {
 
 	}
 
+	/**
+	 * An ASN.1 DER encoded element.
+	 */
+	static final class DerElement {
+
+		private final ValueType valueType;
+
+		private final long tagType;
+
+		private ByteBuffer contents;
+
+		private DerElement(ByteBuffer bytes) {
+			byte b = bytes.get();
+			this.valueType = ((b & 0x20) == 0) ? ValueType.PRIMITIVE : ValueType.ENCODED;
+			this.tagType = decodeTagType(b, bytes);
+			int length = decodeLength(bytes);
+			bytes.limit(bytes.position() + length);
+			this.contents = bytes.slice();
+			bytes.limit(bytes.capacity());
+			bytes.position(bytes.position() + length);
+		}
+
+		private long decodeTagType(byte b, ByteBuffer bytes) {
+			long tagType = (b & 0x1F);
+			if (tagType != 0x1F) {
+				return tagType;
+			}
+			tagType = 0;
+			b = bytes.get();
+			while ((b & 0x80) != 0) {
+				tagType <<= 7;
+				tagType = tagType | (b & 0x7F);
+				b = bytes.get();
+			}
+			return tagType;
+		}
+
+		private int decodeLength(ByteBuffer bytes) {
+			byte b = bytes.get();
+			if ((b & 0x80) == 0) {
+				return b & 0x7F;
+			}
+			int numberOfLengthBytes = (b & 0x7F);
+			Assert.state(numberOfLengthBytes != 0, "Infinite length encoding is not supported");
+			Assert.state(numberOfLengthBytes != 0x7F, "Reserved length encoding is not supported");
+			Assert.state(numberOfLengthBytes <= 4, "Length overflow");
+			int length = 0;
+			for (int i = 0; i < numberOfLengthBytes; i++) {
+				length <<= 8;
+				length |= (bytes.get() & 0xFF);
+			}
+			return length;
+		}
+
+		boolean isType(ValueType valueType) {
+			return this.valueType == valueType;
+		}
+
+		boolean isType(ValueType valueType, TagType tagType) {
+			return this.valueType == valueType && this.tagType == tagType.getNumber();
+		}
+
+		ByteBuffer getContents() {
+			return this.contents;
+		}
+
+		static DerElement of(byte[] bytes) {
+			return of(ByteBuffer.wrap(bytes));
+		}
+
+		static DerElement of(ByteBuffer bytes) {
+			return (bytes.remaining() > 0) ? new DerElement(bytes) : null;
+		}
+
+		enum ValueType {
+
+			PRIMITIVE, ENCODED
+
+		}
+
+		enum TagType {
+
+			INTEGER(0x02), OCTET_STRING(0x04), OBJECT_IDENTIFIER(0x06), SEQUENCE(0x10);
+
+			private final int number;
+
+			TagType(int number) {
+				this.number = number;
+			}
+
+			int getNumber() {
+				return this.number;
+			}
+
+		}
+
+	}
+
 }

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/server/PrivateKeyParserTests.java

@@ -17,6 +17,7 @@
 package org.springframework.boot.web.server;
 
 import java.security.PrivateKey;
+import java.security.interfaces.ECPrivateKey;
 
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
@@ -30,6 +31,7 @@ import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
  *
  * @author Scott Frederick
  * @author Moritz Halbritter
+ * @author Phillip Webb
  */
 class PrivateKeyParserTests {
 
@@ -60,11 +62,21 @@ class PrivateKeyParserTests {
 	}
 
 	@Test
-	void parsePkcs8KeyFileWithEcdsa() {
+	void parsePemKeyFileWithEcdsa() {
-		PrivateKey privateKey = PrivateKeyParser.parse("classpath:test-ec-key.pem");
+		ECPrivateKey privateKey = (ECPrivateKey) PrivateKeyParser.parse("classpath:test-ec-key.pem");
 		assertThat(privateKey).isNotNull();
 		assertThat(privateKey.getFormat()).isEqualTo("PKCS#8");
 		assertThat(privateKey.getAlgorithm()).isEqualTo("EC");
+		assertThat(privateKey.getParams().toString()).contains("1.3.132.0.34").doesNotContain("prime256v1");
+	}
+
+	@Test
+	void parsePemKeyFileWithEcdsaPrime256v1() {
+		ECPrivateKey privateKey = (ECPrivateKey) PrivateKeyParser.parse("classpath:test-ec-key-prime256v1.pem");
+		assertThat(privateKey).isNotNull();
+		assertThat(privateKey.getFormat()).isEqualTo("PKCS#8");
+		assertThat(privateKey.getAlgorithm()).isEqualTo("EC");
+		assertThat(privateKey.getParams().toString()).contains("prime256v1").doesNotContain("1.3.132.0.34");
 	}
 
 	@Test

spring-boot-project/spring-boot/src/test/resources/test-ec-key-prime256v1.pem

@@ -0,0 +1,5 @@
+-----BEGIN EC PRIVATE KEY-----
+MHcCAQEEINa5DtcVrTiC4pMDX+rnMbhIlxm26Rn5FYz2+uT5DmBcoAoGCCqGSM49
+AwEHoUQDQgAEj0M8x6W5fF5y5tdHvFCp0ws8gCOcETQk52uXSL46G3Uukng6lscf
+yNobOV+NjmBCqJRZd0bKEvbIiMvpo6B0Fw==
+-----END EC PRIVATE KEY-----

spring-boot-project/spring-boot/src/test/resources/test-ec-key.pem

@@ -1,5 +1,6 @@
 -----BEGIN EC PRIVATE KEY-----
-MHcCAQEEIBEZhSR+d8kwL5L/K0f/eNBm4RfzyyA1jfg+dV1/8WvqoAoGCCqGSM49
+MIGkAgEBBDB21WGGOb1DokKW0MUHO7RQ6jZSUYXfO2iyfCbjmSJhyK8fSuq1V0N2
-AwEHoUQDQgAEBbfdBTSUWuui7O2R+W9mDPjAHjgdBJsjrjnvkjnq8f/k4U/OqvjK
+Bj7X+XYhS6ygBwYFK4EEACKhZANiAATsRaYri/tDMvrrB2NJlxWFOZ4YBLYdSM+a
-qnHEZwYgdaF2WqYdqBYMns0n+tSMgBoonQ==
+FlGh1FuLjOHW9cx8w0iRHd1Hxn4sxqsa62KzGoCj63lGoaJgi67YNCF0lBa/zCLy
+ktaMsQePDOR8UR0Cfi2J9bh+IjxXd+o=
 -----END EC PRIVATE KEY-----

src/checkstyle/import-control.xml

@@ -93,6 +93,7 @@
 		</subpackage>
 		<subpackage name="server">
 			<disallow pkg="org.springframework.context" />
+			<allow pkg="org.springframework.boot.web.server"/>
 			<subpackage name="context">
 				<allow pkg="org.springframework.context" />
 			</subpackage>