From 0a2da6012df36eb58f0c288533494a1160166d9f Mon Sep 17 00:00:00 2001 From: James Perkins Date: Mon, 8 Jun 2020 08:20:13 -0700 Subject: [PATCH] [LOGMGR-275] Use privileged actions for rotating files as the security context is not guaranteed to be the system level context in WildFly. https://issues.redhat.com/browse/LOGMGR-275 --- .../handlers/PeriodicRotatingFileHandler.java | 29 ++- .../PeriodicSizeRotatingFileHandler.java | 39 +++- .../ext/handlers/SecurityActions.java | 68 +++++++ .../ext/handlers/SizeRotatingFileHandler.java | 41 +++-- .../ext/handlers/SuffixRotator.java | 168 +++++++++++++++--- 5 files changed, 302 insertions(+), 43 deletions(-) create mode 100644 ext/src/main/java/org/jboss/logmanager/ext/handlers/SecurityActions.java diff --git a/ext/src/main/java/org/jboss/logmanager/ext/handlers/PeriodicRotatingFileHandler.java b/ext/src/main/java/org/jboss/logmanager/ext/handlers/PeriodicRotatingFileHandler.java index 323f1407..f54a2626 100644 --- a/ext/src/main/java/org/jboss/logmanager/ext/handlers/PeriodicRotatingFileHandler.java +++ b/ext/src/main/java/org/jboss/logmanager/ext/handlers/PeriodicRotatingFileHandler.java @@ -22,6 +22,10 @@ import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; +import java.io.UncheckedIOException; +import java.security.AccessControlContext; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.text.SimpleDateFormat; import java.util.Calendar; import java.util.Date; @@ -37,6 +41,7 @@ */ public class PeriodicRotatingFileHandler extends FileHandler { + private final AccessControlContext acc = AccessController.getContext(); private SimpleDateFormat format; private String nextSuffix; private Period period = Period.NEVER; @@ -129,7 +134,7 @@ protected void preWrite(final ExtLogRecord record) { * @throws IllegalArgumentException if the suffix is not valid */ public void setSuffix(String suffix) throws IllegalArgumentException { - final SuffixRotator suffixRotator = SuffixRotator.parse(suffix); + final SuffixRotator suffixRotator = SuffixRotator.parse(acc, suffix); final String dateSuffix = suffixRotator.getDatePattern(); final SimpleDateFormat format = new SimpleDateFormat(dateSuffix); format.setTimeZone(timeZone); @@ -193,11 +198,11 @@ private void rollOver() { try { final File file = getFile(); // first, close the original file (some OSes won't let you move/rename a file that is open) - setFile(null); + setFileInternal(null); // next, rotate it - suffixRotator.rotate(getErrorManager(), file.toPath(), nextSuffix); + suffixRotator.rotate(SecurityActions.getErrorManager(acc, this), file.toPath(), nextSuffix); // start new file - setFile(file); + setFileInternal(file); } catch (IOException e) { reportError("Unable to rotate log file", e, ErrorManager.OPEN_FAILURE); } @@ -293,6 +298,22 @@ public void setTimeZone(final TimeZone timeZone) { this.timeZone = timeZone; } + private void setFileInternal(final File file) throws FileNotFoundException { + // At this point we should have already checked the security required + if (System.getSecurityManager() == null) { + super.setFile(file); + } else { + AccessController.doPrivileged((PrivilegedAction) () -> { + try { + super.setFile(file); + } catch (FileNotFoundException e) { + throw new UncheckedIOException(e); + } + return null; + }, acc); + } + } + private static > T min(T a, T b) { return a.compareTo(b) <= 0 ? a : b; } diff --git a/ext/src/main/java/org/jboss/logmanager/ext/handlers/PeriodicSizeRotatingFileHandler.java b/ext/src/main/java/org/jboss/logmanager/ext/handlers/PeriodicSizeRotatingFileHandler.java index 29eb3efa..c22746ef 100644 --- a/ext/src/main/java/org/jboss/logmanager/ext/handlers/PeriodicSizeRotatingFileHandler.java +++ b/ext/src/main/java/org/jboss/logmanager/ext/handlers/PeriodicSizeRotatingFileHandler.java @@ -23,6 +23,10 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.io.OutputStream; +import java.io.UncheckedIOException; +import java.security.AccessControlContext; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.util.logging.ErrorManager; import org.jboss.logmanager.ExtLogRecord; @@ -38,6 +42,7 @@ * @author James R. Perkins */ public class PeriodicSizeRotatingFileHandler extends PeriodicRotatingFileHandler { + private final AccessControlContext acc = AccessController.getContext(); // by default, rotate at 10MB private long rotateSize = 0xa0000L; private int maxBackupIndex = 1; @@ -148,6 +153,7 @@ public void setOutputStream(final OutputStream outputStream) { */ @Override public void setFile(final File file) throws FileNotFoundException { + checkAccess(); synchronized (outputLock) { // Check for a rotate if (rotateOnBoot && maxBackupIndex > 0 && file != null && file.exists() && file.length() > 0L) { @@ -155,11 +161,11 @@ public void setFile(final File file) throws FileNotFoundException { final SuffixRotator suffixRotator = getSuffixRotator(); if (suffixRotator != SuffixRotator.EMPTY && suffix != null) { // Make sure any previous files are closed before we attempt to rotate - setFileInternal(null); + setFileInternal(null, false); suffixRotator.rotate(getErrorManager(), file.toPath(), suffix, maxBackupIndex); } } - setFileInternal(file); + setFileInternal(file, false); } } @@ -224,19 +230,34 @@ protected void preWrite(final ExtLogRecord record) { return; } // close the old file. - setFileInternal(null); - getSuffixRotator().rotate(getErrorManager(), file.toPath(), getNextSuffix(), maxBackupIndex); + setFileInternal(null, true); + getSuffixRotator().rotate(SecurityActions.getErrorManager(acc, this), file.toPath(), getNextSuffix(), maxBackupIndex); // start with new file. - setFileInternal(file); + setFileInternal(file, true); } catch (IOException e) { reportError("Unable to rotate log file", e, ErrorManager.OPEN_FAILURE); } } } - private void setFileInternal(final File file) throws FileNotFoundException { - super.setFile(file); - if (outputStream != null) - outputStream.currentSize = file == null ? 0L : file.length(); + private void setFileInternal(final File file, final boolean doPrivileged) throws FileNotFoundException { + if (System.getSecurityManager() == null || !doPrivileged) { + super.setFile(file); + if (outputStream != null) { + outputStream.currentSize = file == null ? 0L : file.length(); + } + } else { + AccessController.doPrivileged((PrivilegedAction) () -> { + try { + super.setFile(file); + if (outputStream != null) { + outputStream.currentSize = file == null ? 0L : file.length(); + } + } catch (FileNotFoundException e) { + throw new UncheckedIOException(e); + } + return null; + }, acc); + } } } diff --git a/ext/src/main/java/org/jboss/logmanager/ext/handlers/SecurityActions.java b/ext/src/main/java/org/jboss/logmanager/ext/handlers/SecurityActions.java new file mode 100644 index 00000000..d318e525 --- /dev/null +++ b/ext/src/main/java/org/jboss/logmanager/ext/handlers/SecurityActions.java @@ -0,0 +1,68 @@ +/* + * JBoss, Home of Professional Open Source. + * + * Copyright 2020 Red Hat, Inc., and individual contributors + * as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.jboss.logmanager.ext.handlers; + +import java.security.AccessControlContext; +import java.security.AccessController; +import java.security.PrivilegedAction; +import java.util.function.Supplier; +import java.util.logging.ErrorManager; +import java.util.logging.Handler; + +/** + * @author James R. Perkins + */ +class SecurityActions { + + static ErrorManager getErrorManager(final AccessControlContext acc, final Handler handler) { + Supplier supplier = () -> { + if (System.getSecurityManager() == null) { + return handler.getErrorManager(); + } + return AccessController.doPrivileged((PrivilegedAction) handler::getErrorManager, acc); + }; + return new LazyErrorManager(supplier); + } + + private static class LazyErrorManager extends ErrorManager { + private final Supplier supplier; + private volatile ErrorManager delegate; + + private LazyErrorManager(final Supplier supplier) { + this.supplier = supplier; + } + + @Override + public synchronized void error(final String msg, final Exception ex, final int code) { + getDelegate().error(msg, ex, code); + } + + private ErrorManager getDelegate() { + if (delegate == null) { + synchronized (this) { + if (delegate == null) { + delegate = supplier.get(); + } + } + } + return delegate; + } + } +} diff --git a/ext/src/main/java/org/jboss/logmanager/ext/handlers/SizeRotatingFileHandler.java b/ext/src/main/java/org/jboss/logmanager/ext/handlers/SizeRotatingFileHandler.java index f5b29393..ea432a9a 100644 --- a/ext/src/main/java/org/jboss/logmanager/ext/handlers/SizeRotatingFileHandler.java +++ b/ext/src/main/java/org/jboss/logmanager/ext/handlers/SizeRotatingFileHandler.java @@ -23,12 +23,17 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.io.OutputStream; +import java.io.UncheckedIOException; +import java.security.AccessControlContext; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.util.logging.ErrorManager; import org.jboss.logmanager.ExtLogRecord; import org.jboss.logmanager.handlers.FileHandler; public class SizeRotatingFileHandler extends FileHandler { + private final AccessControlContext acc = AccessController.getContext(); // by default, rotate at 10MB private long rotateSize = 0xa0000L; private int maxBackupIndex = 1; @@ -137,14 +142,15 @@ public void setOutputStream(final OutputStream outputStream) { * @throws RuntimeException if there is an attempt to rotate file and the rotation fails */ public void setFile(final File file) throws FileNotFoundException { + checkAccess(); synchronized (outputLock) { // Check for a rotate if (rotateOnBoot && maxBackupIndex > 0 && file != null && file.exists() && file.length() > 0L) { // Make sure any previous files are closed before we attempt to rotate - setFileInternal(null); + setFileInternal(null, false); suffixRotator.rotate(getErrorManager(), file.toPath(), maxBackupIndex); } - setFileInternal(file); + setFileInternal(file, false); } } @@ -229,7 +235,7 @@ public String getSuffix() { public void setSuffix(final String suffix) { checkAccess(); synchronized (outputLock) { - this.suffixRotator = SuffixRotator.parse(suffix); + this.suffixRotator = SuffixRotator.parse(acc, suffix); } } @@ -245,19 +251,34 @@ protected void preWrite(final ExtLogRecord record) { return; } // close the old file. - setFileInternal(null); - suffixRotator.rotate(getErrorManager(), file.toPath(), maxBackupIndex); + setFileInternal(null, true); + suffixRotator.rotate(SecurityActions.getErrorManager(acc, this), file.toPath(), maxBackupIndex); // start with new file. - setFileInternal(file); + setFileInternal(file, true); } catch (IOException e) { reportError("Unable to rotate log file", e, ErrorManager.OPEN_FAILURE); } } } - private void setFileInternal(final File file) throws FileNotFoundException { - super.setFile(file); - if (outputStream != null) - outputStream.currentSize = file == null ? 0L : file.length(); + private void setFileInternal(final File file, final boolean doPrivileged) throws FileNotFoundException { + if (System.getSecurityManager() == null || !doPrivileged) { + super.setFile(file); + if (outputStream != null) { + outputStream.currentSize = file == null ? 0L : file.length(); + } + } else { + AccessController.doPrivileged((PrivilegedAction) () -> { + try { + super.setFile(file); + if (outputStream != null) { + outputStream.currentSize = file == null ? 0L : file.length(); + } + } catch (FileNotFoundException e) { + throw new UncheckedIOException(e); + } + return null; + }, acc); + } } } diff --git a/ext/src/main/java/org/jboss/logmanager/ext/handlers/SuffixRotator.java b/ext/src/main/java/org/jboss/logmanager/ext/handlers/SuffixRotator.java index 9f77ce86..ba8e7e54 100644 --- a/ext/src/main/java/org/jboss/logmanager/ext/handlers/SuffixRotator.java +++ b/ext/src/main/java/org/jboss/logmanager/ext/handlers/SuffixRotator.java @@ -21,11 +21,16 @@ import java.io.IOException; import java.io.InputStream; +import java.io.OutputStream; +import java.io.UncheckedIOException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.StandardCopyOption; +import java.security.AccessControlContext; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.text.SimpleDateFormat; import java.util.Date; import java.util.Locale; @@ -53,15 +58,18 @@ public enum CompressionType { /** * An empty rotation suffix. */ - static final SuffixRotator EMPTY = new SuffixRotator("", "", "", CompressionType.NONE); + static final SuffixRotator EMPTY = new SuffixRotator(AccessController.getContext(), "", "", "", CompressionType.NONE); + private final AccessControlContext acc; private final String originalSuffix; private final String datePattern; private final SimpleDateFormat formatter; private final String compressionSuffix; private final CompressionType compressionType; - private SuffixRotator(final String originalSuffix, final String datePattern, final String compressionSuffix, final CompressionType compressionType) { + private SuffixRotator(final AccessControlContext acc, final String originalSuffix, final String datePattern, + final String compressionSuffix, final CompressionType compressionType) { + this.acc = acc; this.originalSuffix = originalSuffix; this.datePattern = datePattern; this.compressionSuffix = compressionSuffix; @@ -80,7 +88,7 @@ private SuffixRotator(final String originalSuffix, final String datePattern, fin * * @return the file rotator used to determine the suffix parts and rotate the file. */ - static SuffixRotator parse(final String suffix) { + static SuffixRotator parse(final AccessControlContext acc, final String suffix) { if (suffix == null || suffix.isEmpty()) { return EMPTY; } @@ -103,9 +111,9 @@ static SuffixRotator parse(final String suffix) { } } if (compressionSuffix.isEmpty() && datePattern.isEmpty()) { - return new SuffixRotator(suffix, suffix, "", CompressionType.NONE); + return new SuffixRotator(acc, suffix, suffix, "", CompressionType.NONE); } - return new SuffixRotator(suffix, datePattern, compressionSuffix, compressionType); + return new SuffixRotator(acc, suffix, datePattern, compressionSuffix, compressionType); } /** @@ -156,7 +164,7 @@ void rotate(final ErrorManager errorManager, final Path source, final String suf try { archiveGzip(source, target); // Delete the file after it's archived to behave like a file move or rename - Files.delete(source); + deleteFile(source); } catch (Exception e) { errorManager.error(String.format("Failed to compress %s to %s. Compressed file may be left on the " + "filesystem corrupted.", source, target), e, ErrorManager.WRITE_FAILURE); @@ -165,7 +173,7 @@ void rotate(final ErrorManager errorManager, final Path source, final String suf try { archiveZip(source, target); // Delete the file after it's archived to behave like a file move or rename - Files.delete(source); + deleteFile(source); } catch (Exception e) { errorManager.error(String.format("Failed to compress %s to %s. Compressed file may be left on the " + "filesystem corrupted.", source, target), e, ErrorManager.WRITE_FAILURE); @@ -219,13 +227,13 @@ void rotate(final ErrorManager errorManager, final Path source, final String suf final String fileWithSuffix = source.toAbsolutePath() + rotationSuffix; final Path lastFile = Paths.get(fileWithSuffix + "." + maxBackupIndex + compressionSuffix); try { - Files.deleteIfExists(lastFile); + deleteFile(lastFile); } catch (Exception e) { errorManager.error(String.format("Failed to delete file %s", lastFile), e, ErrorManager.GENERIC_FAILURE); } for (int i = maxBackupIndex - 1; i >= 1; i--) { final Path src = Paths.get(fileWithSuffix + "." + i + compressionSuffix); - if (Files.exists(src)) { + if (fileExists(src)) { final Path target = Paths.get(fileWithSuffix + "." + (i + 1) + compressionSuffix); move(errorManager, src, target); } @@ -242,19 +250,23 @@ public String toString() { } private void move(final ErrorManager errorManager, final Path src, final Path target) { - try { - Files.move(src, target, StandardCopyOption.REPLACE_EXISTING); - } catch (Exception e) { - // Report the error, but allow the rotation to continue - errorManager.error(String.format("Failed to move file %s to %s.", src, target), e, ErrorManager.GENERIC_FAILURE); + if (System.getSecurityManager() == null) { + try { + Files.move(src, target, StandardCopyOption.REPLACE_EXISTING); + } catch (Exception e) { + // Report the error, but allow the rotation to continue + errorManager.error(String.format("Failed to move file %s to %s.", src, target), e, ErrorManager.GENERIC_FAILURE); + } + } else { + AccessController.doPrivileged(new MoveFileAction(errorManager, src, target), acc); } } - private static void archiveGzip(final Path source, final Path target) throws IOException { + private void archiveGzip(final Path source, final Path target) throws IOException { final byte[] buff = new byte[512]; - try (final GZIPOutputStream out = new GZIPOutputStream(Files.newOutputStream(target), true)) { - try (final InputStream in = Files.newInputStream(source)) { + try (final GZIPOutputStream out = new GZIPOutputStream(newOutputStream(target), true)) { + try (final InputStream in = newInputStream(source)) { int len; while ((len = in.read(buff)) != -1) { out.write(buff, 0, len); @@ -264,12 +276,12 @@ private static void archiveGzip(final Path source, final Path target) throws IOE } } - private static void archiveZip(final Path source, final Path target) throws IOException { + private void archiveZip(final Path source, final Path target) throws IOException { final byte[] buff = new byte[512]; - try (final ZipOutputStream out = new ZipOutputStream(Files.newOutputStream(target), StandardCharsets.UTF_8)) { + try (final ZipOutputStream out = new ZipOutputStream(newOutputStream(target), StandardCharsets.UTF_8)) { final ZipEntry entry = new ZipEntry(source.getFileName().toString()); out.putNextEntry(entry); - try (final InputStream in = Files.newInputStream(source)) { + try (final InputStream in = newInputStream(source)) { int len; while ((len = in.read(buff)) != -1) { out.write(buff, 0, len); @@ -278,4 +290,120 @@ private static void archiveZip(final Path source, final Path target) throws IOEx out.closeEntry(); } } + + @SuppressWarnings("UnusedReturnValue") + private boolean deleteFile(final Path path) throws IOException { + if (System.getSecurityManager() == null) { + return Files.deleteIfExists(path); + } + return AccessController.doPrivileged(new DeleteFileAction(path), acc); + } + + private boolean fileExists(final Path file) { + if (System.getSecurityManager() == null) { + return Files.exists(file); + } + return AccessController.doPrivileged(new FileExistsAction(file), acc); + } + + private InputStream newInputStream(final Path file) throws IOException { + if (System.getSecurityManager() == null) { + return Files.newInputStream(file); + } + return AccessController.doPrivileged(new InputStreamAction(file), acc); + } + + private OutputStream newOutputStream(final Path file) throws IOException { + if (System.getSecurityManager() == null) { + return Files.newOutputStream(file); + } + return AccessController.doPrivileged(new OutputStreamAction(file), acc); + } + + private static class DeleteFileAction implements PrivilegedAction { + private final Path file; + + private DeleteFileAction(final Path file) { + this.file = file; + } + + @Override + public Boolean run() { + try { + return Files.deleteIfExists(file); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + } + + private static class MoveFileAction implements PrivilegedAction { + private final ErrorManager errorManager; + private final Path source; + private final Path target; + + private MoveFileAction(final ErrorManager errorManager, final Path source, final Path target) { + this.errorManager = errorManager; + this.source = source; + this.target = target; + } + + @Override + public Path run() { + try { + return Files.move(source, target, StandardCopyOption.REPLACE_EXISTING); + } catch (Exception e) { + // Report the error, but allow the rotation to continue + errorManager.error(String.format("Failed to move file %s to %s.", source, target), e, ErrorManager.GENERIC_FAILURE); + } + return null; + } + } + + private static class FileExistsAction implements PrivilegedAction { + private final Path file; + + private FileExistsAction(final Path file) { + this.file = file; + } + + @Override + public Boolean run() { + return Files.exists(file); + } + } + + private static class InputStreamAction implements PrivilegedAction { + private final Path file; + + private InputStreamAction(final Path file) { + this.file = file; + } + + @Override + public InputStream run() { + try { + return Files.newInputStream(file); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + } + + private static class OutputStreamAction implements PrivilegedAction { + private final Path file; + + private OutputStreamAction(final Path file) { + this.file = file; + } + + @Override + public OutputStream run() { + try { + return Files.newOutputStream(file); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + } }