[java-identity-provider] branch main updated: IDP-1769 Installer locks down plugin contents files and strips write access

Rod Widdowson rdw at steadingsoftware.com
Wed Mar 10 11:29:48 UTC 2021


This is an automated email from the git hooks/post-receive script.

rdw pushed a commit to branch main
in repository java-identity-provider.

View the commit online:
http://git.shibboleth.net/view/?p=java-identity-provider.git;a=commit;h=af409f33c11e5213d5773cf5fc6d157b56e4d4fd

The following commit(s) were added to refs/heads/main by this push:
       new  af409f33c IDP-1769 Installer locks down plugin contents files and strips write access
af409f33c is described below

commit af409f33c11e5213d5773cf5fc6d157b56e4d4fd
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Wed Mar 10 11:14:38 2021 +0000

    IDP-1769 Installer locks down plugin contents files and strips write access
    
    https://issues.shibboleth.net/jira/browse/IDP-1769
    
    Extend the previous fix to plugin-webapp.  Refactor some code
    for clarity.
---
 .../shibboleth/idp/installer/InstallerSupport.java | 12 ++++++++++++
 .../net/shibboleth/idp/installer/V4Install.java    | 11 +++++++----
 .../idp/installer/plugin/impl/PluginInstaller.java | 22 ++++++++++++----------
 3 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/InstallerSupport.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/InstallerSupport.java
index bc623b08a..7eb8dee97 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/InstallerSupport.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/InstallerSupport.java
@@ -195,6 +195,10 @@ public final class InstallerSupport {
             log.debug("Not windows. Not [re]setting readonly bit");
             return;
         }
+        if (!Files.exists(path) ) {
+            log.debug("Directory {} does not exist, not performing Attrib -/+r", path);
+            return;
+        }
         if (Files.isDirectory(path)) {
             setReadOnlyDir(path, readOnly);
         } else {
@@ -210,6 +214,10 @@ public final class InstallerSupport {
      */
     public static void setMode(final Path directory, final String permissions, final String includes)
             throws BuildException {
+        if (!Files.exists(directory) ) {
+            log.debug("Directory {} does not exist, not performing chmod", directory);
+            return;
+        }
         log.debug("Performing chmod {} on {} including {}", permissions, directory, includes);
         if (Os.isFamily(Os.FAMILY_WINDOWS)) {
             log.debug("Windows. Not performing chmod");
@@ -232,6 +240,10 @@ public final class InstallerSupport {
      */
     public static void setGroup(final Path directory, final String group, final String includes)
             throws BuildException {
+        if (!Files.exists(directory) ) {
+            log.debug("Directory {} does not exist, not performing chgrp", directory);
+            return;
+        }
         log.debug("Performing chgrp {} on {} including {}", group, directory, includes);
         if (Os.isFamily(Os.FAMILY_WINDOWS)) {
             log.debug("Windows. Not performing chown");
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/V4Install.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/V4Install.java
index 2a3bfd5d0..941f28fd4 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/V4Install.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/V4Install.java
@@ -487,7 +487,12 @@ public class V4Install extends AbstractInitializableComponent {
      * @throws BuildException if badness occurs
      */
     protected void reprotect() throws BuildException {
+        final Path pluginContents = installerProps.getTargetDir().resolve("dist").resolve("plugin-contents");
+        final Path pluginWebapp = installerProps.getTargetDir().resolve("dist").resolve("plugin-webapp");
+
         InstallerSupport.setReadOnly(installerProps.getTargetDir().resolve("dist"), true);
+        InstallerSupport.setReadOnly(pluginContents, false);
+        InstallerSupport.setReadOnly(pluginWebapp, false);
         if (currentState.isSystemPresent()) {
             InstallerSupport.setReadOnly(installerProps.getTargetDir().resolve("system"), true);
         }
@@ -498,10 +503,8 @@ public class V4Install extends AbstractInitializableComponent {
                 InstallerSupport.setMode(installerProps.getTargetDir().resolve("system"), "444", "**/*");
             }
             InstallerSupport.setMode(installerProps.getTargetDir().resolve("dist"), "444", "**/*");
-            final Path pluginContents = installerProps.getTargetDir().resolve("dist").resolve("plugin-contents");
-            if (Files.exists(pluginContents)) {
-                InstallerSupport.setMode(pluginContents,  "640", "**/*");
-            }
+            InstallerSupport.setMode(pluginContents,  "640", "**/*");
+            InstallerSupport.setMode(pluginWebapp,  "640", "**/*");
             if (currentState.getInstalledVersion() == null) {
                 InstallerSupport.setMode(installerProps.getTargetDir().resolve("credentials"),
                         installerProps.getCredentialsKeyFileMode(), "**/*");
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstaller.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstaller.java
index 73f6beb8d..59b84ace0 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstaller.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstaller.java
@@ -133,6 +133,9 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
     /** Pluginss webapp. */
     @NonnullAfterInit private Path pluginsWebapp;
 
+    /** Pluginss webapp. */
+    @NonnullAfterInit private Path pluginsContents;
+
     /** What was installed - this is setup by {@link #loadCopiedFiles()}. */
     @Nullable private List<String> installedContents;
 
@@ -328,7 +331,7 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
             builder.execute();
             LOG.info("Removed resources for {} from the war", pluginId);
         }
-        distPath.resolve("plugin-contents").resolve(pluginId).toFile().deleteOnExit();
+        pluginsContents.resolve(pluginId).toFile().deleteOnExit();
     }
 
     /** Get hold of the {@link IdPPlugin} for this plugin.
@@ -482,12 +485,7 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
      */
     private void saveCopiedFiles(final List<Path> copiedFiles) throws BuildException {
         try {
-            final Path parent = distPath.resolve("plugin-contents");
-            // Just in case it has been deprotected
-            if (Files.exists(parent)) {
-                InstallerSupport.setMode(parent, "640", "**/*");
-            }
-            Files.createDirectories(parent);
+            Files.createDirectories(pluginsContents);
             final Properties props = new Properties(1+copiedFiles.size());
             props.setProperty("idp.plugin.version",
                     new PluginVersion(description).toString());
@@ -496,7 +494,7 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
                 props.setProperty("idp.plugin.file."+Integer.toString(count++),
                         PluginInstallerSupport.canonicalPath(p).toString());
             }
-            final File outFile = parent.resolve(pluginId).toFile();
+            final File outFile = pluginsContents.resolve(pluginId).toFile();
             try (final BufferedOutputStream out = new BufferedOutputStream(new FileOutputStream(outFile))) {
                 props.store(out, "Files Copied "  + Instant.now());
             }
@@ -514,9 +512,8 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
         if (installedContents != null) {
             return;
         }
-        final Path parent = distPath.resolve("plugin-contents");
         final Properties props = new Properties();
-        final File inFile = parent.resolve(pluginId).toFile();
+        final File inFile = pluginsContents.resolve(pluginId).toFile();
         if (!inFile.exists()) {
             LOG.debug("Contents file for plugin {} ({}) does not exist", pluginId, inFile.getAbsolutePath());
             installedContents = Collections.emptyList();
@@ -767,7 +764,12 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
         distPath = idpHome.resolve("dist");
         workspacePath = distPath.resolve("plugin-workspace");
         pluginsWebapp = distPath.resolve("plugin-webapp");
+        pluginsContents = distPath.resolve("plugin-contents");
         InstallerSupport.setReadOnly(distPath, false);
+        // Just in case they have been protected
+        InstallerSupport.setMode(workspacePath, "640", "**/*");
+        InstallerSupport.setMode(pluginsWebapp, "640", "**/*");
+        InstallerSupport.setMode(pluginsContents, "640", "**/*");
     }
 
     /** Generate a {@link URLClassLoader} which looks at the

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list