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

ankedia · 2021-09-13T19:55:44Z

Added a helm variable with name cloudPlatform to detect the Openshift platform.
For the default (not Openshift) platforms, this change makes sure that files in DOMAIN_HOME dir are at a minimum of umask 027. For this, it (a) restricts the execute permission for shell scripts to user-only, (b) removes group write and execute permissions on boot.properties and (c) preserves permissions when extracting the files from zip file.
For the operator running on OpenShift, this change adds a system property to WLS startup to disable umask 027 checks (all domain styles). For Model-in-image domain on Openshift, it also uses chmod -R g=u {domain home} to give group the same permission as the user for files in DOMAIN_HOME dir.

…ed by the Operator in default and Openshift environment.

…m for all domain types.

jshum2479 · 2021-09-13T21:33:15Z

operator/src/main/java/oracle/kubernetes/weblogic/domain/model/ServerEnvVars.java

  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, CLOUD_PLATFORM);


Is this going to cause operator upgrade rolling side effect ?

I believe it will not cause the rolling side effect, I'll run the upgrade to confirm.

I have verified that rolling doesn't happen during upgrade when kubernetesPlatform helm variable is not present. I add the KUBERNETES_PLATFORM environment variable to server pods only when kubernetesPlatform is set in the helm values.

jshum2479 · 2021-09-13T21:34:43Z

operator/src/main/resources/scripts/startServer.sh

+  if [[ ! -z ${CLOUD_PLATFORM} && ${CLOUD_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


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

@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?

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?

@rjeberhard FYI

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

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

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.

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.

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.")

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.

operator/src/main/resources/scripts/startServer.sh

operator/scripts/operator.sh

kubernetes/charts/weblogic-operator/templates/_operator-dep.tpl

operator/src/main/resources/scripts/startServer.sh

russgold

I see changes to two Java files, but no unit tests for those changes.
[ANIL]
Added following new unit tests in JobHelperTest.java and PodHelperTestBase.java -
whenOperatorHasKubernetesPlatformConfigured_introspectorPodSpecHasKubernetesPlatformEnvVariable
whenNotConfigured_KubernetesPlatform_introspectorPodSpecHasNoKubernetesPlatformEnvVariable
whenOperatorHasKubernetesPlatformConfigured_createdPodSpecContainerHasKubernetesPlatformEnvVariable
whenNotConfigured_KubernetesPlatform_createdPodSpecContainerHasNoKubernetesPlatformEnvVariable

…tead of startServer.sh.

…d unit tests.

…enshift.

…ssions for different domain types.

documentation/staging/content/userguide/platforms/environments.md

documentation/staging/content/security/openshift.md

documentation/staging/content/userguide/platforms/environments.md

documentation/staging/content/userguide/managing-operators/using-helm.md

rjeberhard · 2021-09-20T20:09:02Z

documentation/staging/content/security/openshift.md

@@ -116,3 +116,6 @@ see [OpenShift]({{<relref  "/userguide/platforms/environments#openshift">}}).
 #### Using a dedicated namespace

 When the user that installs an individual instance of the operator does not have the required privileges to create resources at the Kubernetes cluster level, a dedicated namespace can be used for the operator instance and all the WebLogic domains that it manages. For more details about the `dedicated` setting, please refer to [Operator Helm configuration values]({{< relref "/userguide/managing-operators/using-helm#operator-helm-configuration-values" >}}).
+
+#### Set the Helm chart property `kubernetesPlatorm` to `OpenShift`
+Beginning with operator version 4.0, set the operator `kubernetesPlatform` Helm chart property to `OpenShift`. This property accommodates OpenShift security requirements. For more information, see [Operator Helm configuration values]({{<relref "/userguide/managing-operators/using-helm#operator-helm-configuration-values">}}).


I think that we should backport this change to 3.3.2. @mriccell, do you agree?

Changed to "Beginning with operator version 3.3.2".

rjeberhard · 2021-09-20T20:12:19Z

documentation/staging/content/userguide/platforms/environments.md

-This could be either the `anyuid` SCC or a custom one that you define for user/group `1000`. For more information, see [OpenShift]({{<relref "/security/openshift.md">}}) in the Security section.
+To accommodate OpenShift security requirements:
+- For security requirements to run WebLogic in OpenShift, see the [OpenShift chapter]({{<relref "/security/openshift.md">}}) in the Security section.
+- Beginning with operator version 4.0, specify the `kubernetesPlatorm` Helm chart property with value `OpenShift`. For more information, see [Operator Helm configuration values]({{<relref "/userguide/managing-operators/using-helm#operator-helm-configuration-values">}}).


As above, I think that this change should go in to 3.3.2.

Changed to "Beginning with operator version 3.3.2".

…o backport the change to 3.3.2.

…are at a minimum of umask 027 and handle Openshift platform. (#2533) * Prevent insecure file system warnings by ensuring files are at a minimum of umask 027 and handle Openshift platform.

…are at a minimum of umask 027 and handle Openshift platform. (#2533) (#2543) * Prevent insecure file system warnings by ensuring files are at a minimum of umask 027 and handle Openshift platform.

ankedia added 6 commits September 11, 2021 01:19

Changes to prevent insecure file system warnings for MII domain creat…

0d8bd23

…ed by the Operator in default and Openshift environment.

Fix unit test failure.

8bbd89f

Change boot.properties permissions for all domain types.

5ebc832

Preseve permissions when unpacking the domain zip file.

cbd1280

Changes to disable insecure file system warnings on Openshift platfor…

f64e72d

…m for all domain types.

Change unit tests to preserve permissions during untar.

c8d09e4

ankedia requested review from jshum2479, rjeberhard and tbarnes-us September 13, 2021 19:55

jshum2479 reviewed Sep 13, 2021

View reviewed changes

tbarnes-us reviewed Sep 13, 2021

View reviewed changes

operator/src/main/resources/scripts/startServer.sh Outdated Show resolved Hide resolved

operator/scripts/operator.sh Outdated Show resolved Hide resolved

kubernetes/charts/weblogic-operator/templates/_operator-dep.tpl Outdated Show resolved Hide resolved

Changes to address review comments.

7c03475

tbarnes-us reviewed Sep 14, 2021

View reviewed changes

operator/src/main/resources/scripts/startServer.sh Outdated Show resolved Hide resolved

russgold suggested changes Sep 14, 2021

View reviewed changes

ankedia added 5 commits September 15, 2021 04:30

Changes to move 'chmod -R g=u' in the introspector for MII domain ins…

1a16e76

…tead of startServer.sh.

checkstyle fix.

568ac46

Changes to add kubernetesPlatform in the operator config map and adde…

357f5a3

…d unit tests.

Fix checkstyle error.

d3f49eb

Initial cut of documentation for kubernetesPlatform property and Op…

a3e40e9

…enshift.

ankedia requested a review from mriccell September 16, 2021 18:55

Clarify the impact of kubernetesPlatfom helm property on file permi…

af03fbe

…ssions for different domain types.

ankedia marked this pull request as ready for review September 17, 2021 13:41

tbarnes-us reviewed Sep 17, 2021

View reviewed changes

documentation/staging/content/userguide/platforms/environments.md Outdated Show resolved Hide resolved

russgold approved these changes Sep 17, 2021

View reviewed changes

tbarnes-us reviewed Sep 20, 2021

View reviewed changes

ankedia added 2 commits September 20, 2021 14:30

Addressed PR review comments.

a36a2bb

Changes as per discussion in slack thread.

913cab2

rjeberhard reviewed Sep 20, 2021

View reviewed changes

Changed the operator version to 3.3.2 as per Ryan's review comments t…

cc7f026

…o backport the change to 3.3.2.

ankedia requested a review from rjeberhard September 21, 2021 17:04

rjeberhard approved these changes Sep 21, 2021

View reviewed changes

rjeberhard merged commit a78f6b3 into main Sep 21, 2021

ankedia deleted the owls_91448 branch September 22, 2021 17:11

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

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

Uh oh!

Conversation

ankedia commented Sep 13, 2021

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!

jshum2479 Sep 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

tbarnes-us Sep 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

russgold left a comment • edited by ankedia Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

Uh oh!

jshum2479 Sep 13, 2021 •

edited

Loading

tbarnes-us Sep 16, 2021 •

edited

Loading

russgold left a comment •

edited by ankedia

Loading