Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build info property key can only be obtained from environment variables #771

Merged
merged 9 commits into from
Dec 19, 2023
Merged

Build info property key can only be obtained from environment variables #771

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9ee0176
Get build info property key from env vars only
Or-Geva Dec 13, 2023
5592d9b
Fix CR comments
Or-Geva Dec 13, 2023
cf80faa
Fix tests by setting the env vars with encryption key instead of syst…
Or-Geva Dec 13, 2023
f97cf74
Excluded build info JFrog internal keys
Or-Geva Dec 14, 2023
96f63aa
Try to fix windows tests
Or-Geva Dec 14, 2023
0166aaa
Try fix windows set env
Or-Geva Dec 14, 2023
725d4cb
Excluded build info JFrog internal keys
Or-Geva Dec 14, 2023
f26d7ad
Fix test
Or-Geva Dec 14, 2023
ade659d
Apply inline comment
Or-Geva Dec 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions ...extractor-pip/src/test/java/org/jfrog/build/extractor/pip/extractor/PipExtractorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -190,8 +197,4 @@ private Map<String, String> getUpdatedEnvPath() throws IOException {
additionalEnvValues.put("PATH", newPathValue);
return additionalEnvValues;
}

private static boolean isWindows() {
return System.getProperty("os.name").toLowerCase().contains("win");
}
}
36 changes: 11 additions & 25 deletions build-info-extractor/src/main/java/org/jfrog/build/extractor/BuildInfoExtractorUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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");
}
}
9 changes: 8 additions & 1 deletion ...extractor/src/main/java/org/jfrog/build/extractor/packageManager/PackageManagerUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Object, Object> 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'");
Expand All @@ -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.
Expand Down
161 changes: 92 additions & 69 deletions build-info-extractor/src/test/java/org/jfrog/build/extractor/BuildExtractorUtilsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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");
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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<String, String> env = System.getenv();
Class<?> cl = env.getClass();
Field field = cl.getDeclaredField("m");
field.setAccessible(true);
Map<String, String> writableEnv = (Map<String, String>) field.get(env);

writableEnv.put(key, newValue);
}
}
34 changes: 34 additions & 0 deletions ...actor/src/test/java/org/jfrog/build/extractor/packageManager/PackageManagerUtilsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
Loading