svn commit: r893394 - /mina/sshd/trunk/sshd-core/src/main/java/org/apache/sshd/agent/AprLibrary.java

[sp...@apache.org]

Author: spearce
Date: Wed Dec 23 03:39:25 2009
New Revision: 893394

URL: http://svn.apache.org/viewvc?rev=893394&view=rev
Log:
SSHD-8: Secure the agent socket against race conditions

Binding a UNIX domain socket creates the socket file with the
current umask.  This may permit evil members of the caller's group
to connect to the socket before the listening process can mark it
as accessible only by the owner.

Instead, create a temporary directory that is accessible only to
the owner, and only after the location is locked down, create the
socket within it as a unique random name.

Modified:
    mina/sshd/trunk/sshd-core/src/main/java/org/apache/sshd/agent/AprLibrary.java

Modified: mina/sshd/trunk/sshd-core/src/main/java/org/apache/sshd/agent/AprLibrary.java
URL: http://svn.apache.org/viewvc/mina/sshd/trunk/sshd-core/src/main/java/org/apache/sshd/agent/AprLibrary.java?rev=893394&r1=893393&r2=893394&view=diff
==============================================================================
--- mina/sshd/trunk/sshd-core/src/main/java/org/apache/sshd/agent/AprLibrary.java (original)
+++ mina/sshd/trunk/sshd-core/src/main/java/org/apache/sshd/agent/AprLibrary.java Wed Dec 23 03:39:25 2009
@@ -106,9 +106,22 @@
     }
 
     static String createLocalSocketAddress() throws IOException {
+        initialize();
+
         String name;
         if (OsUtils.isUNIX()) {
-            File socket = File.createTempFile("mina", "apr");
+            // Since there is a race condition between bind and when
+            // we can mark the socket readable only by its owner, make
+            // the socket in a temporary directory that is visible only
+            // to the owner.
+            //
+            File dir = File.createTempFile("mina", "apr");
+            if (!dir.delete() || !dir.mkdir()) {
+                throw new IOException("Cannot create secure temp directory");
+            }
+            chmodOwner(dir.getAbsolutePath(), true);
+
+            File socket = File.createTempFile("mina","apr", dir);
             socket.delete();
             name = socket.getAbsolutePath();
         } else {
@@ -121,14 +134,21 @@
 
     static void secureLocalSocket(String authSocket, long handle) throws IOException {
         if (OsUtils.isUNIX()) {
-            int perms = org.apache.tomcat.jni.File.APR_FPROT_UREAD
-                      | org.apache.tomcat.jni.File.APR_FPROT_UWRITE;
-            if (org.apache.tomcat.jni.File.permsSet(authSocket, perms) != org.apache.tomcat.jni.Status.APR_SUCCESS) {
-                throw new IOException("Unable to secure local socket");
-            }
+            chmodOwner(authSocket, false);
 
         } else {
             // should be ok on windows
         }
     }
+
+    private static void chmodOwner(String authSocket, boolean execute) throws IOException {
+        int perms = org.apache.tomcat.jni.File.APR_FPROT_UREAD
+                  | org.apache.tomcat.jni.File.APR_FPROT_UWRITE;
+        if (execute) {
+            perms |= org.apache.tomcat.jni.File.APR_FPROT_UEXECUTE;
+        }
+        if (org.apache.tomcat.jni.File.permsSet(authSocket, perms) != org.apache.tomcat.jni.Status.APR_SUCCESS) {
+            throw new IOException("Unable to secure local socket");
+        }
+    }
 }