diff --git a/plugin/src/main/java/io/jenkins/plugins/casc/Attribute.java b/plugin/src/main/java/io/jenkins/plugins/casc/Attribute.java index 14a630b656..39e980d6a6 100644 --- a/plugin/src/main/java/io/jenkins/plugins/casc/Attribute.java +++ b/plugin/src/main/java/io/jenkins/plugins/casc/Attribute.java @@ -50,6 +50,15 @@ public class Attribute { // Nop }; + // TODO: Make the list configurable via system property + // TODO: Move secret-related logic to SecretsHelper + private static final Set SUSPECTED_SECRET_ATTRIBUTE_NAMES_AND_SUFFIXES = + Collections.unmodifiableSet(new HashSet<>( + Arrays.asList("password", "passphrase", "passwd", "pwd", + "secret", "secretkey", "privatekey", + "token", "accesstoken", "apitoken"))); + + //TODO: Concurrent cache? //private static final HashMap SECRET_ATTRIBUTE_CACHE = // new HashMap<>(); @@ -262,8 +271,15 @@ public CNode describe(Owner instance, ConfigurationContext context) throws Confi private CNode _describe(Configurator c, ConfigurationContext context, Object value, boolean shouldBeMasked) throws Exception { CNode node = c.describe(value, context); - if (shouldBeMasked && node instanceof Scalar) { - ((Scalar)node).sensitive(true); + if (node instanceof Scalar) { + Scalar scalar = (Scalar)node; + if (shouldBeMasked) { + scalar.sensitive(true); + scalar.encrypted(this.getter.encrypted()); + } + if (type == Secret.class) { + scalar.encrypted(true); + } } return node; } @@ -295,6 +311,14 @@ public interface Setter { @FunctionalInterface public interface Getter { T getValue(O target) throws Exception; + + /** + * Checks whether getter returns encrypted value. + * @return {@code true} if {@link #getValue(Object)} returns encrypted value instead of the original value. + * {@code false} will be returned for {@link Secret} getters since they do not encrypt the value on their own + * @since TODO + */ + default boolean encrypted() {return false;} } @CheckForNull @@ -342,6 +366,16 @@ public static boolean calculateIfSecret(@CheckForNull Class targetClass, @Non return true; } + // try attributes and suffixes + String lowerCaseFieldName = fieldName.toLowerCase(); + for (String suffix : SUSPECTED_SECRET_ATTRIBUTE_NAMES_AND_SUFFIXES) { + if (lowerCaseFieldName.endsWith(suffix)) { + LOGGER.log(Level.FINER, "Attribute {0} is secret, because '{1}' is in the list of common Secret attribute names and suffixes", + new Object[]{fieldName, suffix}); + return true; + } + } + if (targetClass == null) { LOGGER.log(Level.FINER, "Attribute {0} is assumed to be non-secret, because there is no class instance in the call. " + "This call is used only for fast-fetch caching, and the result may be adjusted later", diff --git a/plugin/src/main/java/io/jenkins/plugins/casc/core/ProxyConfigurator.java b/plugin/src/main/java/io/jenkins/plugins/casc/core/ProxyConfigurator.java index ff13d282c2..26c5cb2fe2 100644 --- a/plugin/src/main/java/io/jenkins/plugins/casc/core/ProxyConfigurator.java +++ b/plugin/src/main/java/io/jenkins/plugins/casc/core/ProxyConfigurator.java @@ -58,7 +58,7 @@ protected ProxyConfiguration instance(Mapping mapping, ConfigurationContext cont .getter(ProxyConfiguration::getUserName) .setter(noop()), new Attribute("password", String.class) - .getter(ProxyConfiguration::getEncryptedPassword) + .getter(ProxyPasswordGetter.INSTANCE) .setter(noop()), new Attribute("noProxyHost", String.class) .getter(config -> config.noProxyHost) @@ -128,4 +128,19 @@ public void setTestUrl(String testUrl) { this.testUrl = testUrl; } } + + static class ProxyPasswordGetter implements Attribute.Getter { + + private static final ProxyPasswordGetter INSTANCE = new ProxyPasswordGetter(); + + @Override + public String getValue(ProxyConfiguration target) throws Exception { + return target.getEncryptedPassword(); + } + + @Override + public boolean encrypted() { + return true; + } + } } diff --git a/plugin/src/main/java/io/jenkins/plugins/casc/model/Scalar.java b/plugin/src/main/java/io/jenkins/plugins/casc/model/Scalar.java index 2fa5fbc2fd..01ee50336e 100644 --- a/plugin/src/main/java/io/jenkins/plugins/casc/model/Scalar.java +++ b/plugin/src/main/java/io/jenkins/plugins/casc/model/Scalar.java @@ -94,23 +94,25 @@ public boolean isMasked() { /** * Sets the sensitive flag. * It indicates that the scalar represents a sensitive argument (secret or other restricted data). + * Sensitive flag cannot be reset to {@code false}. * @param sensitive value to set * @return Object instance * @since 1.25 */ public Scalar sensitive(boolean sensitive) { - this.sensitive = sensitive; + this.sensitive = this.sensitive || sensitive; return this; } /** * Indicates that the data is encrypted and hence safe to be exported. + * Encrypted flag cannot be reset to {@code false}. * @param encrypted Value to set * @return Object instance * @since 1.25 */ public Scalar encrypted(boolean encrypted) { - this.encrypted = encrypted; + this.encrypted = this.encrypted || encrypted; return this; } diff --git a/plugin/src/test/java/io/jenkins/plugins/casc/AttributeTest.java b/plugin/src/test/java/io/jenkins/plugins/casc/AttributeTest.java index 6126457f3d..c33f9dd483 100644 --- a/plugin/src/test/java/io/jenkins/plugins/casc/AttributeTest.java +++ b/plugin/src/test/java/io/jenkins/plugins/casc/AttributeTest.java @@ -41,6 +41,7 @@ public void checkCommonSecretPatterns() { assertFieldIsSecret(SecretFromPublicField.class, "secretField"); assertFieldIsSecret(SecretFromPrivateField.class, "secretField"); + assertFieldIsSecret(SecretFromImpliedAttributeName.class, "password"); assertFieldIsSecret(SecretRenamedFieldFithSecretConstructor.class, "mySecretValueField"); } @@ -61,6 +62,8 @@ public void checkCommonSecretPatternsForOverrides() { assertFieldIsSecret(SecretFromPublicField2.class, "secretField"); assertFieldIsSecret(SecretFromPrivateField2.class, "secretField"); assertFieldIsSecret(SecretFromPrivateField3.class, "secretField"); + + assertFieldIsSecret(SecretFromImpliedAttributeName2.class, "password"); } @Test @@ -69,6 +72,25 @@ public void checkNonSecretPatterns() { assertFieldIsNotSecret(NonSecretField.class, "passwordPath"); } + @Test + public void shouldConsiderSuffixes() { + assertFieldIsSecret(null, "secretKey"); + assertFieldIsSecret(null, "mySecretKey"); + assertFieldIsSecret(null, "myPwd"); + assertFieldIsSecret(null, "superSecretPassword"); + + // Examples of false positives + assertFieldIsSecret(null, "pathToSecretKey"); + assertFieldIsSecret(null, "foretoken"); // https://en.wiktionary.org/wiki/foretoken + } + + @Test + public void shouldNotConsiderSuffixesInTheMiddle() { + assertFieldIsNotSecret(null, "passwordPath"); + assertFieldIsNotSecret(null, "passwordFile"); + assertFieldIsNotSecret(null, "tokenSource"); + } + public static void assertFieldIsSecret(Class clazz, String fieldName) { String displayName = clazz != null ? (clazz.getName() + "#" + fieldName) : fieldName; assertTrue("Field is not secret: " + displayName, @@ -159,6 +181,24 @@ public SecretFromPrivateField3(String secret) { } } + public static class SecretFromImpliedAttributeName { + + public String password; + + @DataBoundConstructor + public SecretFromImpliedAttributeName(String password) { + this.password = password; + } + } + + public static class SecretFromImpliedAttributeName2 extends SecretFromImpliedAttributeName { + + @DataBoundConstructor + public SecretFromImpliedAttributeName2(String password) { + super(password); + } + } + public static class NonSecretField { public String passwordPath;