cmd/tls: set explicit file permissions for generated certs #22286

sujay-hashicorp · 2025-04-23T09:24:52Z

Description

Consul CLI's tls create command was generating some sensitive files (e.g. consul tls ca create -server) that had excessive permissions at 0666 for public certs created and it should be 0644. Updated the code to use 0644 file perms when creating sensitive files with ca and certs.
NOTE: disabled Compatibility Integration Tests until pipeline upgrade is in place

Testing & Reproduction steps

Update default OS masking to view actual code behaviour: umask 000
Run consul tls ca create -domain=example.test and list the directory to view generated files with ls -lah with excessive perms.
Run consul tls cert create -server to see similar results
Revert OS level masking to default: umask 022

Links

https://hashicorp.atlassian.net/browse/SECVULN-8634

PR Checklist

updated test coverage
external facing docs updated
appropriate backport labels added
not a security concern

hashicorp-cla-app · 2025-04-23T09:25:06Z

All committers have signed the CLA.

…r public creds

sriramr98 · 2025-05-21T03:10:16Z

.github/workflows/test-integrations.yml

-          # tput complains if this isn't set to something.
-          TERM: ansi
+      
+      # TODO: Disabling the "Compatibility Integration Tests" test temporarily, need to enable again once dependent pipeline issues are fixed.


Is the pipeline broken from the changes or was it broken earlier?

Yes, it's an existing issue. This issue is being tracked internally here.

sriramr98 · 2025-05-21T03:11:00Z

command/tls/ca/create/tls_ca_create.go

@@ -82,12 +82,14 @@ func (c *cmd) Run(args []string) int {
 		return 1
 	}

-	if err := file.WriteAtomicWithPerms(certFileName, []byte(ca), 0755, 0666); err != nil {
+	// public CA cert file
+	if err := file.WriteAtomicWithPerms(certFileName, []byte(ca), 0755, 0644); err != nil {


I would recommend moving 0755 and 0644 to a constant variable with good naming for readability purposes

Now updated to use variables.

sriramr98 · 2025-05-21T03:11:40Z

command/tls/cert/create/tls_cert_create.go

@@ -193,12 +193,14 @@ func (c *cmd) Run(args []string) int {
 		return 1
 	}

-	if err := file.WriteAtomicWithPerms(certFileName, []byte(pub), 0755, 0666); err != nil {
+	// public cert
+	if err := file.WriteAtomicWithPerms(certFileName, []byte(pub), 0755, 0644); err != nil {


I would recommend moving 0755 and 0644 to a constant variable for better readability purposes

Updated the PR now

sreeram77 · 2025-05-23T10:55:17Z

.changelog/22286.txt

@@ -0,0 +1,3 @@
+```release-note:security
+(cmd/tls) update ca and cert create to reduce excessive file perms for public creds


(cmd/tls) is not adhering to specs - https://github.com/hashicorp/consul/blob/main/docs/contributing/add-a-changelog-entry.md

Fixed it now

sreeram77

LGTM

…place

sujay-hashicorp requested a review from a team as a code owner April 23, 2025 09:24

github-actions bot added the theme/cli Flags and documentation for the CLI interface label Apr 23, 2025

sujay-hashicorp added the backport/all Apply backports for all active releases per .release/versions.hcl label Apr 23, 2025

sujay-hashicorp added a commit that referenced this pull request Apr 23, 2025

add changelog for PR #22286

d1987bc

sujay-hashicorp added a commit that referenced this pull request May 7, 2025

add changelog for PR #22286

b2cd0ab

sujay-hashicorp force-pushed the sujay/tls-file-perms branch from d4a7ff7 to 351d533 Compare May 7, 2025 05:50

sujay-hashicorp added a commit that referenced this pull request May 12, 2025

add changelog for PR #22286

fb31967

sujay-hashicorp force-pushed the sujay/tls-file-perms branch from 351d533 to 6d2c09f Compare May 12, 2025 04:26

sujay-hashicorp added a commit that referenced this pull request May 15, 2025

add changelog for PR #22286

ca418d1

sujay-hashicorp force-pushed the sujay/tls-file-perms branch from 6d2c09f to 3ac8c2d Compare May 15, 2025 06:35

sujay-hashicorp added a commit that referenced this pull request May 15, 2025

add changelog for PR #22286

95eb768

sujay-hashicorp force-pushed the sujay/tls-file-perms branch from 3ac8c2d to 95eb768 Compare May 15, 2025 08:30

(cmd/tls) update ca and cert create to reduce excessive file perms fo…

b70559d

…r public creds

sujay-hashicorp added a commit that referenced this pull request May 20, 2025

add changelog for PR #22286

8589b99

sujay-hashicorp force-pushed the sujay/tls-file-perms branch from 95eb768 to 8589b99 Compare May 20, 2025 05:50

sriramr98 requested changes May 21, 2025

View reviewed changes

sujay-hashicorp requested a review from sriramr98 May 23, 2025 10:36

sujay-hashicorp force-pushed the sujay/tls-file-perms branch 2 times, most recently from 2383d3b to 7d8f8a2 Compare May 23, 2025 10:49

sriramr98 previously approved these changes May 23, 2025

View reviewed changes

sreeram77 requested changes May 23, 2025

View reviewed changes

sujay-hashicorp dismissed sriramr98’s stale review via 831e9df May 23, 2025 10:57

sujay-hashicorp force-pushed the sujay/tls-file-perms branch from 7d8f8a2 to 831e9df Compare May 23, 2025 10:57

sujay-hashicorp added a commit that referenced this pull request May 23, 2025

add changelog for PR #22286

f8d5b98

sujay-hashicorp force-pushed the sujay/tls-file-perms branch from 831e9df to 5cc9463 Compare May 23, 2025 11:02

sujay-hashicorp requested review from sreeram77 and sriramr98 May 23, 2025 11:03

sreeram77 previously approved these changes May 23, 2025

View reviewed changes

add changelog for PR #22286

840246f

sujay-hashicorp added 3 commits May 23, 2025 17:57

disable Compatibility Integration Tests until pipeline upgrade is in …

d2971cf

…place

update .go-version to 1.23.8

777d183

(cmd/tls) use vars for dir and file perms for better readability

f0cd8be

sujay-hashicorp dismissed sreeram77’s stale review via f0cd8be May 23, 2025 12:27

sujay-hashicorp force-pushed the sujay/tls-file-perms branch from 5cc9463 to f0cd8be Compare May 23, 2025 12:27

sriramr98 approved these changes May 23, 2025

View reviewed changes

sujay-hashicorp merged commit d13c81f into main May 23, 2025
94 checks passed

sujay-hashicorp deleted the sujay/tls-file-perms branch May 23, 2025 13:09

hc-github-team-consul-core added backport/1.21 Changes are backported to 1.21 backport/ent/1.18 Changes are backported to 1.18 ent backport/ent/1.19 Changes are backported to 1.19 ent backport/ent/1.20 backport to ent 1.20 labels May 23, 2025

hc-github-team-consul-core mentioned this pull request May 23, 2025

Backport of cmd/tls: set explicit file permissions for generated certs into release/1.21.x #22355

Draft

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

cmd/tls: set explicit file permissions for generated certs #22286

cmd/tls: set explicit file permissions for generated certs #22286

sujay-hashicorp commented Apr 23, 2025 •

edited

Loading

Uh oh!

hashicorp-cla-app bot commented Apr 23, 2025 •

edited

Loading

Uh oh!

sriramr98 May 21, 2025

Uh oh!

sujay-hashicorp May 23, 2025

Uh oh!

sriramr98 May 21, 2025

Uh oh!

sujay-hashicorp May 23, 2025

Uh oh!

sriramr98 May 21, 2025

Uh oh!

sujay-hashicorp May 23, 2025

Uh oh!

sreeram77 May 23, 2025

Uh oh!

sujay-hashicorp May 23, 2025

Uh oh!

sreeram77 left a comment

Uh oh!

Uh oh!

Uh oh!

		@@ -0,0 +1,3 @@
		```release-note:security
		(cmd/tls) update ca and cert create to reduce excessive file perms for public creds

cmd/tls: set explicit file permissions for generated certs #22286

cmd/tls: set explicit file permissions for generated certs #22286

Conversation

sujay-hashicorp commented Apr 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing & Reproduction steps

Links

PR Checklist

Uh oh!

hashicorp-cla-app bot commented Apr 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sreeram77 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sujay-hashicorp commented Apr 23, 2025 •

edited

Loading

hashicorp-cla-app bot commented Apr 23, 2025 •

edited

Loading