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.

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

PE-5714 #321

Closed
wants to merge 2 commits into from
Closed

PE-5714 #321

wants to merge 2 commits into from

Conversation

rajeshfrontline
Copy link

No description provided.

@rajeshfrontline rajeshfrontline marked this pull request as draft December 12, 2024 13:54
@@ -529,18 +529,18 @@ harden_system() {
fi

echo "Fix permission of all cron files"
for each in `echo /etc/cron.daily /etc/cron.hourly /etc/cron.d /etc/cron.monthly /etc/cron.weekly /etc/crontab`
for each in echo /etc/cron.daily /etc/cron.hourly /etc/cron.d /etc/cron.monthly /etc/cron.weekly /etc/crontab
Copy link
Contributor

Choose a reason for hiding this comment

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

This will not work as expected. the recommendation was to use the commands $(...) notation instead of legacy backticks

cis-harden/harden.sh Outdated Show resolved Hide resolved
fi
fi
done

echo "Remove cron and at deny files and have allow files in place"
echo "Remove cron and at deny files anf have allow files in place"
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you fix the typo in this line ?

Copy link
Author

Choose a reason for hiding this comment

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

updated.

echo "auth sufficient pam_faillock.so authsucc audit deny=4 fail_interval=900 unlock_time=600"
echo "auth requisite pam_deny.so"
echo "auth required pam_permit.so"
} > /etc/pam.d/common-auth
Copy link
Contributor

Choose a reason for hiding this comment

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

Using this approach, will the content get appended if the file has existing content?

Copy link
Author

Choose a reason for hiding this comment

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

updated with >> /etc/pam.d/common-auth

echo "Privileged containers are not allowed for the current user."
exit 1
fi
if [ -z "$HTTP_PROXY" ] && [ -z "$HTTPS_PROXY" ] && [ -z "$(find certs -type f ! -name '.*' -print -quit)" ]; then
if [ -z "$HTTP_PROXY" ] && [ -z "$HTTPS_PROXY"]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

The 3rd condition is missing ?

Copy link
Author

Choose a reason for hiding this comment

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

updated.

@@ -94,14 +94,6 @@ PE_VERSION=$(git describe --abbrev=0 --tags)
SPECTRO_PUB_REPO=us-docker.pkg.dev/palette-images
EARTHLY_VERSION=v0.8.15
source .arg

# Workaround to support deprecated field PROXY_CERT_PATH
Copy link
Contributor

Choose a reason for hiding this comment

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

This special handling is missing in your changes. Could you check it out again ?

@rajeshfrontline
Copy link
Author

closing this PR, have a new one to work on the updates.

@rajeshfrontline rajeshfrontline deleted the PE-5714 branch December 27, 2024 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants