[icedtea-web] RFC: reduce permissions on created files
Dr Andrew John Hughes
ahughes at redhat.com
Fri Nov 12 03:54:18 PST 2010
On 20:14 Thu 11 Nov , Omair Majid wrote:
> Hi,
>
> The attached patch tries to make files created by netx/plugin more
> secure by removing unnecessary permissions. IcedTea6 used to carry a
> patch to change the umask used by the javaws process which IcedTea-Web
> does not. This patch tries to make netx/plugin behave like they would in
> the presence of such a patch.
>
> This patch does not change the file permissions on files that are cached
> (mostly because I dont see why they should be protected), but does
> change permissions on KeyStores, native directories created under /tmp/,
> lock files, files created through the JNLP api and log files.
>
I think this is a better approach, though we should be aware that the permissions
are now changed after the file has been created, whereas before (I presume) they
were created with the correct permissions to begin with.
> ChangeLog:
> 2010-11-11 Omair Majid <omajid at redhat.com>
>
> * netx/net/sourceforge/jnlp/util/FileUtils.java
> (restrictFile): New method. Removes extra permissions on a file.
> * netx/net/sourceforge/jnlp/Launcher.java
> (markNetxRunning): Do not grant unnecessary file permissions.
> * netx/net/sourceforge/jnlp/security/viewer/CertificatePane.java
> (ImportButtonListener.actionPerformed): Likewise.
> (RemoveButtonListener.actionPerformed): Likewise.
> * netx/net/sourceforge/jnlp/services/SingleInstanceLock.java
> (createWithPort): Likewise.
> (getLockFile): Likewise.
> * netx/net/sourceforge/jnlp/services/XExtendedService.java
> (openFile): Likewise.
> * netx/net/sourceforge/jnlp/services/XFileSaveService.java
> (writeToFile): Likewise.
> * netx/net/sourceforge/jnlp/services/XPersistenceService.java
> (create): Likewise.
> * netx/net/sourceforge/jnlp/util/XDesktopEntry.java
> (installDesktopLauncher): Likewise.
> * plugin/icedteanp/java/sun/applet/PluginMain.java
> (PluginMain): Likewise.
>
> Any thoughts or comments?
>
> Cheers,
> Omair
> diff -r e82455c47f08 netx/net/sourceforge/jnlp/Launcher.java
> --- a/netx/net/sourceforge/jnlp/Launcher.java Thu Nov 11 11:43:13 2010 -0500
> +++ b/netx/net/sourceforge/jnlp/Launcher.java Thu Nov 11 19:56:02 2010 -0500
> @@ -48,6 +48,7 @@
> import net.sourceforge.jnlp.runtime.JNLPRuntime;
> import net.sourceforge.jnlp.services.InstanceExistsException;
> import net.sourceforge.jnlp.services.ServiceUtil;
> +import net.sourceforge.jnlp.util.FileUtils;
> import net.sourceforge.jnlp.util.Reflect;
>
> import javax.swing.SwingUtilities;
> @@ -729,6 +730,7 @@
> .getProperty(DeploymentConfiguration.KEY_USER_NETX_RUNNING_FILE));
> netxRunningFile.getParentFile().mkdirs();
> if (netxRunningFile.createNewFile()) {
> + FileUtils.restrictFile(netxRunningFile, true);
> FileOutputStream fos = new FileOutputStream(netxRunningFile);
> try {
> fos.write(message.getBytes());
> diff -r e82455c47f08 netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
> --- a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java Thu Nov 11 11:43:13 2010 -0500
> +++ b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java Thu Nov 11 19:56:02 2010 -0500
> @@ -815,7 +815,10 @@
> nativeDir = getNativeDir();
>
> File outFile = new File(nativeDir, name);
> -
> + if (!outFile.isFile() && !outFile.createNewFile()) {
> + throw new IOException("unable to create " + outFile);
> + }
> + FileUtils.restrictFile(outFile, true);
> CacheUtil.streamCopy(jarFile.getInputStream(e),
> new FileOutputStream(outFile));
>
> @@ -842,6 +845,7 @@
> else {
> // add this new native directory to the search path
> addNativeDirectory(nativeDir);
> + FileUtils.restrictFile(nativeDir, true);
> return nativeDir;
> }
> }
> diff -r e82455c47f08 netx/net/sourceforge/jnlp/security/CertWarningPane.java
> --- a/netx/net/sourceforge/jnlp/security/CertWarningPane.java Thu Nov 11 11:43:13 2010 -0500
> +++ b/netx/net/sourceforge/jnlp/security/CertWarningPane.java Thu Nov 11 19:56:02 2010 -0500
> @@ -47,6 +47,7 @@
> import java.awt.GridLayout;
> import java.awt.event.ActionEvent;
> import java.awt.event.ActionListener;
> +import java.io.File;
> import java.io.FileOutputStream;
> import java.io.OutputStream;
> import java.security.KeyStore;
> @@ -68,6 +69,7 @@
> import net.sourceforge.jnlp.security.KeyStores.Level;
> import net.sourceforge.jnlp.security.KeyStores.Type;
> import net.sourceforge.jnlp.security.SecurityWarning.AccessType;
> +import net.sourceforge.jnlp.util.FileUtils;
>
> /**
> * Provides the panel for using inside a SecurityWarningDialog. These dialogs are
> @@ -246,7 +248,13 @@
> KeyStore ks = KeyStores.getKeyStore(Level.USER, Type.CERTS);
> X509Certificate c = (X509Certificate) parent.getJarSigner().getPublisher();
> CertificateUtils.addToKeyStore(c, ks);
> - OutputStream os = new FileOutputStream(KeyStores.getKeyStoreLocation(Level.USER, Type.CERTS));
> + File keyStoreFile = new File(KeyStores.getKeyStoreLocation(Level.USER, Type.CERTS));
> + if (!keyStoreFile.isFile()) {
> + keyStoreFile.createNewFile();
> + }
> + FileUtils.restrictFile(keyStoreFile, true);
> +
> + OutputStream os = new FileOutputStream(keyStoreFile);
> ks.store(os, KeyStores.getPassword());
> if (JNLPRuntime.isDebug()) {
> System.out.println("certificate is now permanently trusted");
> diff -r e82455c47f08 netx/net/sourceforge/jnlp/security/KeyStores.java
> --- a/netx/net/sourceforge/jnlp/security/KeyStores.java Thu Nov 11 11:43:13 2010 -0500
> +++ b/netx/net/sourceforge/jnlp/security/KeyStores.java Thu Nov 11 19:56:02 2010 -0500
> @@ -53,6 +53,7 @@
> import net.sourceforge.jnlp.runtime.DeploymentConfiguration;
> import net.sourceforge.jnlp.runtime.JNLPRuntime;
> import net.sourceforge.jnlp.runtime.Translator;
> +import net.sourceforge.jnlp.util.FileUtils;
>
> /**
> * The <code>KeyStores</code> class allows easily accessing the various KeyStores
> @@ -344,6 +345,8 @@
> FileOutputStream fos = new FileOutputStream(file);
> ks.store(fos, password.toCharArray());
> fos.close();
> +
> + FileUtils.restrictFile(file, true);
> }
>
> // TODO catch exception when password is incorrect and prompt user
> diff -r e82455c47f08 netx/net/sourceforge/jnlp/security/viewer/CertificatePane.java
> --- a/netx/net/sourceforge/jnlp/security/viewer/CertificatePane.java Thu Nov 11 11:43:13 2010 -0500
> +++ b/netx/net/sourceforge/jnlp/security/viewer/CertificatePane.java Thu Nov 11 19:56:02 2010 -0500
> @@ -45,6 +45,7 @@
> import java.awt.event.ActionEvent;
> import java.awt.event.ActionListener;
> import java.awt.event.KeyEvent;
> +import java.io.File;
> import java.io.FileOutputStream;
> import java.io.OutputStream;
> import java.io.PrintStream;
> @@ -76,6 +77,7 @@
> import net.sourceforge.jnlp.security.SecurityUtil;
> import net.sourceforge.jnlp.security.SecurityWarningDialog;
> import net.sourceforge.jnlp.security.KeyStores.Level;
> +import net.sourceforge.jnlp.util.FileUtils;
>
> public class CertificatePane extends JPanel {
>
> @@ -361,8 +363,13 @@
> try {
> KeyStore ks = keyStore;
> CertificateUtils.addToKeyStore(chooser.getSelectedFile(), ks);
> - OutputStream os = new FileOutputStream(
> - KeyStores.getKeyStoreLocation(currentKeyStoreLevel, currentKeyStoreType));
> + File keyStoreFile = new File(KeyStores
> + .getKeyStoreLocation(currentKeyStoreLevel, currentKeyStoreType));
> + if (!keyStoreFile.isFile()) {
> + keyStoreFile.createNewFile();
> + }
> + FileUtils.restrictFile(keyStoreFile, true);
> + OutputStream os = new FileOutputStream(keyStoreFile);
> ks.store(os, KeyStores.getPassword());
> repopulateTables();
> } catch (Exception ex) {
> @@ -436,8 +443,14 @@
> JOptionPane.YES_NO_OPTION);
> if (i == 0) {
> keyStore.deleteEntry(alias);
> - FileOutputStream fos = new FileOutputStream(
> - KeyStores.getKeyStoreLocation(currentKeyStoreLevel, currentKeyStoreType));
> + File keyStoreFile = new File(KeyStores
> + .getKeyStoreLocation(currentKeyStoreLevel, currentKeyStoreType));
> + if (!keyStoreFile.isFile()) {
> + keyStoreFile.createNewFile();
> + }
> + FileUtils.restrictFile(keyStoreFile, true);
> +
> + FileOutputStream fos = new FileOutputStream(keyStoreFile);
> keyStore.store(fos, KeyStores.getPassword());
> fos.close();
> }
> diff -r e82455c47f08 netx/net/sourceforge/jnlp/services/SingleInstanceLock.java
> --- a/netx/net/sourceforge/jnlp/services/SingleInstanceLock.java Thu Nov 11 11:43:13 2010 -0500
> +++ b/netx/net/sourceforge/jnlp/services/SingleInstanceLock.java Thu Nov 11 19:56:02 2010 -0500
> @@ -67,6 +67,8 @@
> */
> public void createWithPort(int localPort) throws IOException {
>
> + lockFile.createNewFile();
> + FileUtils.restrictFile(lockFile, true);
> BufferedWriter lockFileWriter = new BufferedWriter(new FileWriter(lockFile, false));
> lockFileWriter.write(String.valueOf(localPort));
> lockFileWriter.newLine();
> @@ -135,6 +137,9 @@
> if (!baseDir.isDirectory() && !baseDir.mkdirs()) {
> throw new RuntimeException(R("RNoLockDir", baseDir));
> }
> +
> + FileUtils.restrictFile(baseDir, true);
> +
> String lockFileName = getLockFileName();
> File applicationLockFile = new File(baseDir, lockFileName);
> return applicationLockFile;
> diff -r e82455c47f08 netx/net/sourceforge/jnlp/services/XExtendedService.java
> --- a/netx/net/sourceforge/jnlp/services/XExtendedService.java Thu Nov 11 11:43:13 2010 -0500
> +++ b/netx/net/sourceforge/jnlp/services/XExtendedService.java Thu Nov 11 19:56:02 2010 -0500
> @@ -23,6 +23,7 @@
> import javax.jnlp.FileContents;
>
> import net.sourceforge.jnlp.security.SecurityWarning.AccessType;
> +import net.sourceforge.jnlp.util.FileUtils;
>
> /**
> * Implementation of ExtendedService
> @@ -36,6 +37,7 @@
>
> /* FIXME: this opens a file with read/write mode, not just read or write */
> if (ServiceUtil.checkAccess(AccessType.READ_FILE, new Object[]{ file.getAbsolutePath() })) {
> + FileUtils.restrictFile(file, true);
> return (FileContents) ServiceUtil.createPrivilegedProxy(FileContents.class,
> new XFileContents(file));
> } else {
> diff -r e82455c47f08 netx/net/sourceforge/jnlp/services/XFileSaveService.java
> --- a/netx/net/sourceforge/jnlp/services/XFileSaveService.java Thu Nov 11 11:43:13 2010 -0500
> +++ b/netx/net/sourceforge/jnlp/services/XFileSaveService.java Thu Nov 11 19:56:02 2010 -0500
> @@ -44,6 +44,7 @@
> import javax.jnlp.*;
>
> import net.sourceforge.jnlp.security.SecurityWarning.AccessType;
> +import net.sourceforge.jnlp.util.FileUtils;
>
> import javax.swing.JFileChooser;
> import javax.swing.JOptionPane;
> @@ -123,6 +124,7 @@
> } else {
> file.createNewFile();
> }
> + FileUtils.restrictFile(file, true);
>
> if (file.canWrite()) {
> FileOutputStream out = new FileOutputStream(file);
> diff -r e82455c47f08 netx/net/sourceforge/jnlp/services/XPersistenceService.java
> --- a/netx/net/sourceforge/jnlp/services/XPersistenceService.java Thu Nov 11 11:43:13 2010 -0500
> +++ b/netx/net/sourceforge/jnlp/services/XPersistenceService.java Thu Nov 11 19:56:02 2010 -0500
> @@ -26,6 +26,7 @@
> import net.sourceforge.jnlp.*;
> import net.sourceforge.jnlp.cache.*;
> import net.sourceforge.jnlp.runtime.*;
> +import net.sourceforge.jnlp.util.FileUtils;
>
> /**
> * The BasicService JNLP service.
> @@ -99,6 +100,8 @@
> if (!file.createNewFile())
> throw new IOException("File already exists.");
>
> + FileUtils.restrictFile(file, true);
> +
> return maxsize;
> }
>
> diff -r e82455c47f08 netx/net/sourceforge/jnlp/util/FileUtils.java
> --- a/netx/net/sourceforge/jnlp/util/FileUtils.java Thu Nov 11 11:43:13 2010 -0500
> +++ b/netx/net/sourceforge/jnlp/util/FileUtils.java Thu Nov 11 19:56:02 2010 -0500
> @@ -72,6 +72,30 @@
> }
>
> /**
> + * Sets the file permissions on a file such that it is not readable or
> + * writable by anyone other than the owner. If writeableByOnwer is false,
> + * even the owner can not write to it. The file must exist before this
> + * method is called.
> + */
> + public static void restrictFile(File file, boolean writableByOwner) {
> +
> + file.setExecutable(false, false);
> + file.setReadable(false, false);
> + file.setWritable(false, false);
> +
> + file.setReadable(true, true);
> +
> + if (writableByOwner) {
> + file.setWritable(true, true);
> + }
> +
> + if (file.isDirectory()) {
> + file.setExecutable(true, true);
> + }
> +
> + }
> +
> + /**
> * Returns a String that is suitable for using in GUI elements for
> * displaying (long) paths to users.
> *
> diff -r e82455c47f08 netx/net/sourceforge/jnlp/util/XDesktopEntry.java
> --- a/netx/net/sourceforge/jnlp/util/XDesktopEntry.java Thu Nov 11 11:43:13 2010 -0500
> +++ b/netx/net/sourceforge/jnlp/util/XDesktopEntry.java Thu Nov 11 19:56:02 2010 -0500
> @@ -142,6 +142,10 @@
> if (!shortcutFile.getParentFile().isDirectory() && !shortcutFile.getParentFile().mkdirs()) {
> throw new IOException(shortcutFile.getParentFile().toString());
> }
> +
> + shortcutFile.createNewFile();
> + FileUtils.restrictFile(shortcutFile, true);
> +
> /*
> * Write out a Java String (UTF-16) as a UTF-8 file
> */
> diff -r e82455c47f08 plugin/icedteanp/java/sun/applet/PluginMain.java
> --- a/plugin/icedteanp/java/sun/applet/PluginMain.java Thu Nov 11 11:43:13 2010 -0500
> +++ b/plugin/icedteanp/java/sun/applet/PluginMain.java Thu Nov 11 19:56:02 2010 -0500
> @@ -77,6 +77,7 @@
>
> import net.sourceforge.jnlp.runtime.DeploymentConfiguration;
> import net.sourceforge.jnlp.runtime.JNLPRuntime;
> +import net.sourceforge.jnlp.util.FileUtils;
>
> /**
> * The main entry point into PluginAppletViewer.
> @@ -130,8 +131,12 @@
> try {
> File errFile = new File(logDir, PLUGIN_STDERR_FILE);
> errFile.getParentFile().mkdirs();
> + errFile.createNewFile();
> + FileUtils.restrictFile(errFile, true);
> File outFile = new File(logDir, PLUGIN_STDOUT_FILE);
> outFile.getParentFile().mkdirs();
> + outFile.createNewFile();
> + FileUtils.restrictFile(outFile, true);
>
> System.setErr(new TeeOutputStream(new FileOutputStream(errFile), System.err));
> System.setOut(new TeeOutputStream(new FileOutputStream(outFile), System.out));
--
Andrew :)
Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)
Support Free Java!
Contribute to GNU Classpath and the OpenJDK
http://www.gnu.org/software/classpath
http://openjdk.java.net
PGP Key: 94EFD9D8 (http://subkeys.pgp.net)
Fingerprint = F8EF F1EA 401E 2E60 15FA 7927 142C 2591 94EF D9D8
More information about the distro-pkg-dev
mailing list