Merge pull request #2407 from pdbain-ibm/attach_access

Improve attach API access checking

jcl/src/java.base/share/classes/com/ibm/oti/util/ExternalMessages-MasterIndex.properties

@@ -1283,5 +1283,13 @@ K0800="Invalid Unicode code point - {0}"
 K0801="Last character in replacement string can't be \, character to be escaped is required."
 K0802="Last character in replacement string can't be $, group index is required."
 
+# attach API
+K0803="File {0} is owned by {1}, should be owned by current user"
+K0804="Illegal file {0} found in target directory"
+K0805="{0} has permissions {1}, should have owner access only"
+K0806="Cannot verify permissions {0}"
+K0807="Cannot delete file {0}"
+K0808="Cannot create new file {0}"
+
 #java.lang.ref.Reference
 K0900="Create a new Reference, since a Reference cannot be cloned."

jcl/src/java.base/share/classes/com/ibm/tools/attach/target/Advertisement.java

@@ -1,7 +1,7 @@
 /*[INCLUDE-IF Sidecar16]*/
 package com.ibm.tools.attach.target;
 /*******************************************************************************
- * Copyright (c) 2009, 2010 IBM Corp. and others
+ * Copyright (c) 2009, 2018 IBM Corp. and others
  *
  * This program and the accompanying materials are made available under
  * the terms of the Eclipse Public License 2.0 which accompanies this
@@ -189,7 +189,7 @@ private static void encodeString(StringBuilder buffer, String originalString) {
 	 * This is called during initialization or after successful initialization only.
 	 * @param vmId ID of this VM
 	 * @param displayName display name of this VM
-	 * @throws IOException if cannot open the advertisement file
+	 * @throws IOException if cannot open the advertisement file or cannot delete an existing one
 	 */
 	static void createAdvertisementFile(String vmId, String displayName) throws IOException {
 
@@ -198,22 +198,20 @@ static void createAdvertisementFile(String vmId, String displayName) throws IOEx
 			return;
 		}
 		File advertFile = TargetDirectory.getAdvertisementFileObject();
-		IPC.createFileWithPermissions(advertFile.getAbsolutePath(), TargetDirectory.ADVERTISEMENT_FILE_PERMISSIONS);
 		/* AttachHandler.terminate() will delete this file on shutdown */ 
-		FileOutputStream advertOutputStream = new FileOutputStream(advertFile);
-		try {
+		IPC.createNewFileWithPermissions(advertFile, TargetDirectory.ADVERTISEMENT_FILE_PERMISSIONS);
+		/* we have a brand new, empty file with correct ownership and permissions */
+		try (FileOutputStream advertOutputStream = new FileOutputStream(advertFile);){
 			StringBuilder advertContent = createAdvertContent(vmId, displayName);
 			if (null == advertContent) {
-				IPC.logMessage("createAdvertisementFile failed to create advertisement file : file objects null"); //$NON-NLS-1$
+				IPC.logMessage("createAdvertisementFile failed to create advertisement file : file object is null"); //$NON-NLS-1$
 				return;
 			}
 
 			advertOutputStream.write(advertContent.toString().getBytes("ISO8859_1")); //$NON-NLS-1$
 			if (IPC.loggingEnabled ) {
 				IPC.logMessage("createAdvertisementFile ", advertFile.getAbsolutePath()); //$NON-NLS-1$
 			}
-		} finally {
-			advertOutputStream.close(); /* cleanup from findbugs */
 		}
 	}
 

jcl/src/java.base/share/classes/com/ibm/tools/attach/target/AttachHandler.java

@@ -144,8 +144,12 @@ static void initializeAttachAPI() {
 		/*[PR Jazz 59196 LIR: Disable attach API by default on z/OS (31972)]*/
 		boolean enableAttach = true;
 		String osName = com.ibm.oti.vm.VM.getVMLangAccess().internalGetProperties().getProperty("os.name"); //$NON-NLS-1$
-		if ((null != osName) && (osName.equalsIgnoreCase("z/OS"))) { //$NON-NLS-1$
-			enableAttach = false;
+		if (null != osName) {
+			if (osName.equalsIgnoreCase("z/OS")) { //$NON-NLS-1$
+				enableAttach = false;
+			} else if (osName.startsWith("Windows")) { //$NON-NLS-1$
+				IPC.isWindows = true;
+			}
 		}
 		/* the system property overrides the default */
 		String enableAttachProp = com.ibm.oti.vm.VM.getVMLangAccess().internalGetProperties().getProperty("com.ibm.tools.attach.enable");  //$NON-NLS-1$
@@ -335,7 +339,9 @@ private boolean initialize() throws IOException {
 	 * @throws IOException if there is a problem reading the reply file.
 	 */
 	public Attachment connectToAttacher() throws IOException {
-		Reply attacherReply = Reply.readReply(TargetDirectory.getTargetDirectoryPath(AttachHandler.getVmId()));
+		String targetDirectoryPath = TargetDirectory.getTargetDirectoryPath(AttachHandler.getVmId());
+		IPC.checkOwnerAccessOnly(targetDirectoryPath);
+		Reply attacherReply = Reply.readReply(targetDirectoryPath);
 		Attachment at = null;
 		if (null != attacherReply) {
 			int portNumber = attacherReply.getPortNumber();

jcl/src/java.base/share/classes/com/ibm/tools/attach/target/CommonDirectory.java

@@ -328,10 +328,10 @@ static private boolean isCommonControlFile(String dirMemberName) {
 				|| MASTER_NOTIFIER.equalsIgnoreCase(dirMemberName));
 	}
 
-	/*
-	 * look for leftover files and directories from previous VMs
+	/**
+	 * Look for and delete leftover files and directories from previous VMs
+	 * @param myId VMID of the current.  Set to non-null to prevent deleting own directory.
 	 */
-	/*[PR Jazz 37778 deleteStaleDirectories was always called with checkProcess=true */
 	static void deleteStaleDirectories(String myId) {
 		long myUid = IPC.getUid();
 		File[] vmDirs = getCommonDirFileObject().listFiles(new DirectorySampler());

jcl/src/java.base/share/classes/com/ibm/tools/attach/target/IPC.java

@@ -30,10 +30,22 @@
 import java.io.OutputStream;
 import java.io.PrintStream;
 import java.security.SecureRandom;
+import java.io.File;
+import java.nio.file.Files;
+import java.nio.file.LinkOption;
+import java.nio.file.Paths;
+import java.nio.file.attribute.PosixFilePermission;
+import java.util.EnumSet;
+import java.util.Set;
 import java.util.Objects;
 import java.util.Properties;
 import java.util.Random;
 
+import static java.nio.file.attribute.PosixFilePermission.GROUP_READ;
+import static java.nio.file.attribute.PosixFilePermission.GROUP_WRITE;
+import static java.nio.file.attribute.PosixFilePermission.OTHERS_READ;
+import static java.nio.file.attribute.PosixFilePermission.OTHERS_WRITE;
+
 /**
  * Utility class for operating system calls
  */
@@ -49,7 +61,13 @@ public class IPC {
 	static final int TRACEPOINT_STATUS_OOM_DURING_WAIT = -2;
 	static final int TRACEPOINT_STATUS_OOM_DURING_TERMINATE = -3;
 	static final String LOCAL_CONNECTOR_ADDRESS = "com.sun.management.jmxremote.localConnectorAddress"; //$NON-NLS-1$
+	private static final EnumSet<PosixFilePermission> NON_OWNER_READ_WRITE =
+				EnumSet.of(GROUP_READ, GROUP_WRITE, OTHERS_READ, OTHERS_WRITE);
 
+	/**
+	 * True if operating system is Windows.
+	 */
+	public static boolean isWindows = false;
 
 	private static Random randomGen; /* Cleanup. this is used by multiple threads */
 	static  PrintStream logStream; /* cleanup.  Used by multiple threads */
@@ -90,6 +108,43 @@ static int mkdirWithPermissions(String absolutePath, int perms)
 
 	static native int mkdirWithPermissionsImpl(String absolutePath, int perms);
 
+	/**
+	 * Ensure that a file or directory is not readable or writable by others
+	 * and is owned by the current user.
+	 * @param filePath File or directory path
+	 * @throws IOException if the access or ownership is wrong
+	 */
+	public static void checkOwnerAccessOnly(String filePath) throws IOException {
+		final long myUid = getUid();
+		/* Ensure file is owned by current user, or current user is root */
+		final long fileOwner = CommonDirectory.getFileOwner(filePath);
+		if ((0 != myUid) && (fileOwner != myUid)) {
+			logMessage("Wrong permissions or ownership for ", filePath); //$NON-NLS-1$
+			/*[MSG "K0803", "File {0} is owned by {1}, should be owned by current user"]*/
+			throw new IOException(com.ibm.oti.util.Msg.getString("K0803", filePath, Long.valueOf(fileOwner)));//$NON-NLS-1$
+		}
+		if (!isWindows) {
+			try {
+				/* ensure that the directory is not readable or writable by others */
+				Set<PosixFilePermission> actualPermissions =
+						Files.getPosixFilePermissions(Paths.get(filePath), LinkOption.NOFOLLOW_LINKS);
+				actualPermissions.retainAll(NON_OWNER_READ_WRITE);
+				if (!actualPermissions.isEmpty()) {
+					final String permissionString = Files.getPosixFilePermissions(Paths.get(filePath), LinkOption.NOFOLLOW_LINKS).toString();
+					logMessage("Wrong permissions: " +permissionString + " for ", filePath); //$NON-NLS-1$ //$NON-NLS-2$
+					/*[MSG "K0805", "{0} has permissions {1}, should have owner access only"]*/
+					throw new IOException(com.ibm.oti.util.Msg.getString("K0805", filePath, permissionString));//$NON-NLS-1$
+				}
+			} catch (UnsupportedOperationException e) {
+				String osName = com.ibm.oti.vm.VM.getVMLangAccess().internalGetProperties().getProperty("os.name"); //$NON-NLS-1$
+				if ((null != osName) && !osName.startsWith("Windows")) { //$NON-NLS-1$
+					/*[MSG "K0806", "Cannot verify permissions {0}"]*/
+					throw new IOException(com.ibm.oti.util.Msg.getString("K0806", filePath), e); //$NON-NLS-1$
+				}
+			}
+		}
+	}
+
 	/*[PR Jazz 30075] setupSemaphore was re-doing what createDirectoryAndSemaphore (now called prepareCommonDirectory) did already */
 
 	/**
@@ -167,11 +222,29 @@ public static boolean processExists(long pid) {
 
 	private static native int processExistsImpl(long pid);
 
-	static void createFileWithPermissions(String path, int perms)
-			throws IOException {
-		int rc = createFileWithPermissionsImpl(path, perms);
+	/**
+	 * Create a new file with specified the permissions (to override umask) and close it.
+	 * If the file exists, delete it.
+	 * @param path file system path
+	 * @param mode file access permissions (posix format) for the new file
+	 * @throws IOException if the file exists and cannot be removed, or 
+	 * a new file cannot be created with the specified permission
+	 */
+	static void createNewFileWithPermissions(File theFile, int perms) throws IOException {
+		final String filePathString = theFile.getAbsolutePath();
+		if (theFile.exists()) {
+			IPC.logMessage("Found existing file ", filePathString); //$NON-NLS-1$
+			if (!theFile.delete()) {
+				IPC.logMessage("Cannot delete existing file ", filePathString); //$NON-NLS-1$
+				/*[MSG "K0807", "Cannot delete file {0}"]*/
+				throw (new IOException(com.ibm.oti.util.Msg.getString("K0807", filePathString))); //$NON-NLS-1$
+			}
+		}
+		int rc = createFileWithPermissionsImpl(theFile.getAbsolutePath(), perms);
 		if (JNI_OK != rc) {
-			throw new IOException(path);
+			IPC.logMessage("Cannot create new file ", filePathString); //$NON-NLS-1$
+			/*[MSG "K0808", "Cannot create new file {0}"]*/
+			throw (new IOException(com.ibm.oti.util.Msg.getString("K0808", filePathString))); //$NON-NLS-1$
 		}
 	}
 

jcl/src/java.base/share/classes/com/ibm/tools/attach/target/Reply.java

@@ -34,7 +34,6 @@ public final class Reply {
 	 * Manages the file created by an attaching process
 	 *
 	 */
-	private RandomAccessFile replyChannelRAF;
 	private  String key;
 	private  Integer portNumber;
 	private final File replyFile;
@@ -68,31 +67,30 @@ public Reply(Integer thePort, String theKey, String targetDirectory, long theUid
 
 	/**
 	 * Write data to the reply file.  Must be matched with an eraseReply()
-	 * @param keyText Security key to validate transaction
-	 * @param portNumberText to be written to file. Virtual machine ID, name, semaphore name etc.
-	 * @throws IOException if files are not accessible
+	 * keyText is a security key to validate transaction
+	 * portNumberText is text to be written to file. Virtual machine ID, name, semaphore name etc.
+	 * @throws IOException if file are not accessible or cannot be created with correct permissions
 	 */
 	public void writeReply() throws IOException {
-		/* replyFIle has been created by the constructor */
-		try {
-			IPC.logMessage("writing reply file port=", portNumber.intValue(), " file path=", replyFile.getAbsolutePath());  //$NON-NLS-1$//$NON-NLS-2$
-			
-			replyChannelRAF = new RandomAccessFile(replyFile, "rw"); //$NON-NLS-1$
-			replyChannelRAF.setLength(0); /* delete whatever crud was there before */
+		/* replyFile has been created by the constructor */
+		final String replyFileAbsolutePath = replyFile.getAbsolutePath();
+		IPC.logMessage("writing reply file port=", portNumber.intValue(), " file path=", replyFileAbsolutePath);  //$NON-NLS-1$//$NON-NLS-2$
+		IPC.createNewFileWithPermissions(replyFile, TargetDirectory.ADVERTISEMENT_FILE_PERMISSIONS);
+		/* we have a brand new, empty file with correct ownership and permissions */
+		try (RandomAccessFile replyChannelRAF = new RandomAccessFile(replyFile, "rw")) { //$NON-NLS-1$
 			replyChannelRAF.writeBytes(key);
 			replyChannelRAF.writeByte('\n');
 			replyChannelRAF.writeBytes(portNumber.toString());
 			replyChannelRAF.writeByte('\n');
 			replyChannelRAF.close();
 
-			IPC.chmod(replyFile.getAbsolutePath(), REPLY_PERMISSIONS);
 			long myUid = IPC.getUid();
 			if ((ROOT_UID == myUid) && (ROOT_UID != targetUid)) {
-				IPC.chownFileToTargetUid(replyFile.getAbsolutePath(), targetUid);
+				IPC.chownFileToTargetUid(replyFileAbsolutePath, targetUid);
 			}
 		} catch (FileNotFoundException e) {
 			/*[MSG "K0552", "File not found exception thrown in writeReply for file {0}"]*/
-			throw new IOException(com.ibm.oti.util.Msg.getString("K0552", replyFile.getAbsolutePath()), e); //$NON-NLS-1$
+			throw new IOException(com.ibm.oti.util.Msg.getString("K0552", replyFileAbsolutePath), e); //$NON-NLS-1$
 		}
 	}
 
@@ -102,12 +100,12 @@ public void writeReply() throws IOException {
 	 */
 	static Reply readReply(String path) throws IOException {
 		Reply rply = new Reply(path);
+		String replyFileAbsolutePath = rply.replyFile.getAbsolutePath();
 		if (rply.fileDoesNotExist()) {
 			return null;
 		}
-		BufferedReader replyStream;
-		try {
-			replyStream = new BufferedReader(new FileReader(rply.replyFile));
+		IPC.checkOwnerAccessOnly(replyFileAbsolutePath);
+		try (BufferedReader replyStream = new BufferedReader(new FileReader(rply.replyFile));) {
 			rply.key = replyStream.readLine();
 			String line = replyStream.readLine();
 			replyStream.close();
@@ -119,7 +117,7 @@ static Reply readReply(String path) throws IOException {
 			}
 		} catch (FileNotFoundException e) {
 			/*[MSG "K0548", "Cannot read reply file {0}"]*/
-			throw new IOException(com.ibm.oti.util.Msg.getString("K0548", rply.replyFile.getAbsolutePath()), e); //$NON-NLS-1$
+			throw new IOException(com.ibm.oti.util.Msg.getString("K0548", replyFileAbsolutePath), e); //$NON-NLS-1$
 		}
 		return rply;
 	}