From 00c1f350376b76740444a986e07f6deb15513e60 Mon Sep 17 00:00:00 2001 From: SylvainJuge <763082+SylvainJuge@users.noreply.github.com> Date: Tue, 6 Feb 2024 10:04:14 +0100 Subject: [PATCH] Fix agent downloader with new signing key for 1.46.0 (#3513) --- CHANGELOG.asciidoc | 4 + .../elastic/apm/attach/AgentDownloader.java | 89 ++++++++++++++----- .../bouncycastle/BouncyCastleVerifier.java | 11 ++- .../resources/pub_key_8AB554FD8F207067.asc | 18 ++++ ...c_key.asc => pub_key_D27D666CD88E42B4.asc} | 0 .../apm/attach/AgentDownloaderTest.java | 30 +++++-- 6 files changed, 122 insertions(+), 30 deletions(-) create mode 100644 apm-agent-attach-cli/src/main/resources/pub_key_8AB554FD8F207067.asc rename apm-agent-attach-cli/src/main/resources/{public_key.asc => pub_key_D27D666CD88E42B4.asc} (100%) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 2a1ed3dcf0..dbd65b93a4 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -34,6 +34,10 @@ Use subheadings with the "=====" level for adding notes for unreleased changes: [[release-notes-1.x]] === Java Agent version 1.x +[float] +===== Bug fixes +* Add support to CLI attach download for new agent signature for 1.46.0+ - {pull}3513[#3513] + [[release-notes-1.46.0]] ==== 1.46.0 - 2024/01/29 diff --git a/apm-agent-attach-cli/src/main/java/co/elastic/apm/attach/AgentDownloader.java b/apm-agent-attach-cli/src/main/java/co/elastic/apm/attach/AgentDownloader.java index a1e7c7e66e..235fb3e548 100644 --- a/apm-agent-attach-cli/src/main/java/co/elastic/apm/attach/AgentDownloader.java +++ b/apm-agent-attach-cli/src/main/java/co/elastic/apm/attach/AgentDownloader.java @@ -23,6 +23,7 @@ import org.apache.logging.log4j.Logger; import java.io.BufferedReader; +import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; @@ -31,6 +32,8 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.Locale; +import java.util.HashMap; +import java.util.Map; import java.util.Properties; import java.util.TreeSet; import java.util.regex.Matcher; @@ -49,6 +52,8 @@ public class AgentDownloader { private static final String CLI_JAR_VERSION; public static final String USER_AGENT; + private final Map keys; + static { CLI_JAR_VERSION = readCliJarVersion(); StringBuilder userAgent = new StringBuilder("elastic-apm-agent-java-attach-cli"); @@ -84,6 +89,9 @@ public static String getCliJarVersion() { public AgentDownloader(PgpSignatureVerifier pgpSignatureVerifier) { this.pgpSignatureVerifier = pgpSignatureVerifier; + this.keys = new HashMap(); + keys.put("D27D666CD88E42B4", getPubKeyContent("/pub_key_D27D666CD88E42B4.asc", 1780)); + keys.put("8AB554FD8F207067", getPubKeyContent("/pub_key_8AB554FD8F207067.asc", 977)); } private final PgpSignatureVerifier pgpSignatureVerifier; @@ -91,6 +99,7 @@ public AgentDownloader(PgpSignatureVerifier pgpSignatureVerifier) { /** * Downloads the agent jar, authenticates and verifies its PGP signature and returns the path for the locally * stored jar. + * * @param agentVersion the target agent version * @return the path for the locally stored agent jar * @throws Exception failure either with downloading, copying to local file system, or in PGP signature verification @@ -181,8 +190,9 @@ private static HttpURLConnection openConnection(String urlString) throws IOExcep /** * Downloads a file from the provided URL into the provided local file path + * * @param remoteFileUrlString remote file URL as string - * @param localFilePath destination path for the dewnloaded file + * @param localFilePath destination path for the dewnloaded file * @throws IOException indicating a failure during class reading or writing, or the file already exists */ void downloadFile(String remoteFileUrlString, Path localFilePath) throws IOException { @@ -200,23 +210,47 @@ void downloadFile(String remoteFileUrlString, Path localFilePath) throws IOExcep * * @param agentJarFile the path to the downloaded agent jar * @param mavenAgentUrlString the maven base URL for the agent - * @throws Exception if an I/O exception occurs reading from various input streams + * @throws Exception if an I/O exception occurs reading from various input streams */ void verifyPgpSignature(final Path agentJarFile, final String mavenAgentUrlString) throws Exception { final String ascUrlString = mavenAgentUrlString + ".asc"; logger.info("Verifying Elastic APM Java Agent jar PGP signature..."); + HttpURLConnection signatureFileUrlConnection = openConnection(ascUrlString); - try ( - InputStream agentJarIS = Files.newInputStream(agentJarFile); - InputStream pgpSignatureIS = signatureFileUrlConnection.getInputStream(); - InputStream publicKeyIS = getPublicKey() - ) { - if (!pgpSignatureVerifier.verifyPgpSignature(agentJarIS, pgpSignatureIS, publicKeyIS, getPublicKeyId())) { - throw new IllegalStateException("Signature verification for " + mavenAgentUrlString + - " failed, downloaded jar may have been tampered with."); + int signatureLength = signatureFileUrlConnection.getContentLength(); + if (signatureLength <= 0) { + throw new IllegalStateException("unexpected signature size"); + } + + byte[] signatureBytes; + try (InputStream inputStream = signatureFileUrlConnection.getInputStream()) { + signatureBytes = toByteArray(inputStream, signatureLength); + } + + for (Map.Entry entry : getPublicKeys().entrySet()) { + try ( + InputStream agentJarIS = Files.newInputStream(agentJarFile); + ) { + InputStream pgpSignatureIS = new ByteArrayInputStream(signatureBytes); + logger.debug("attempt to verify with key [{}]", entry.getKey()); + InputStream publicKeyIS = new ByteArrayInputStream(entry.getValue()); + try { + if (pgpSignatureVerifier.verifyPgpSignature(agentJarIS, pgpSignatureIS, publicKeyIS, entry.getKey())) { + logger.info("Elastic APM Java Agent jar PGP signature successfully verified."); + return; + } else { + logger.debug("key verification failed with key [{}]", entry.getKey()); + } + } catch (Exception e) { + logger.debug(e.getMessage()); + } } + } - logger.info("Elastic APM Java Agent jar PGP signature successfully verified."); + throw new IllegalStateException("Signature verification for " + mavenAgentUrlString + " failed, downloaded jar may have been tampered with."); + + + } static String findLatestVersion() throws Exception { @@ -250,20 +284,31 @@ static TreeSet parseMavenMetadataXml(InputStream htmlInputStream) throw } /** - * Return the public key ID of our agent signing key. + * Returns the public keys used to sign agent artifacts * - * @return the public key ID + * @return map of signing keys, with key ID as key, and the raw public key value as value */ - String getPublicKeyId() { - return "D27D666CD88E42B4"; + public Map getPublicKeys() { + return keys; } - /** - * An input stream to the public key of the signing key. - * - * @return an input stream to the public key - */ - InputStream getPublicKey() { - return AgentDownloader.class.getResourceAsStream("/public_key.asc"); + private static byte[] getPubKeyContent(String path, int size) { + try (InputStream inputStream = AgentDownloader.class.getResourceAsStream(path)) { + if (inputStream == null) { + throw new IllegalStateException("unknown key file: " + path); + } + return toByteArray(inputStream, size); + } catch (IOException e) { + throw new IllegalStateException(e); + } } + + private static byte[] toByteArray(InputStream inputStream, int size) throws IOException { + byte[] result = new byte[size]; + if (size == 0 || size != inputStream.read(result) || inputStream.read() >= 0) { + throw new IllegalStateException("invalid input size" + size); + } + return result; + } + } diff --git a/apm-agent-attach-cli/src/main/java/co/elastic/apm/attach/bouncycastle/BouncyCastleVerifier.java b/apm-agent-attach-cli/src/main/java/co/elastic/apm/attach/bouncycastle/BouncyCastleVerifier.java index c76f253254..f1d5596941 100644 --- a/apm-agent-attach-cli/src/main/java/co/elastic/apm/attach/bouncycastle/BouncyCastleVerifier.java +++ b/apm-agent-attach-cli/src/main/java/co/elastic/apm/attach/bouncycastle/BouncyCastleVerifier.java @@ -53,7 +53,16 @@ public boolean verifyPgpSignature(InputStream toVerify, InputStream expectedPgpS InputStream rawPublicKey, String keyID) throws Exception { // read expected signature final JcaPGPObjectFactory factory = new JcaPGPObjectFactory(PGPUtil.getDecoderStream(expectedPgpSignature)); - final PGPSignature signature = ((PGPSignatureList) factory.nextObject()).get(0); + Object factoryObj = factory.nextObject(); + if(!(factoryObj instanceof PGPSignatureList)) { + throw new IllegalStateException("unexpected signature list"); + } + PGPSignatureList signatureList = (PGPSignatureList) factoryObj; + if (signatureList.size() != 1) { + throw new IllegalStateException("unexpected signature list size"); + } + + final PGPSignature signature = signatureList.get(0); // validate the signature has key ID matching our public key ID final String signatureKeyID = Long.toHexString(signature.getKeyID()).toUpperCase(Locale.ROOT); diff --git a/apm-agent-attach-cli/src/main/resources/pub_key_8AB554FD8F207067.asc b/apm-agent-attach-cli/src/main/resources/pub_key_8AB554FD8F207067.asc new file mode 100644 index 0000000000..1780fcf12d --- /dev/null +++ b/apm-agent-attach-cli/src/main/resources/pub_key_8AB554FD8F207067.asc @@ -0,0 +1,18 @@ +-----BEGIN PGP PUBLIC KEY BLOCK----- + +mQENBGWg4zoBCACh2Npi39oBcGn+pYm+ycxPBpUfs2Es0coHbSnbhMEcqyc9uMmE +hOwbD9p/I/5KjKBQn9FOWzshoARutAekn9rDZBjFjZFBaG8t9Gr2IHcET4BvmtjT +ShU3ekr8Xcl44j4X5IPeIa5y/TRTibEK9g+89f/9Ga0SlDuUDaJQVFClSVFhEACM +OiSY40E7sh9ZEwJjYOHz+BmKXDTBq5WYcmc8y9+sonwVGcwPDw2IRa3R0Fd1+hKH +2614Z+ILdzD3CP0Pla6tHjCE6v5mYNk9qUhTY2pvP+hxLfSNDBps0+tDdCWTcShq +gPXSDTraRMYVaat2P9OxQqoW8ZI+J1A6cViVABEBAAG0L2FwbS1yZWxlYXNlQGVs +YXN0aWMuY28gPGFwbS1yZWxlYXNlQGVsYXN0aWMuY28+iQFSBBMBCAA8FiEEbR1A +Z4ggBH/1xXydirVU/Y8gcGcFAmWg4zoDGy8EBQsJCAcCAiICBhUKCQgLAgQWAgMB +Ah4HAheAAAoJEIq1VP2PIHBnUC4H/i/nieduK/netXwOWXTHyaIlfetbJrZr/7LB +5Stj3KuA82kqrD+dgPQjQ2P2FRUrHhhjeUuS7DsyigT3JZdZQz6GLJA7ooWlsV9K +swCLEYrna/j/Nj3Flv6u33VQhRdHBTs+dcgqP3W2PrtKT7Zpji+pG6EDdxIgPocN +fqmUwyJ90dCJuhStQ3lO7Ky3f/4azGXdlQB3l2L0HuUWVB0SUS9eE/Ezv0cc7iuA +PWbWdOXh+X7KumxT482jvccNCisJL3xrUHtmEnhp2drCtoz41Kj7oZw0LU0mLDuB +/ZleA88yx2IbIx8lTNqF1Vt8qWGByegOzW6rG2/ksJLJNj7eCLo= +=TtVG +-----END PGP PUBLIC KEY BLOCK----- diff --git a/apm-agent-attach-cli/src/main/resources/public_key.asc b/apm-agent-attach-cli/src/main/resources/pub_key_D27D666CD88E42B4.asc similarity index 100% rename from apm-agent-attach-cli/src/main/resources/public_key.asc rename to apm-agent-attach-cli/src/main/resources/pub_key_D27D666CD88E42B4.asc diff --git a/apm-agent-attach-cli/src/test/java/co/elastic/apm/attach/AgentDownloaderTest.java b/apm-agent-attach-cli/src/test/java/co/elastic/apm/attach/AgentDownloaderTest.java index 6b4b2c0271..4d3711e3bc 100644 --- a/apm-agent-attach-cli/src/test/java/co/elastic/apm/attach/AgentDownloaderTest.java +++ b/apm-agent-attach-cli/src/test/java/co/elastic/apm/attach/AgentDownloaderTest.java @@ -21,12 +21,17 @@ import co.elastic.apm.agent.common.util.Version; import co.elastic.apm.attach.bouncycastle.BouncyCastleVerifier; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import java.io.IOException; +import java.io.InputStream; import java.nio.file.FileAlreadyExistsException; import java.nio.file.Files; import java.nio.file.Path; import java.util.Locale; +import java.util.Map; +import java.util.Objects; import java.util.TreeSet; import static org.assertj.core.api.Assertions.assertThat; @@ -36,6 +41,7 @@ class AgentDownloaderTest { + // valid key stored in valid_key.asc, but not the one used to sign agent artifacts private static final String TEST_KEY_ID = "90AD76CD56AA73A9"; private final AgentDownloader agentDownloader = new AgentDownloader(new BouncyCastleVerifier()); @@ -69,9 +75,12 @@ void testDownloadFile() throws Exception { assertThatThrownBy(() -> agentDownloader.downloadFile(mavenPgpSignatureUrl, localFilePath)).isInstanceOf(FileAlreadyExistsException.class); } - @Test - void testDownloadAndVerifyAgent() throws Exception { - String agentVersion = AgentDownloader.findLatestVersion(); + @ParameterizedTest + @ValueSource(strings = {"1.24.0", "1.46.0", "latest"}) + void testDownloadAndVerifyAgent(String agentVersion) throws Exception { + if ("latest".equals(agentVersion)) { + agentVersion = AgentDownloader.findLatestVersion(); + } Path targetDir = AgentDownloadUtils.of(agentVersion).getTargetAgentDir(); final Path localAgentPath = targetDir.resolve(agentDownloader.computeAgentJarName(agentVersion)); System.out.println("localAgentPath = " + localAgentPath); @@ -81,11 +90,18 @@ void testDownloadAndVerifyAgent() throws Exception { assertThat(Files.isReadable(localAgentPath)).isTrue(); } - @Test - void testDownloadAgentAndFailVerification() throws Exception { + @ParameterizedTest + @ValueSource(strings = {"1.24.0", "1.46.0"}) + void testDownloadAgentAndFailVerification(String agentVersion) throws Exception { AgentDownloader spyAgentDownloader = spy(new AgentDownloader(new BouncyCastleVerifier())); - doReturn(TEST_KEY_ID).when(spyAgentDownloader).getPublicKeyId(); - String agentVersion = "1.24.0"; + + // using invalid key ID but valid key value + byte[] key; + try (InputStream is = Objects.requireNonNull(AgentDownloaderTest.class.getResourceAsStream("/valid_key.asc"))) { + key = is.readAllBytes(); + } + doReturn(Map.of(TEST_KEY_ID, key)).when(spyAgentDownloader).getPublicKeys(); + Path targetDir = AgentDownloadUtils.of(agentVersion).getTargetAgentDir(); final Path localAgentPath = targetDir.resolve(spyAgentDownloader.computeAgentJarName(agentVersion)); System.out.println("localAgentPath = " + localAgentPath);