Adds support for using job-specific policies #223

bluesliverx · 2022-06-20T23:47:45Z

Fixes #214

This adds support for separating job policies as described in #214. This is inspired by the Salt integration and their usage of per-minion policies. The vault plugin may be configured with optional "policies" that may be templatized with tokens that are unique to the job. This allows more isolation of job permissions in Vault, thereby allowing for more secure Jenkins configurations.

I was able to add most of the logic for this functionality into the expiring credential base class. I also had to separate the cached token value by the list of policies specified so that different lists of policies resulted in new tokens. I have tested this on our Jenkins environment and it has allowed me to completely separate the policies of two jobs so that they can only read specific paths in Vault. I also added unit testing for all functionality I added (AFAIK, let me know if there is any missing).

I opted for two configuration options instead of a single one, since in our environment we need to migrate from one method to the other (i.e. AppRole based auth that has all permissions vs AppRole based auth that enables using the limited tokens). This is done in a separate commit in case there are disagreements if this should be how it is configured or not. Without this option, as soon as I enabled policies in the folder or global configuration, the existing auth method no longer worked properly. This may not be useful outside of our use case, so please let me know if I need to remove it.

Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
Ensure that the pull request title represents the desired changelog entry
Please describe what you did
Link to relevant issues in GitHub or Jira
Link to relevant pull requests, esp. upstream and downstream changes
Ensure you have provided tests - that demonstrates feature works or fixes the issue

bluesliverx · 2022-06-21T17:06:39Z

RETEST

jetersen · 2022-07-12T08:52:07Z

@bluesliverx not sure what this achieves? Wouldn't it be relatively easy to copy a job and remove the use policy and it continues to work.

This is only implemented on the plugin side, nothing on hashicorp vault enforces this.

Or am I mistaken?

bluesliverx · 2022-07-18T17:37:25Z

@jetersen sorry, I've been vacationing for a couple of weeks.

Yes, there are some security flaws in this depending on the job configuration. I was thinking I should probably remove the ability to configure this on the job itself so that people can't override it at that level. Thoughts?

The idea here is that there is a structure of folders/jobs which can be utilized to segment the vault policies. This limits the ability of one job to read the secrets that should only be read by another job. This is the same methodology used in Salt and works very well there. I liked how it was done and wanted to extend the Vault plugin to do the same thing.

For a little background, my team manages a Jenkins cluster that has thousands of jobs across different folders. Currently we just allow all jobs to read secrets based on a single Jenkins policy, but we really want to segment that so that we limit security holes in the system. This would allow us to do that.

bluesliverx · 2022-08-24T18:38:56Z

@jetersen, any feedback on this?

bluesliverx · 2022-09-30T17:20:20Z

I fixed a bug that we identified when we changed the TTL on the approle auth. The child tokens were still created with their default TTL, which was much longer than the approle-based token, and once it expired the child tokens were expired but they were still attempted to be used. I now set the child token TTL to the parent token TTL.

jetersen · 2023-11-17T14:41:55Z

src/main/java/com/datapipe/jenkins/vault/VaultAccessor.java

+    public static String replacePolicyTokens(String policy, EnvVars envVars) {
+        if (!policy.contains("{")) {
+            return policy;
+        }
+        String jobName = envVars.get("JOB_NAME");
+        String jobBaseName = envVars.get("JOB_BASE_NAME");
+        String folder = "";
+        if (!jobName.equals(jobBaseName) && jobName.contains("/")) {
+            String[] jobElements = jobName.split("/");
+            folder = Arrays.stream(jobElements)
+                .limit(jobElements.length - 1)
+                .collect(Collectors.joining("/"));
+        }
+        return policy
+            .replaceAll("\\{job_base_name}", jobBaseName)
+            .replaceAll("\\{job_name}", jobName)
+            .replaceAll("\\{job_name_us}", jobName.replaceAll("/", "_"))
+            .replaceAll("\\{job_folder}", folder)
+            .replaceAll("\\{job_folder_us}", folder.replaceAll("/", "_"))
+            .replaceAll("\\{node_name}", envVars.get("NODE_NAME"));
+    }
+
+    public static List<String> generatePolicies(String policies, EnvVars envVars) {
+        if (StringUtils.isBlank(policies)) {
+            return null;
+        }
+        return Arrays.stream(policies.split("\n"))
+            .filter(StringUtils::isNotBlank)
+            .map(policy -> replacePolicyTokens(policy.trim(), envVars))
+            .collect(Collectors.toList());
+    }


Not sure I like this, I wonder if StringSubstitutor is a better solution 🤔

You can see usage of StringSubstitutor over here
https://github.com/jenkinsci/configuration-as-code-plugin/blob/master/plugin/src/main/java/io/jenkins/plugins/casc/SecretSourceResolver.java

I replaced the usage with a simple StringSubstitutor, it does clean up the tokens a bit and get rid of some ugly regex. Take a look and let me know what you think.

This is a lot cleaner 😅

bluesliverx · 2023-11-20T16:12:43Z

@jetersen thanks for taking a look at this again. In addition to the code review changes, I also implemented a disable flag for sub-folders and jobs, which should prevent escalation of policies on rogue folders/jobs when the admin enables it. Let me know what you think!

jetersen

Not sure if it is a valid concern but I feel like Caffeine cache might help here but I wonder if that is a separate PR

Overall the changes look fine.

bluesliverx · 2023-11-20T21:05:55Z

I'm happy to do a separate PR for that if you want. We run a decently large build grid (15k+ jobs with likely hundreds of runs each hour) and I haven't noticed any slowdowns to any of those jobs based on retrieving Vault secrets, just FYI.

jetersen · 2024-01-28T19:27:31Z

.../java/com/datapipe/jenkins/vault/credentials/AbstractVaultTokenCredentialWithExpiration.java

+    private Map<String, Calendar> tokenExpiry;
+    private Map<String, String> tokenCache;


This should probably be marked transitive to avoid being serialized which causes #323

The reason for getOwner returning null is because serializer is not allowed to deserialize Map<String, Calendar>

https://github.com/jenkinsci/jenkins/blob/419539c1fa889155cee4ea27a415bc101c64f6dc/core/src/main/resources/jenkins/security/whitelisted-classes.txt#L4

Only GregorianCalendar is allowed 🤔

bluesliverx force-pushed the job-policies branch from 89affe4 to 24a1887 Compare June 21, 2022 14:16

bluesliverx force-pushed the job-policies branch from 24a1887 to be7448a Compare June 21, 2022 17:08

bluesliverx force-pushed the job-policies branch from cf4899a to 95b4ddc Compare November 16, 2023 21:56

saville added 3 commits November 17, 2023 07:31

Fixes jenkinsci#214, adds support for separating job policies

b4b4edd

Add configuration to credentials to enable using limited policies

3c99d78

Fix handling of TTL in child tokens

ed920da

bluesliverx force-pushed the job-policies branch from 95b4ddc to ed920da Compare November 17, 2023 14:33

jetersen changed the title ~~Fixes #214, adds support for using job-specific policies~~ Adds support for using job-specific policies Nov 17, 2023

jetersen added the enhancement label Nov 17, 2023

jetersen reviewed Nov 17, 2023

View reviewed changes

saville added 2 commits November 20, 2023 08:46

Add ability to disable folders or jobs from overriding policies

2c55e5e

Use StringSubstitutor for templating policies

8d00c71

Fix flaky test

820ef82

jetersen reviewed Nov 20, 2023

View reviewed changes

jetersen approved these changes Nov 20, 2023

View reviewed changes

jetersen merged commit f5d54b3 into jenkinsci:master Nov 20, 2023
14 checks passed

jetersen reviewed Jan 28, 2024

View reviewed changes

jetersen mentioned this pull request Jan 28, 2024

Make sure token does not expire within 10 seconds instead of 2 #268

Merged

5 tasks

This was referenced Aug 23, 2024

Upgrading to the lastest plugin (364.vf5d54b_3dc313) breaks any Org or pipeline that uses with Vault. #326

Open

Rename tokenExpiry field to prevent Folder corruption issues when upgrading from old versions of this plugin #336

Merged

This was referenced Oct 16, 2024

Exception on java 17 #340

Closed

Make sure both tokenCache and tokenExpiryCache are initialized if needed #342

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds support for using job-specific policies #223

Adds support for using job-specific policies #223

bluesliverx commented Jun 20, 2022 •

edited by jetersen

Loading

bluesliverx commented Jun 21, 2022

jetersen commented Jul 12, 2022 •

edited

Loading

bluesliverx commented Jul 18, 2022 •

edited

Loading

bluesliverx commented Aug 24, 2022

bluesliverx commented Sep 30, 2022

jetersen Nov 17, 2023

bluesliverx Nov 20, 2023

jetersen Nov 20, 2023

bluesliverx commented Nov 20, 2023

jetersen left a comment •

edited

Loading

bluesliverx commented Nov 20, 2023

jetersen Jan 28, 2024

jetersen Jan 28, 2024

		private Map<String, Calendar> tokenExpiry;
		private Map<String, String> tokenCache;

Adds support for using job-specific policies #223

Adds support for using job-specific policies #223

Conversation

bluesliverx commented Jun 20, 2022 • edited by jetersen Loading

bluesliverx commented Jun 21, 2022

jetersen commented Jul 12, 2022 • edited Loading

bluesliverx commented Jul 18, 2022 • edited Loading

bluesliverx commented Aug 24, 2022

bluesliverx commented Sep 30, 2022

jetersen Nov 17, 2023

Choose a reason for hiding this comment

bluesliverx Nov 20, 2023

Choose a reason for hiding this comment

jetersen Nov 20, 2023

Choose a reason for hiding this comment

bluesliverx commented Nov 20, 2023

jetersen left a comment • edited Loading

Choose a reason for hiding this comment

bluesliverx commented Nov 20, 2023

jetersen Jan 28, 2024

Choose a reason for hiding this comment

jetersen Jan 28, 2024

Choose a reason for hiding this comment

bluesliverx commented Jun 20, 2022 •

edited by jetersen

Loading

jetersen commented Jul 12, 2022 •

edited

Loading

bluesliverx commented Jul 18, 2022 •

edited

Loading

jetersen left a comment •

edited

Loading