Skip to content

Owls 91448 - Prevent insecure file system warnings by ensuring files are at a minimum of umask 027 and handle Openshift platform. #2533

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.

Sign up for GitHub

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

Merged
merged 16 commits into from
Sep 21, 2021
Merged

Owls 91448 - Prevent insecure file system warnings by ensuring files are at a minimum of umask 027 and handle Openshift platform. #2533

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
0d8bd23
Changes to prevent insecure file system warnings for MII domain creat…
ankedia Sep 11, 2021
8bbd89f
Fix unit test failure.
ankedia Sep 11, 2021
5ebc832
Change boot.properties permissions for all domain types.
ankedia Sep 13, 2021
cbd1280
Preseve permissions when unpacking the domain zip file.
ankedia Sep 13, 2021
f64e72d
Changes to disable insecure file system warnings on Openshift platfor…
ankedia Sep 13, 2021
c8d09e4
Change unit tests to preserve permissions during untar.
ankedia Sep 13, 2021
7c03475
Changes to address review comments.
ankedia Sep 14, 2021
1a16e76
Changes to move 'chmod -R g=u' in the introspector for MII domain ins…
ankedia Sep 15, 2021
568ac46
checkstyle fix.
ankedia Sep 15, 2021
357f5a3
Changes to add kubernetesPlatform in the operator config map and adde…
ankedia Sep 16, 2021
d3f49eb
Fix checkstyle error.
ankedia Sep 16, 2021
a3e40e9
Initial cut of documentation for `kubernetesPlatform` property and Op…
ankedia Sep 16, 2021
af03fbe
Clarify the impact of `kubernetesPlatfom` helm property on file permi…
ankedia Sep 17, 2021
a36a2bb
Addressed PR review comments.
ankedia Sep 20, 2021
913cab2
Changes as per discussion in slack thread.
ankedia Sep 20, 2021
cc7f026
Changed the operator version to 3.3.2 as per Ryan's review comments t…
ankedia Sep 21, 2021
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
2 changes: 2 additions & 0 deletions kubernetes/charts/weblogic-operator/templates/_operator-dep.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ spec:
value: "false"
- name: "JAVA_LOGGING_LEVEL"
value: {{ .javaLoggingLevel | quote }}
- name: "KUBERNETES_PLATFORM"
value: {{ .kubernetesPlatform | quote }}
- name: "JAVA_LOGGING_MAXSIZE"
value: {{ .javaLoggingFileSizeLimit | default 20000000 | quote }}
- name: "JAVA_LOGGING_COUNT"
Expand Down
6 changes: 5 additions & 1 deletion operator/scripts/operator.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ if [ "${MOCK_WLS}" == 'true' ]; then
MOCKING_WLS="-DmockWLS=true"
fi

if [ ! -z "${KUBERNETES_PLATFORM}" ]; then
PLATFORM="-DkubernetesPlatform=$KUBERNETES_PLATFORM"
fi

LOGGING="-Djava.util.logging.config.file=${LOGGING_CONFIG}"
mkdir -m 777 -p /logs
cp /operator/logstash.conf /logs/logstash.conf
Expand All @@ -69,6 +73,6 @@ cp /operator/logstash.conf /logs/logstash.conf
HEAP="-XshowSettings:vm"

# Start operator
java $HEAP $MOCKING_WLS $DEBUG $LOGGING -jar /operator/weblogic-kubernetes-operator.jar &
java $HEAP $MOCKING_WLS $DEBUG $LOGGING $PLATFORM -jar /operator/weblogic-kubernetes-operator.jar &
PID=$!
wait $PID
5 changes: 5 additions & 0 deletions operator/src/main/java/oracle/kubernetes/operator/helpers/PodStepContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -929,6 +929,7 @@ void addStartupEnvVars(List<V1EnvVar> vars) {
Optional.ofNullable(getDataHome()).ifPresent(v -> addEnvVar(vars, ServerEnvVars.DATA_HOME, v));
Optional.ofNullable(getServerSpec().getAuxiliaryImages()).ifPresent(cm -> addAuxiliaryImageEnv(cm, vars));
addEnvVarIfTrue(mockWls(), vars, "MOCK_WLS");
Optional.ofNullable(getKubernetesPlatform()).ifPresent(v -> addEnvVar(vars, ServerEnvVars.KUBERNETES_PLATFORM, v));
}

protected void addAuxiliaryImageEnv(List<AuxiliaryImage> auxiliaryImageList, List<V1EnvVar> vars) {
Expand Down Expand Up @@ -1079,6 +1080,10 @@ private boolean mockWls() {
return Boolean.getBoolean("mockWLS");
}

private String getKubernetesPlatform() {
return System.getProperty("kubernetesPlatform");
}

private abstract class BaseStep extends Step {
BaseStep() {
this(null);
Expand Down
4 changes: 3 additions & 1 deletion operator/src/main/java/oracle/kubernetes/weblogic/domain/model/ServerEnvVars.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,12 @@ public class ServerEnvVars {
/** If present, pod scripts will watch for changes to override configurations and move them into place. */
public static final String DYNAMIC_CONFIG_OVERRIDE = "DYNAMIC_CONFIG_OVERRIDE";

public static final String KUBERNETES_PLATFORM = "KUBERNETES_PLATFORM";

private static final List<String> RESERVED_NAMES = Arrays.asList(
DOMAIN_UID, DOMAIN_NAME, DOMAIN_HOME, NODEMGR_HOME, SERVER_NAME, SERVICE_NAME,
ADMIN_NAME, AS_SERVICE_NAME, ADMIN_PORT, ADMIN_PORT_SECURE, ADMIN_SERVER_PORT_SECURE,
LOG_HOME, SERVER_OUT_IN_POD_LOG, DATA_HOME, ACCESS_LOG_IN_LOG_HOME, DYNAMIC_CONFIG_OVERRIDE);
LOG_HOME, SERVER_OUT_IN_POD_LOG, DATA_HOME, ACCESS_LOG_IN_LOG_HOME, DYNAMIC_CONFIG_OVERRIDE, KUBERNETES_PLATFORM);

static boolean isReserved(String name) {
return RESERVED_NAMES.contains(name);
Expand Down
6 changes: 3 additions & 3 deletions operator/src/main/resources/scripts/modelInImage.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,7 @@ function createModelDomain() {
trace "Using newly created domain"
elif [ -f ${PRIMORDIAL_DOMAIN_ZIPPED} ] ; then
trace "Using existing primordial domain"
cd / && base64 -d ${PRIMORDIAL_DOMAIN_ZIPPED} > ${LOCAL_PRIM_DOMAIN_ZIP} && tar -xzf ${LOCAL_PRIM_DOMAIN_ZIP}
cd / && base64 -d ${PRIMORDIAL_DOMAIN_ZIPPED} > ${LOCAL_PRIM_DOMAIN_ZIP} && tar -pxzf ${LOCAL_PRIM_DOMAIN_ZIP}
# create empty lib since we don't archive it in primordial zip and WDT will fail without it
mkdir ${DOMAIN_HOME}/lib
# Since the SerializedSystem ini is encrypted, restore it first
Expand All @@ -564,7 +564,7 @@ function createModelDomain() {
function restoreDomainConfig() {
restoreEncodedTar "domainzip.secure" || return 1

chmod +x ${DOMAIN_HOME}/bin/*.sh ${DOMAIN_HOME}/*.sh || return 1
chmod u+x ${DOMAIN_HOME}/bin/*.sh ${DOMAIN_HOME}/*.sh || return 1
}

# Expands into the root directory the MII primordial domain, stored in one or more config maps
Expand All @@ -580,7 +580,7 @@ function restoreEncodedTar() {
cat $(ls ${OPERATOR_ROOT}/introspector*/${1} | sort -t- -k3) > /tmp/domain.secure || return 1
base64 -d "/tmp/domain.secure" > /tmp/domain.tar.gz || return 1

tar -xzf /tmp/domain.tar.gz || return 1
tar -pxzf /tmp/domain.tar.gz || return 1
}

# This is before WDT compareModel implementation
Expand Down
15 changes: 15 additions & 0 deletions operator/src/main/resources/scripts/startServer.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,12 +290,27 @@ createFolder ${DOMAIN_HOME}/servers/${SERVER_NAME}/security
copyIfChanged /weblogic-operator/introspector/boot.properties \
${DOMAIN_HOME}/servers/${SERVER_NAME}/security/boot.properties

# remove write and execute permissions for group to prevent insecure file system warnings.
chmod g-wx ${DOMAIN_HOME}/servers/${SERVER_NAME}/security/boot.properties


if [ ${DOMAIN_SOURCE_TYPE} != "FromModel" ]; then
trace "Copying situational configuration files from operator cm to ${DOMAIN_HOME}/optconfig directory"
copySitCfgWhileBooting /weblogic-operator/introspector ${DOMAIN_HOME}/optconfig 'Sit-Cfg-CFG--'
copySitCfgWhileBooting /weblogic-operator/introspector ${DOMAIN_HOME}/optconfig/jms 'Sit-Cfg-JMS--'
copySitCfgWhileBooting /weblogic-operator/introspector ${DOMAIN_HOME}/optconfig/jdbc 'Sit-Cfg-JDBC--'
copySitCfgWhileBooting /weblogic-operator/introspector ${DOMAIN_HOME}/optconfig/diagnostics 'Sit-Cfg-WLDF--'
else
if [[ ${KUBERNETES_PLATFORM^^} == "OPENSHIFT" ]]; then
# Operator running on Openshift platform - change file permissions in the DOMAIN_HOME dir to give
# group same permissions as user .
chmod -R g=u ${DOMAIN_HOME} || return 1
Copy link
Member

@jshum2479 jshum2479 Sep 13, 2021

Choose a reason for hiding this comment

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

Should we also do it for non mii case? Or this is left as an exercise for the user when they create the domain?

Copy link

@tbarnes-us tbarnes-us Sep 13, 2021

Choose a reason for hiding this comment

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

@jshum2479

I originally suggested that we handle the non-MII cases, but I have a similar concern. In detail:

For one, a recursive file traversal of the entire domain-home seems like it could be prohibitively expensive, especially as it's done for every pod cycle.

For DII in particular, maybe this can this occur solely when the image is created (and be left up to the customer)?

And for DiPV, if we must do something automatic, can it be limited to occur only in the introspector, and, even then, only the first time the introspector sees the domain?

Copy link
Member

@jshum2479 jshum2479 Sep 13, 2021

Choose a reason for hiding this comment

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

I agreed, doing it in the introspector (before zipping up the domain) is a better place. Do we really need -R or is the check only for bin directory?

Copy link

@tbarnes-us tbarnes-us Sep 13, 2021

Choose a reason for hiding this comment

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

@rjeberhard FYI

Copy link
Member

@jshum2479 jshum2479 Sep 13, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Also, have we explored doing it in WDT also, it won't solve existing domain or one created with older WDT though?

Copy link
Member

@jshum2479 jshum2479 Sep 13, 2021

Choose a reason for hiding this comment

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

Sorry, one more question. Does the new was check provide any guidance on JKS keystore files? Best practice is user read only.

Copy link
Member Author

@ankedia ankedia Sep 13, 2021

Choose a reason for hiding this comment

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

I'll set up a meeting to go over this. As per the requirements in JIRA OWLS-91448, we should NOT use chmod -R g=u for Model-in-image domain unless the environment is OpenShift. My understanding from JIRA was that, DII case will be handled in image tool and it's customers responsibility to do it for domain-on-pv.

Copy link
Member Author

@ankedia ankedia Sep 16, 2021

Choose a reason for hiding this comment

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

I have made the changes to perform the chmod -R g=u ${DOMAIN_HOME} in the introspector for model-in-image domain and to perform chmod -R g=u ${DOMAIN_HOME} in startServer.sh script for domain-home-in-image. My understanding is that we should not be doing chmod -R for the domain-on-pv domains since those changes will be persistent.

Copy link

@tbarnes-us tbarnes-us Sep 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

If the image tool is going to handle DII then why does startServer need to? (You wrote "DII case will be handled in image tool " and "I have made the changes to perform ... chmod -R g=u ${DOMAIN_HOME} in startServer.sh script for domain-home-in-image.")

Copy link
Member Author

@ankedia ankedia Sep 16, 2021

Choose a reason for hiding this comment

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

My comment "DII case will be handled in the image tool" on 9/13 was based on my previous understanding. However, in the standup meeting discussion on Tuesday 9/14, Derek and Robert Patrick suggested to also do chmod -R u=g ${DOMAIN_HOME} for DII in the operator.

fi
fi

if [[ ${KUBERNETES_PLATFORM^^} == "OPENSHIFT" ]]; then
# When the Operator is running on Openshift platform, disable insecure file system warnings.
export JAVA_OPTIONS="-Dweblogic.SecureMode.WarnOnInsecureFileSystem=false $JAVA_OPTIONS"
fi

#
Expand Down
8 changes: 4 additions & 4 deletions operator/src/test/sh/modelInImageTest.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,13 +82,13 @@ testOnRestoreDomainConfig_base64DecodeZip() {
testOnRestoreDomainConfig_unTarDomain() {
restoreDomainConfig

assertEquals "TAR command arguments" "-xzf /tmp/domain.tar.gz" "$TAR_ARGS"
assertEquals "TAR command arguments" "-pxzf /tmp/domain.tar.gz" "$TAR_ARGS"
}

testOnRestoreDomainConfig_makeScriptsExecutable() {
restoreDomainConfig

assertEquals "CD command arguments" "+x ${DOMAIN_HOME}/bin/*.sh ${DOMAIN_HOME}/*.sh" "$CHMOD_ARGS"
assertEquals "CD command arguments" "u+x ${DOMAIN_HOME}/bin/*.sh ${DOMAIN_HOME}/*.sh" "$CHMOD_ARGS"
}

testOnRestorePrimordialDomain_useRootDirectory() {
Expand Down Expand Up @@ -119,7 +119,7 @@ testOnRestoreDomainConfig_whenNoIndexesDefinedCatSingleFile() {
testOnRestorePrimordialDomain_unTarDomain() {
restorePrimordialDomain

assertEquals "TAR command arguments" "-xzf /tmp/domain.tar.gz" "$TAR_ARGS"
assertEquals "TAR command arguments" "-pxzf /tmp/domain.tar.gz" "$TAR_ARGS"
}

######################### Mocks for the tests ###############
Expand Down Expand Up @@ -168,4 +168,4 @@ chmod() {
. ${SCRIPTPATH}/modelInImage.sh

# shellcheck source=target/classes/shunit/shunit2
. ${SHUNIT2_PATH}
. ${SHUNIT2_PATH}