--- /dev/null
+From: Mark Vitale <mvitale@sinenomine.net>
+Date: Fri, 13 May 2016 00:01:31 -0400
+Subject: dir: fileserver leaks names of file and directories
+
+Summary:
+Due to incomplete initialization or clearing of reused memory,
+fileserver directory objects are likely to contain "dead" directory
+entry information. These extraneous entries are not active - that is,
+they are logically invisible to the fileserver and client. However,
+they are physically visible on the fileserver vice partition, on the
+wire in FetchData replies, and on the client cache partition. This
+constitutes a leak of directory information.
+
+Characterization:
+There are three different kinds of "dead" residual directory entry
+leaks, each with a different cause:
+
+1. There may be partial name data after the null terminator in a live
+directory entry. This happens when a previously used directory entry
+becomes free, then is reused for a directory entry with a shorter name.
+This may be addressed in a future commit.
+
+2. "Dead" directory entries are left uncleared after an object is
+deleted or renamed. This may be addressed in a future commit.
+
+3. Residual directory entries may be inadvertently picked up when a new
+directory is created or an existing directory is extended by a 2kiBi
+page. This is the most severe problem and is addressed by this commit.
+
+This third kind of leak is the most severe because the leaked
+directory information may be from _any_ other directory residing on the
+fileserver, even if the current user is not authorized to see that
+directory.
+
+Root cause:
+The fileserver's directory/buffer package shares a pool of directory
+page buffers among all fileserver threads for both directory reads and
+directory writes. When the fileserver creates a new directory or
+extends an existing one, it uses any available unlocked buffer in the
+pool. This buffer is likely to contain another directory page recently
+read or written by the fileserver. Unfortunately the fileserver only
+initializes the page header fields (and the first two "dot" and "dotdot"
+entries in the case of a new directory). Any residual entries in the
+rest of the directory page are now logically "dead", but still
+physically present in the directory. They can easily be seen on the
+vice partition, on the wire in a FetchData reply, and on the cache
+partition.
+
+Note:
+The directory/buffer package used by the fileserver is also used by the
+salvager and the volserver. Therefore, salvager activity may also leak
+directory information to a certain extent. The volserver vos split
+command may also contribute to leaks. Any volserver operation that
+creates volumes (create, move, copy, restore, release) may also have
+insignificant leaks. These less significant leaks are addressed by this
+commit as well.
+
+Exploits:
+Any AFS user authorized to read directories may passively exploit this
+leak by capturing wire traffic or examining his local cache as he/she
+performs authorized reads on existing directories. Any leaked data will
+be for other directories the fileserver had in the buffer pool at the
+time the authorized directories were created or extended.
+
+Any AFS user authorized to write a new directory may actively exploit
+this leak by creating a new directory, flushing cache, then re-reading
+the newly created directory. Any leaked data will be for other
+directories the fileserver had in the buffer pool within the last few
+seconds. In this way an authorized user may sample current fileserver
+directory buffer contents for as long as he/she desires, without being
+detected.
+
+Directories already containing leaked data may themselves be leaked,
+leading to multiple layers of leaked data propagating with every new or
+extended directory.
+
+The names of files and directories are the most obvious source of
+information in this leak, but the FID vnode and uniqueid are leaked as
+well. Careful examination of the sequences of leaked vnode numbers and
+uniqueids may allow an attacker to:
+- Discern each layer of old directories by observing breaks in
+ consecutive runs of vnode and/or uniqueid numbers.
+- Infer which objects may reside on the same volume.
+- Discover the order in which objects were created (vnode) or modified
+ (uniqueid).
+- Know whether an object is a file (even vnode) or a directory (odd
+ vnode).
+
+Prevent new leaks by always clearing a pool buffer before using it to
+create or extend a directory.
+
+Existing leaks on the fileserver vice partitions may be addressed in a
+future commit.
+
+Reviewed-on: https://gerrit.openafs.org/12458
+Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
+Tested-by: Mark Vitale <mvitale@sinenomine.net>
+Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
+Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
+(cherry picked from commit 70065cb1831dbcfd698c8fee216e33511a314904)
+
+Change-Id: Ifa9d9266368ed3775898b7628ca980edcb230356
+Reviewed-on: https://gerrit.openafs.org/12463
+Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
+Tested-by: BuildBot <buildbot@rampaginggeek.com>
+Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
+(cherry picked from commit f234083a324085ebc7a2509b5878da5a3d10b956)
+---
+ src/dir/buffer.c | 3 +++
+ 1 file changed, 3 insertions(+)
+
+diff --git a/src/dir/buffer.c b/src/dir/buffer.c
+index 3379159..e4df861 100644
+--- a/src/dir/buffer.c
++++ b/src/dir/buffer.c
+@@ -11,6 +11,7 @@
+ #include <afs/param.h>
+
+
++#include <string.h>
+ #include <stdlib.h>
+ #include <lock.h>
+
+@@ -449,7 +450,9 @@ DNew(afs_int32 *fid, int page)
+ }
+ ObtainWriteLock(&tb->lock);
+ tb->lockers++;
++ memset(tb->data, 0, BUFFER_PAGE_SIZE); /* don't leak other people's dirs */
+ ReleaseWriteLock(&afs_bufferLock);
+ ReleaseWriteLock(&tb->lock);
++
+ return tb->data;
+ }
--- /dev/null
+From: Benjamin Kaduk <kaduk@mit.edu>
+Date: Sun, 6 Nov 2016 23:29:22 -0600
+Subject: afs: do not leak stale data in buffers
+
+Similar to the previous commit, zero out the buffer when fetching
+a new slot, to avoid the possibility of leaving stale data in
+a reused buffer.
+
+We are not supposed to write such stale data back to a fileserver,
+but this is an extra precaution in case of bugs elsewhere -- memset
+is not as expensive as it was in the 1980s.
+
+Reviewed-on: https://gerrit.openafs.org/12459
+Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
+Tested-by: Mark Vitale <mvitale@sinenomine.net>
+Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
+Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
+(cherry picked from commit a26c5054ee501ec65db3104f6a6a0fef634d9ea7)
+
+Change-Id: Id60559ed84581e2f6a50cd4313f64780b8a0bafd
+Reviewed-on: https://gerrit.openafs.org/12464
+Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
+Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
+Tested-by: Mark Vitale <mvitale@sinenomine.net>
+Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
+(cherry picked from commit 27515016cf92d0456b2a6e8a90758a02729f7407)
+---
+ src/afs/afs_buffer.c | 2 ++
+ 1 file changed, 2 insertions(+)
+
+diff --git a/src/afs/afs_buffer.c b/src/afs/afs_buffer.c
+index 7d856c2..83a30fc 100644
+--- a/src/afs/afs_buffer.c
++++ b/src/afs/afs_buffer.c
+@@ -368,6 +368,8 @@ afs_newslot(struct dcache *adc, afs_int32 apage, struct buffer *lp)
+ AFS_STATS(afs_stats_cmperf.bufFlushDirty++);
+ }
+
++ /* Zero out the data so we don't leak something we shouldn't. */
++ memset(lp->data, 0, AFS_BUFFER_PAGESIZE);
+ /* Now fill in the header. */
+ lp->fid = adc->index;
+ afs_copy_inode(&lp->inode, &adc->f.inode);
--- /dev/null
+From: Mark Vitale <mvitale@sinenomine.net>
+Date: Mon, 7 Nov 2016 14:16:50 -0500
+Subject: dir: do not leak contents of deleted directory entries
+
+Deleting an AFS directory entry (afs_dir_Delete) merely removes the
+entry logically by updating the allocation map and hash table. However,
+the entry itself remains on disk - that is, both the cache manager's
+cache partition and the fileserver's vice partitions.
+
+This constitutes a leak of directory entry information, including the
+object's name and MKfid (vnode and uniqueid). This leaked information
+is also visible on the wire during FetchData requests and volume
+operations.
+
+Modify afs_dir_Delete to clear the contents of deleted directory
+entries.
+
+Patchset notes:
+This commit only prevents leaks for newly deleted entries. Another
+commit in this patchset prevents leaks of partial object names upon
+reuse of pre-existing deleted entries. A third commit in this
+patchset prevents yet another kind of directory entry leak, when
+internal buffers are reused to create or enlarge existing directories.
+All three patches are required to prevent new leaks. Two additional
+salvager patches are also included to assist administrators in the
+cleanup of pre-existing leaks.
+
+[kaduk@mit.edu: style nit for sizeof() argument]
+
+Reviewed-on: https://gerrit.openafs.org/12460
+Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
+Tested-by: Mark Vitale <mvitale@sinenomine.net>
+Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
+Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
+(cherry picked from commit f591f6fae3d8b8d44140ca64e53bad840aeeeba0)
+
+Change-Id: I41f76649f4bed609793b944db32c5ae62aa07458
+Reviewed-on: https://gerrit.openafs.org/12465
+Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
+Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
+Tested-by: BuildBot <buildbot@rampaginggeek.com>
+Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
+(cherry picked from commit 2161e741da94b3ceb60cbd57a78643dd4169dfb7)
+---
+ src/dir/dir.c | 4 +++-
+ 1 file changed, 3 insertions(+), 1 deletion(-)
+
+diff --git a/src/dir/dir.c b/src/dir/dir.c
+index 51a78eb..1b8ec5c 100644
+--- a/src/dir/dir.c
++++ b/src/dir/dir.c
+@@ -206,7 +206,9 @@ Delete(void *dir, char *entry)
+ DRelease(previtem, 1);
+ index = DVOffset(firstitem) / 32;
+ nitems = NameBlobs(firstitem->name);
+- DRelease(firstitem, 0);
++ /* Clear entire DirEntry and any DirXEntry extensions */
++ memset(firstitem, 0, nitems * sizeof(*firstitem));
++ DRelease(firstitem, 1);
+ FreeBlobs(dir, index, nitems);
+ return 0;
+ }
projects / packages / o / openafs.git / commitdiff