diff --git a/build-info-extractor-pip/src/test/java/org/jfrog/build/extractor/pip/extractor/PipExtractorTest.java b/build-info-extractor-pip/src/test/java/org/jfrog/build/extractor/pip/extractor/PipExtractorTest.java index 719ced05c..60db78b81 100644 --- a/build-info-extractor-pip/src/test/java/org/jfrog/build/extractor/pip/extractor/PipExtractorTest.java +++ b/build-info-extractor-pip/src/test/java/org/jfrog/build/extractor/pip/extractor/PipExtractorTest.java @@ -17,9 +17,16 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.*; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; -import static org.testng.Assert.*; +import static org.jfrog.build.extractor.BuildInfoExtractorUtils.isWindows; +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertNotNull; +import static org.testng.Assert.fail; /** * Created by Bar Belity on 21/07/2020. @@ -190,8 +197,4 @@ private Map getUpdatedEnvPath() throws IOException { additionalEnvValues.put("PATH", newPathValue); return additionalEnvValues; } - - private static boolean isWindows() { - return System.getProperty("os.name").toLowerCase().contains("win"); - } } diff --git a/build-info-extractor/src/main/java/org/jfrog/build/extractor/BuildInfoExtractorUtils.java b/build-info-extractor/src/main/java/org/jfrog/build/extractor/BuildInfoExtractorUtils.java index 6ee5e0922..0063d54e5 100644 --- a/build-info-extractor/src/main/java/org/jfrog/build/extractor/BuildInfoExtractorUtils.java +++ b/build-info-extractor/src/main/java/org/jfrog/build/extractor/BuildInfoExtractorUtils.java @@ -45,6 +45,8 @@ import static org.apache.commons.lang3.StringUtils.removeEnd; import static org.jfrog.build.extractor.UrlUtils.encodeUrl; import static org.jfrog.build.extractor.UrlUtils.encodeUrlPathPart; +import static org.jfrog.build.extractor.ci.BuildInfoConfigProperties.PROP_PROPS_FILE_KEY; +import static org.jfrog.build.extractor.ci.BuildInfoConfigProperties.PROP_PROPS_FILE_KEY_IV; import static org.jfrog.build.extractor.clientConfiguration.util.encryption.PropertyEncryptor.decryptPropertiesFromFile; /** @@ -109,7 +111,7 @@ private static Properties searchAdditionalPropertiesFile(Properties existingProp } try { - EncryptionKeyPair keyPair = new EncryptionKeyPair(getPropertiesFileEncryptionKey(existingProps), getPropertiesFileEncryptionKeyIv(existingProps)); + EncryptionKeyPair keyPair = new EncryptionKeyPair(getPropertiesFileEncryptionKey(), getPropertiesFileEncryptionKeyIv()); if (!keyPair.isEmpty()) { log.debug("[buildinfo] Found an encryption for buildInfo properties file for this build."); props.putAll(decryptPropertiesFromFile(propertiesFile.getPath(), keyPair)); @@ -253,37 +255,17 @@ public static void saveBuildInfoToFile(BuildInfo buildInfo, File toFile) throws } /** - * @param additionalProps Additional properties containing the encryption key. * @return The encryption key obtained from system properties or additional properties. */ - private static String getPropertiesFileEncryptionKey(Properties additionalProps) { - return getPropertiesFileEncryption(additionalProps, BuildInfoConfigProperties.PROP_PROPS_FILE_KEY); + private static String getPropertiesFileEncryptionKey() { + return StringUtils.isNotBlank(System.getenv(PROP_PROPS_FILE_KEY)) ? System.getenv(PROP_PROPS_FILE_KEY) : System.getProperty(PROP_PROPS_FILE_KEY); } /** - * @param additionalProps Additional properties containing the encryption IV. * @return The encryption IV obtained from system properties or additional properties. */ - private static String getPropertiesFileEncryptionKeyIv(Properties additionalProps) { - return getPropertiesFileEncryption(additionalProps, BuildInfoConfigProperties.PROP_PROPS_FILE_KEY_IV); - } - - private static String getPropertiesFileEncryption(Properties additionalProps, String key) { - // Check if the encryption key is set in system properties - if (StringUtils.isNotBlank(System.getProperty(key))) { - return System.getProperty(key); - } - if (additionalProps != null) { - // Check for the encryption key directly in additional properties - if (StringUtils.isNotBlank(additionalProps.getProperty(key))) { - return additionalProps.getProperty(key); - } - // Jenkins prefixes these variables with "env." so let's try that - if (StringUtils.isNotBlank(additionalProps.getProperty("env." + key))) { - return additionalProps.getProperty("env." + key); - } - } - return null; + private static String getPropertiesFileEncryptionKeyIv() { + return StringUtils.isNotBlank(System.getenv(PROP_PROPS_FILE_KEY_IV)) ? System.getenv(PROP_PROPS_FILE_KEY_IV) : System.getProperty(PROP_PROPS_FILE_KEY_IV); } private static String getAdditionalPropertiesFile(Properties additionalProps, Log log) { @@ -480,4 +462,8 @@ private static String createBuildInfoUrl(String artifactoryUrl, String buildName } return String.format("%s/%s/%s", artifactoryUrl + BUILD_BROWSE_URL, buildName, buildNumber); } + + public static boolean isWindows() { + return System.getProperty("os.name").toLowerCase().contains("win"); + } } diff --git a/build-info-extractor/src/main/java/org/jfrog/build/extractor/packageManager/PackageManagerUtils.java b/build-info-extractor/src/main/java/org/jfrog/build/extractor/packageManager/PackageManagerUtils.java index 9a5d19b61..1e5c4f009 100644 --- a/build-info-extractor/src/main/java/org/jfrog/build/extractor/packageManager/PackageManagerUtils.java +++ b/build-info-extractor/src/main/java/org/jfrog/build/extractor/packageManager/PackageManagerUtils.java @@ -5,6 +5,7 @@ import org.jfrog.build.api.util.Log; import org.jfrog.build.extractor.BuildInfoExtractorUtils; import org.jfrog.build.extractor.ci.BuildInfo; +import org.jfrog.build.extractor.ci.BuildInfoConfigProperties; import org.jfrog.build.extractor.ci.Module; import org.jfrog.build.extractor.clientConfiguration.ArtifactoryClientConfiguration; import org.jfrog.build.extractor.clientConfiguration.IncludeExcludePatterns; @@ -115,7 +116,7 @@ private static void filterExcludeIncludeProperties(IncludeExcludePatterns includ private static Properties getExcludeIncludeProperties(IncludeExcludePatterns patterns, Properties properties, Log log) { Properties props = new Properties(); for (Map.Entry entry : properties.entrySet()) { - if (!isExcludedByKey(patterns, entry) && !containsSuspectedSecrets(entry.getValue().toString())) { + if (!isExcludedByKey(patterns, entry) && !containsSuspectedSecrets(entry.getValue().toString()) && !isJfrogInternalKey(entry.getKey().toString())) { props.put(entry.getKey(), entry.getValue()); } else { log.debug("[buildinfo] Property '" + entry.getKey() + "' has been excluded'"); @@ -137,6 +138,12 @@ public static boolean containsSuspectedSecrets(String value) { containsSuspectedSecret(value, accessTokenSecretPrefix, accessTokenSecretMinimalLength); } + public static boolean isJfrogInternalKey(String key) { + return StringUtils.contains(key, BuildInfoConfigProperties.PROP_PROPS_FILE) || + StringUtils.contains(key, BuildInfoConfigProperties.PROP_PROPS_FILE_KEY) || + StringUtils.contains(key, BuildInfoConfigProperties.PROP_PROPS_FILE_KEY_IV); + } + /** * Checks whether the value of a variable contains a suspected secret. * Done by searching for a known constant prefix of the secret and verifying the length of the substring is sufficient to include the expected length of the secret. diff --git a/build-info-extractor/src/test/java/org/jfrog/build/extractor/BuildExtractorUtilsTest.java b/build-info-extractor/src/test/java/org/jfrog/build/extractor/BuildExtractorUtilsTest.java index 881fad897..0a595f5d6 100644 --- a/build-info-extractor/src/test/java/org/jfrog/build/extractor/BuildExtractorUtilsTest.java +++ b/build-info-extractor/src/test/java/org/jfrog/build/extractor/BuildExtractorUtilsTest.java @@ -17,17 +17,13 @@ import org.testng.annotations.DataProvider; import org.testng.annotations.Test; -import javax.crypto.BadPaddingException; -import javax.crypto.IllegalBlockSizeException; -import javax.crypto.NoSuchPaddingException; import java.io.FileOutputStream; import java.io.IOException; +import java.lang.reflect.Field; import java.nio.file.Files; import java.nio.file.Path; -import java.security.InvalidAlgorithmParameterException; -import java.security.InvalidKeyException; -import java.security.NoSuchAlgorithmException; import java.util.Base64; +import java.util.Map; import java.util.Properties; import static org.jfrog.build.IntegrationTestsBase.getLog; @@ -36,6 +32,7 @@ import static org.jfrog.build.extractor.BuildInfoExtractorUtils.createBuildInfoUrl; import static org.jfrog.build.extractor.BuildInfoExtractorUtils.filterDynamicProperties; import static org.jfrog.build.extractor.BuildInfoExtractorUtils.getEnvProperties; +import static org.jfrog.build.extractor.BuildInfoExtractorUtils.isWindows; import static org.jfrog.build.extractor.BuildInfoExtractorUtils.jsonStringToBuildInfo; import static org.jfrog.build.extractor.BuildInfoExtractorUtils.mergePropertiesWithSystemAndPropertyFile; import static org.testng.Assert.assertEquals; @@ -61,15 +58,6 @@ private void setUp() throws IOException { tempFile = Files.createTempFile("BuildInfoExtractorUtilsTest", "").toAbsolutePath(); } - @AfterMethod - private void tearDown() throws IOException { - Files.deleteIfExists(tempFile); - - System.clearProperty(BuildInfoConfigProperties.PROP_PROPS_FILE); - System.clearProperty(BuildInfoConfigProperties.PROP_PROPS_FILE_KEY); - System.clearProperty(BuildInfoConfigProperties.PROP_PROPS_FILE_KEY_IV); - } - public void getBuildInfoPropertiesFromSystemProps() { System.setProperty(POPO_KEY, "buildname"); System.setProperty(MOMO_KEY, "1"); @@ -99,41 +87,13 @@ public void getBuildInfoPropertiesFromFile() throws IOException { assertEquals(fileProps.getProperty(MOMO_KEY), "1", "momo property does not match"); } - public void getBuildInfoPropertiesFromEncryptedFile() throws IOException, InvalidAlgorithmParameterException, NoSuchPaddingException, IllegalBlockSizeException, NoSuchAlgorithmException, BadPaddingException, InvalidKeyException { - setupEncryptedFileTest(createProperties()); - - Properties fileProps = filterDynamicProperties( - mergePropertiesWithSystemAndPropertyFile(new Properties(), getLog()), - BUILD_INFO_PROP_PREDICATE); - assertEquals(fileProps.size(), 2, "there should only be 2 properties after the filtering"); - assertEquals(fileProps.getProperty(POPO_KEY), "buildname", "popo property does not match"); - assertEquals(fileProps.getProperty(MOMO_KEY), "1", "momo property does not match"); - } - - public void failToReadEncryptedFileWithNoKey() throws InvalidAlgorithmParameterException, NoSuchPaddingException, IllegalBlockSizeException, IOException, NoSuchAlgorithmException, BadPaddingException, InvalidKeyException { - // Create encrypted file with properties - setupEncryptedFileTest(createProperties()); - // Remove key - System.setProperty(BuildInfoConfigProperties.PROP_PROPS_FILE_KEY, ""); - // Read properties from the encrypted file - Properties fileProps = filterDynamicProperties( - mergePropertiesWithSystemAndPropertyFile(new Properties(), getLog()), - BUILD_INFO_PROP_PREDICATE); - // Check if no properties are read - assertEquals(fileProps.size(), 0, "0 properties should be present, the file is encrypted, and the key is not available"); - } - - private void setupEncryptedFileTest(Properties props) throws IOException, InvalidAlgorithmParameterException, NoSuchPaddingException, IllegalBlockSizeException, NoSuchAlgorithmException, BadPaddingException, InvalidKeyException { - props.put(BuildInfoConfigProperties.PROP_PROPS_FILE, tempFile.toString()); - System.setProperty(BuildInfoConfigProperties.PROP_PROPS_FILE, tempFile.toString()); - ArtifactoryClientConfiguration client = new ArtifactoryClientConfiguration(new NullLog()); - client.fillFromProperties(props); + @AfterMethod + private void tearDown() throws Exception { + Files.deleteIfExists(tempFile); - try (FileOutputStream fileOutputStream = new FileOutputStream(tempFile.toFile())) { - EncryptionKeyPair keyPair = client.persistToEncryptedPropertiesFile(fileOutputStream); - System.setProperty(BuildInfoConfigProperties.PROP_PROPS_FILE_KEY, Base64.getEncoder().encodeToString(keyPair.getSecretKey())); - System.setProperty(BuildInfoConfigProperties.PROP_PROPS_FILE_KEY_IV, Base64.getEncoder().encodeToString(keyPair.getIv())); - } + unsetEnv(BuildInfoConfigProperties.PROP_PROPS_FILE); + unsetEnv(BuildInfoConfigProperties.PROP_PROPS_FILE_KEY); + unsetEnv(BuildInfoConfigProperties.PROP_PROPS_FILE_KEY_IV); } public void getBuildInfoProperties() throws IOException { @@ -195,27 +155,15 @@ public void getEnvAndSysPropertiesFromFile() throws IOException { System.clearProperty(gogoKey); } - public void getEnvAndSysPropertiesFromEncryptedFile() throws IOException, InvalidAlgorithmParameterException, NoSuchPaddingException, IllegalBlockSizeException, NoSuchAlgorithmException, BadPaddingException, InvalidKeyException { - // Put system properties - String kokoKey = "koko"; - String gogoKey = "gogo"; - System.setProperty(kokoKey, "parent"); - System.setProperty(gogoKey, "2"); - - // Encrypt properties and write to the file - setupEncryptedFileTest(createPropertiesEnvs()); - - // Read properties from the encrypted file - Properties buildInfoProperties = getEnvProperties(new Properties(), new NullLog()); - - // Check if decrypted properties are as expected - assertEquals(buildInfoProperties.getProperty(ENV_POPO_KEY), "buildname", "popo property does not match"); - assertEquals(buildInfoProperties.getProperty(ENV_MOMO_KEY), "1", "momo number property does not match"); - assertEquals(buildInfoProperties.getProperty("koko"), "parent", "koko parent name property does not match"); - assertEquals(buildInfoProperties.getProperty("gogo"), "2", "gogo parent number property does not match"); + public void getBuildInfoPropertiesFromEncryptedFile() throws Exception { + setupEncryptedFileTest(createProperties()); - System.clearProperty(kokoKey); - System.clearProperty(gogoKey); + Properties fileProps = filterDynamicProperties( + mergePropertiesWithSystemAndPropertyFile(new Properties(), getLog()), + BUILD_INFO_PROP_PREDICATE); + assertEquals(fileProps.size(), 2, "there should only be 2 properties after the filtering"); + assertEquals(fileProps.getProperty(POPO_KEY), "buildname", "popo property does not match"); + assertEquals(fileProps.getProperty(MOMO_KEY), "1", "momo property does not match"); } public void testExcludePatterns() { @@ -339,4 +287,79 @@ private Properties createPropertiesEnvs() { props.put(ENV_MOMO_KEY, "1"); return props; } + + public void failToReadEncryptedFileWithNoKey() throws Exception { + // Create encrypted file with properties + setupEncryptedFileTest(createProperties()); + // Remove key + unsetEnv(BuildInfoConfigProperties.PROP_PROPS_FILE_KEY); + // Read properties from the encrypted file + Properties fileProps = filterDynamicProperties( + mergePropertiesWithSystemAndPropertyFile(new Properties(), getLog()), + BUILD_INFO_PROP_PREDICATE); + // Check if no properties are read + assertEquals(fileProps.size(), 0, "0 properties should be present, the file is encrypted, and the key is not available"); + } + + private void setupEncryptedFileTest(Properties props) throws Exception { + props.put(BuildInfoConfigProperties.PROP_PROPS_FILE, tempFile.toString()); + System.setProperty(BuildInfoConfigProperties.PROP_PROPS_FILE, tempFile.toString()); + ArtifactoryClientConfiguration client = new ArtifactoryClientConfiguration(new NullLog()); + client.fillFromProperties(props); + + try (FileOutputStream fileOutputStream = new FileOutputStream(tempFile.toFile())) { + EncryptionKeyPair keyPair = client.persistToEncryptedPropertiesFile(fileOutputStream); + setEnv(BuildInfoConfigProperties.PROP_PROPS_FILE_KEY, Base64.getEncoder().encodeToString(keyPair.getSecretKey())); + setEnv(BuildInfoConfigProperties.PROP_PROPS_FILE_KEY_IV, Base64.getEncoder().encodeToString(keyPair.getIv())); + } + } + + public void getEnvAndSysPropertiesFromEncryptedFile() throws Exception { + // Put system properties + String kokoKey = "koko"; + String gogoKey = "gogo"; + System.setProperty(kokoKey, "parent"); + System.setProperty(gogoKey, "2"); + + // Encrypt properties and write to the file + setupEncryptedFileTest(createPropertiesEnvs()); + + // Read properties from the encrypted file + Properties buildInfoProperties = getEnvProperties(new Properties(), new NullLog()); + + // Check if decrypted properties are as expected + assertEquals(buildInfoProperties.getProperty(ENV_POPO_KEY), "buildname", "popo property does not match"); + assertEquals(buildInfoProperties.getProperty(ENV_MOMO_KEY), "1", "momo number property does not match"); + assertEquals(buildInfoProperties.getProperty("koko"), "parent", "koko parent name property does not match"); + assertEquals(buildInfoProperties.getProperty("gogo"), "2", "gogo parent number property does not match"); + + System.clearProperty(kokoKey); + System.clearProperty(gogoKey); + } + + private void setEnv(String key, String value) throws Exception { + modifyEnv(key, value); + } + + private void unsetEnv(String key) throws Exception { + modifyEnv(key, ""); + } + + /** + * Modifies (set/unset) environment variables using reflection + */ + @SuppressWarnings("unchecked") + private static void modifyEnv(String key, String newValue) throws Exception { + if (isWindows()) { + System.setProperty(key, newValue); + return; + } + Map env = System.getenv(); + Class cl = env.getClass(); + Field field = cl.getDeclaredField("m"); + field.setAccessible(true); + Map writableEnv = (Map) field.get(env); + + writableEnv.put(key, newValue); + } } diff --git a/build-info-extractor/src/test/java/org/jfrog/build/extractor/packageManager/PackageManagerUtilsTest.java b/build-info-extractor/src/test/java/org/jfrog/build/extractor/packageManager/PackageManagerUtilsTest.java index 73b433164..1c3429269 100644 --- a/build-info-extractor/src/test/java/org/jfrog/build/extractor/packageManager/PackageManagerUtilsTest.java +++ b/build-info-extractor/src/test/java/org/jfrog/build/extractor/packageManager/PackageManagerUtilsTest.java @@ -14,6 +14,9 @@ import java.util.Properties; import static org.jfrog.build.extractor.BuildInfoExtractorUtils.getEnvProperties; +import static org.jfrog.build.extractor.ci.BuildInfoConfigProperties.PROP_PROPS_FILE; +import static org.jfrog.build.extractor.ci.BuildInfoConfigProperties.PROP_PROPS_FILE_KEY; +import static org.jfrog.build.extractor.ci.BuildInfoConfigProperties.PROP_PROPS_FILE_KEY_IV; import static org.jfrog.build.extractor.packageManager.PackageManagerUtils.containsSuspectedSecrets; import static org.jfrog.build.extractor.packageManager.PackageManagerUtils.filterBuildInfoProperties; import static org.testng.Assert.assertEquals; @@ -103,6 +106,37 @@ public void testExcludePatterns() { } + @Test + public void testExcludeJfrogInternalKey() { + Properties props = new Properties(); + Properties buildInfoProperties = getEnvProperties(props, new NullLog()); + Properties moduleProps = new Properties(); + moduleProps.setProperty(key1, value1); + moduleProps.setProperty(PROP_PROPS_FILE_KEY, value1); + moduleProps.setProperty(PROP_PROPS_FILE_KEY_IV, value1); + moduleProps.setProperty(PROP_PROPS_FILE, value1); + Module module = new Module(); + module.setId("foo"); + module.setProperties(moduleProps); + BuildInfo buildInfo = new BuildInfoBuilder("BUILD_NAME") + .number("BUILD_NUMBER") + .properties(buildInfoProperties) + .startedDate(new Date()) + .properties(buildInfoProperties) + .addModule(module).build(); + + filterBuildInfoPropertiesTestHelper(buildInfo); + + // Excluded build info JFrog internal keys + assertFalse(buildInfo.getProperties().containsKey(PROP_PROPS_FILE), "Should not find '" + PROP_PROPS_FILE + "' property due to exclude JFrog internal key"); + assertFalse(buildInfo.getProperties().containsKey(PROP_PROPS_FILE_KEY_IV), "Should not find '" + PROP_PROPS_FILE_KEY_IV + "' property due to exclude JFrog internal key"); + assertFalse(buildInfo.getProperties().containsKey(PROP_PROPS_FILE_KEY), "Should not find '" + PROP_PROPS_FILE_KEY + "' property due to exclude JFrog internal key"); + + // Keep build info property + assertEquals(buildInfo.getModule("foo").getProperties().getProperty(key1), value1, key1 + " property does not match"); + assertTrue(buildInfo.getProperties().containsKey(key1), "Should find '" + key1 + "'"); + } + @Test public void testIncludePatterns() { Properties props = new Properties();