Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions pkg/cmd/render/render.go
Original file line number Diff line number Diff line change
Expand Up @@ -306,13 +306,13 @@ func (r *renderOpts) Run() error {
certDir := filepath.Join(memberDir, "etcd-all-certs")

// Creating the cert dir recursively will create the base path too
err = os.MkdirAll(certDir, 0755)
err = os.MkdirAll(certDir, 0700)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you sure that this changes the dataDir?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only location I could find in the codebase where the /var/lib/etcd dir is created, via the memberDir.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also curious about the permission of "/var/lib/etcd" which set to 0755

but I'm afraid u got the wrong place

maybe u could change the dir permission in the following place just like :

mkdir -p /var/log/etcd && chmod 0600 /var/log/etcd

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the reason is that the entire folder structure is on 0755:

sh-5.1# stat --format '%a' /var/    
755
sh-5.1# stat --format '%a' /var/lib/etcd/
755 

same with other openshift folders:

sh-5.1# stat --format '%a' /var/lib/ovn-ic/                 
755
sh-5.1# stat --format '%a' /var/lib/kubelet/
755

so I would rather advocate to change the CIS rule.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lance5890 you're right, I'm mistaken. I will take a look at an initContainers approach. Should I turn this into a draft / WIP?

@tjungblu
I think you're right.

I couldn't find the /var/lib/etcd dir explicitly created by the operator. But, doing some digging through older issues, it sounds like the kubelet will create the dir with 0755 if it doesn't exist (not sure how much has changed since 2018...)

kubernetes/kubeadm#1308 (comment)

so I would rather advocate to change the CIS rule.

Looks like 0700 is also part of the STIG: https://github.com/ComplianceAsCode/content/blob/49189d32c6039a5e4ca68f6e4a04de1f719e47b2/products/ocp4/profiles/stig-node-v1r1.profile#L62

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shaneboulden are you sure that the data dir is part of the STIG? https://stigviewer.com/stigs/kubernetes
The data files I agree and understand, but the datadir itself seems pretty strange to me.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume that the STIG is generated from the git repo ;)
Oz is not around anymore for a few years, so we won't be able to ask him. I remember my old team was writing a few of those rules too.

Tracking this back to 2020: ComplianceAsCode/content#6341 and ComplianceAsCode/content#6341 (comment)

not sure whether this ever worked, it also seems an additional check that is not listed in the initial ticket.

if err != nil {
return fmt.Errorf("failed to create directory %s: %w", memberDir, err)
}
// tlsDir contains the ca bundle and client cert pair for bootkube.sh and the bootstrap apiserver
tlsDir := filepath.Join(r.assetOutputDir, "tls")
err = os.MkdirAll(tlsDir, 0755)
err = os.MkdirAll(tlsDir, 0700)

// Write the etcd ca bundle required by the bootstrap etcd member
for _, bundle := range templateData.caBundles {
Expand Down
2 changes: 1 addition & 1 deletion pkg/operator/etcd_assets/bindata.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/tnf/assets/bindata.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.