So as to get a trivial change as confirmation that an updated
gerrit is functioning correctly.
Change-Id: I04eb12cab982a3f1b5ecc92d60c455e7a0d2242c
Reviewed-on: http://gerrit.openafs.org/12156
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
There's no reason for this file to start with a blank line.
Change-Id: I175390d3c9796fc10ef8086a5b179f4fc27362b5
Reviewed-on: http://gerrit.openafs.org/12153
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
Commit 15260c7fdc did not function
as advertised, since the conditional which attempted to make
the configure option --(en|dis)able-linux-d_splice_alias-extra-iput
mandatory on linux checked a variable for the system type which
was not set at the time the check ran.
Subsequent discussion of this behavior produced a consensus that
there is not a need to make the configure option mandatory,
due to the narrow range of kernels affected by the bug in question,
so this follow-up commit just fixes the documentation and removes
the ineffective code.
Change-Id: I36d1f8801d355f33c3132fcab166ea76faab8e87
Reviewed-on: http://gerrit.openafs.org/11710
Reviewed-by: Anders Kaseorg <andersk@mit.edu>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Don't overlow the buffer that's used for the input filename by
copying in to much with sprintf. Use asprintf to dynamically
allocate a buffer instead.
Link roken for rk_asprintf where needed.
Build compile_et with libtool, to ensure that it is linked statically,
as is needed for build tools such as compile_et. (This requires
a preceding change to set a buildtool_roken make variable.)
Caught by coverity (#985907)
Change-Id: I207dd2c49bcae3f04fa41c826b08a0a615d5f422
Reviewed-on: http://gerrit.openafs.org/9545
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
When using roken in build tools, i.e., binaries which must be
executed during the build stage, the roken library must be usable
prior to the 'install' stage. In particular, if the internal
rokenafs is used, the shared library will not be installed and
the runtime linker will not be able to find it, causing execution
of the build tool to fail. To avoid this failure, librokenafs
must be linked statically into these build tools.
Unfortunately, the way we currently use libtool is not very
well aligned to libtools model of how it should be used. As a result,
it does not seem feasible to cause libtool to link librokenafs
statically without breaking other parts of the build.
Libtool peeks at the compiler command-line arguments to affect its
behavior when invoked as a linker. The flags -static, -all-static,
and -static-libtool-libs can affect whether dynamic or static linkage
is used for various libraries being linked into the executable.
Passing -all-static tells libtool to not do any dynamic linking at
all, but is silently a no-op if static linking is not possible (the
default situation on most modern Linuxen, OS X, and Solaris).
Passing -static causes libtool to not do any dynamic linking of
libtool libraries which have not been installed, and passing
-static-libtool-libs causes libtool to not do any dynamic linking
of libtool libraries at all.
In order to get libtool to actually link statically in all cases,
we should pass -all-static, not just -static. However, because
too many platforms disallow static linking by default, this is
not a viable option.
If we retain the libtool archive librokenafs.la in the linker search
path, libtool then records the library dependency of libafshcrypto on
librokenafs in its metadata and refuses to install libafshcrypto.la to
any path other than the configured prefix. This restriction of
libtool is incompatible with our use in 'make dest', and it is not
feasible to desupport 'make dest' before the 1.8 release.
The most appropriate workaround seems to be to just pass the
path to librokenafs.a on the linker command line when linking
build tools. As such, provide a new make variable buildtool_roken
which is appropriate for linking roken into build tools -- this
variable will be set to the path to librokenafs.a when the internal
roken is used, and the normal -lrokenafs when an external roken
is used.
Change-Id: I079fc6de5d0aa6403eb1071f3d58a248b1777853
Reviewed-on: http://gerrit.openafs.org/11763
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Chas Williams <3chas3@gmail.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Resolves these warnings:
ticket5.c: In function ‘tkt_MakeTicket5’:
ticket5.c:574:33: warning: pointer targets in passing argument 1 of ‘_rxkad_v5_encode_EncTicketPart’ differ in signedness [-Wpointer-sign]
code = encode_EncTicketPart(encodebuf, allocsiz, &data, &encodelen);
^
In file included from ticket5.c:80:0:
v5gen-rewrite.h:43:30: note: expected ‘unsigned char *’ but argument is of type ‘char *’
#define encode_EncTicketPart _rxkad_v5_encode_EncTicketPart
^
v5gen.c:1889:1: note: in expansion of macro ‘encode_EncTicketPart’
encode_EncTicketPart(unsigned char *p, size_t len, const EncTicketPart * data, size_t * size)
^
ticket5.c:602:33: warning: pointer targets in passing argument 1 of ‘_rxkad_v5_encode_EncryptedData’ differ in signedness [-Wpointer-sign]
code = encode_EncryptedData(ticket + *ticketLen - 1, *ticketLen, &encdata, &tl);
^
In file included from ticket5.c:80:0:
v5gen-rewrite.h:16:30: note: expected ‘unsigned char *’ but argument is of type ‘char *’
#define encode_EncryptedData _rxkad_v5_encode_EncryptedData
^
v5gen.c:690:1: note: in expansion of macro ‘encode_EncryptedData’
encode_EncryptedData(unsigned char *p, size_t len, const EncryptedData * data, size_t * size)
^
ticket5.c: In function ‘tkt_DecodeTicket5’:
ticket5.c:320:10: warning: ‘plainsiz’ may be used uninitialized in this function [-Wmaybe-uninitialized]
code = decode_EncTicketPart((unsigned char *)plain, plainsiz, &decr_part, &siz);
^
Change-Id: Ic1b878f01cf82222dc258847747ce192ee5948fc
Reviewed-on: http://gerrit.openafs.org/11955
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Even though master is not using it, we still want to be able to
find it.
Change-Id: I31fa39fe4d4bed5144c5169236b1106bd9f18501
Reviewed-on: http://gerrit.openafs.org/11784
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Make freeing a NULL key pointer a no-op.
Allow passing NULL to afsconf_typedKey_values() when not all
values are needed.
Change-Id: I3a4088747913e9e88be094da891cd2cca0cbb114
Reviewed-on: http://gerrit.openafs.org/11783
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
The function ‘hpr_Initialize’ overwrites the code
returned by ‘ubik_ClientInit’. As a result, ‘hpr_Initialize’
will not report any failure triggered by ‘ubik_ClientInit’.
To fix this problem, store the code returned by ‘rxs_Release’
in a new variable. Only return this code if the function
‘ubik_ClientInit’ worked properly. Otherwise, return the code
provided by ‘ubik_ClientInit’.
Change-Id: I1820e3cbc2131daace01cec0464e56fd2982a783
Reviewed-on: http://gerrit.openafs.org/12137
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
DNS lookup results were being handled with int types.
Fortunately, this seems to be harmless, due to use of
memcpy when the types don't match, and assignment only
when both sides were int.
However, to avoid any future unpleasantness, change
them to afs_uint32.
No functional change should be incurred.
Change-Id: I31aeabb4ae3194a00b29a1fa767d05af167b4e4f
Reviewed-on: http://gerrit.openafs.org/12117
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Chas Williams <3chas3@gmail.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
CVE-2015-8312:
Any pioctl with an input buffer size (ViceIoctl->in_size)
exactly equal to AFS_LRALLOCSIZE (4096 bytes) will cause
a one-byte overwrite of its kernel memory working buffer.
This may crash the operating system or cause other
undefined behavior.
The attacking pioctl must be a valid AFS pioctl code.
However, it need not specify valid arguments (in the ViceIoctl),
since only rudimentary checking is done in afs_HandlePioctl.
Most argument validation occurs later in the individual
pioctl handlers.
Nor does the issuer need to be authenticated or authorized
in any way, since authorization checks also occur much later,
in the individual pioctl handlers. An unauthorized user
may therefore trigger the overrun by either crafting his
own malicious pioctl, or by issuing a privileged
command, e.g. 'fs newalias', with appropriately sized but
otherwise arbitrary arguments. In the latter case, the
attacker will see the expected error message:
"fs: You do not have the required rights to do this operation"
but in either case the damage has been done.
Pioctls are not logged or audited in any way (except those
that cause loggable or auditable events as side effects).
root cause:
afs_HandlePioctli() calls afs_pd_alloc() to allocate two
two afs_pdata structs, one for input and one for output.
The memory for these buffers is based on the requested
size, plus at least one extra byte for the null terminator
to be set later:
requested size allocated
================= =================================
> AFS_LRALLOCSIZ osi_Alloc(size+1)
<= AFS_LRALLOCSIZ afs_AllocLargeSize(AFS_LRALLOCSIZ)
afs_HandlePioctl then adds a null terminator to each buffer,
one byte past the requested size. This is safe in all cases
except one: if the requested in_size was _exactly_
AFS_LRALLOCSIZ (4096 bytes), this null is one byte beyond
the allocated storage, zeroing a byte of kernel memory.
Commit 6260cbecd0 introduced
the null terminators and they were correct at that time.
But the commit message warns:
"note that this works because PIGGYSIZE is always less than
AFS_LRALLOCSIZ"
Commit f8ed1111d7 introduced
the bug by increasing the maximum size of the buffers but
failing to account correctly for the null terminator in
the case of input buffer size == AFS_LRALLOCSIZ.
Commit 592a99d6e6 (master
version of one of the fixes in the recent 1.6.13 security
release) is the fix that drew my attention to this new
bug. Ironically, 592a99 (combined with this commit), will
make it possible to eliminate the "offending" null termination
line altogether since it will now be performed automatically by
afs_pd_alloc().
[kaduk@mit.edu: adjust commit message for CVE number assignment,
reduce unneeded churn in the diff.]
Change-Id: I1a536b3a53ec4b6721fbd39a915207da4358720c
The server logger requires an explicit new line.
Change-Id: Iffbfcfee7499bfa745a63d1b5ccb8038ee06acd0
Reviewed-on: http://gerrit.openafs.org/11841
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Perform range checks on the acl header fields when reading an
acl from a dump stream and when writing an acl to a dump
stream.
Before this change, a bogus value in the total, positive, or
negative acl fields from a dump stream could cause an out of
bounds access of the acl entries table, crashing the volume
server.
Change-Id: Ic7d7f615a37491835af8d92f3c5f1b6a667d9d01
Reviewed-on: http://gerrit.openafs.org/11702
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
The rfail cleanup handler overwrites 'code' ~unconditionally, but
does use an existing 'error' value if present. Since the intent
is to return failure to the caller, preserve the code in the error
variable and do so.
FIXES 131897
Change-Id: I25db2f9ad75a5b856626d39d35f97a09f26bd7a9
Reviewed-on: http://gerrit.openafs.org/12108
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Clean up the show log flag so it is only set by the salvager and
is reset when spawning a child process.
Change-Id: I1702cf98faca583409594d1199a8215ffe08a75e
Reviewed-on: http://gerrit.openafs.org/12001
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
When the -client option is given to the salvageserver, print
Log() messages to stderr instead of losing them.
Change-Id: I065e8136db9a8cc241639fbe34607db884751b95
Reviewed-on: http://gerrit.openafs.org/11729
Reviewed-by: Perry Ruiter <pruiter@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Remove the salvagerserver option to print log messages to stdout. This
was a carry over from the stand-alone salvager and is not appropriate for
a daemon.
Change-Id: I55b99112278cdabb3e9911948dbda6a628030951
Reviewed-on: http://gerrit.openafs.org/11815
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
configure now checks for the standard getmaxyx() macro; failing that,
it looks for the older but pre-standardization getmaxx() and getmaxy(),
then falls back to the 4.2BSD curses _maxx and _maxy fields; if all
else fails, gtx building is disabled.
gtx now defines getmaxyx() itself if necessary, based on the above.
This also fixes a bug in gtx with all ncurses versions > 1.8.0 on
platforms other than NetBSD and OS X: gtx was using the _maxx and
_maxy fields, which starting with ncurses 1.8.1 were off by 1 from
the expected values. As such, behavior of scout and/or afsmonitor
may change on most ncurses-using platforms.
Change-Id: I49778e87adacef2598f0965e09538dfc3d840dcc
Reviewed-on: http://gerrit.openafs.org/12107
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Chas Williams <3chas3@gmail.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
As reported on the -info mailing list, docker is now exporting the
/proc filesystem as read only. ioctl() doesn't need write permissions
to do its work, so change O_RDWR to O_RDONLY.
Change-Id: I2068888b13b6b5e31b1a2205bbcbe43f5f9fc55a
Reviewed-on: http://gerrit.openafs.org/12122
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Remove the undocumented -datelogs option from the salavageserver, which
was a carry over from the standalone salvager program, but is not
appropriate for a daemon.
Change-Id: Ia382d6550e0641edcba55a414e00323755487e18
Reviewed-on: http://gerrit.openafs.org/11814
Reviewed-by: Perry Ruiter <pruiter@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
These options were copied from the salvager man page and are not implemented by
the salvageserver.
Change-Id: Ib6c5b3fd494f1662b958442863e5fbfc0755a0c2
Reviewed-on: http://gerrit.openafs.org/11817
Reviewed-by: Perry Ruiter <pruiter@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Commit 0b9986c875 converted ptserver
to use libcmd for parsing, but erroneously listed the
-default_access argument as CMD_SINGLE instead of CMD_LIST, since
two arguments are needed. This made it impossible to use
-default_access at all, since libcmd would reject an extra argument
and the later argument processing would notice that the second
argument was missing.
FIXES 131731
Change-Id: Ib8241308d4f40f980d635513e2255aafa06c3d8a
Reviewed-on: http://gerrit.openafs.org/12110
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Add the missing -syslog and -syslogfacility options to
the salvageserver man page.
Change-Id: I1cb057a8085c4aeda32bb003cc4cec5035d00407
Reviewed-on: http://gerrit.openafs.org/11816
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
If the footer line is not allocated, programs segfault at runtime.
The printFooter functions should check if the footer
is allocated before printing them.
Change-Id: Ib4066a67ee104be918811e178c0b7d7d33d790b8
Reviewed-on: http://gerrit.openafs.org/11753
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Commit a14e791541
refactored and corrected the counting of requests and aborts.
However, it inadvertently introduced a new undercount for
VL_GetEntryByName* requests, counting them only if
NameIsId(volname), e.g. volname="536870911".
Ensure that the normal case of a non-"numeric" volname is
also counted.
Discovered during review of pullup to 1.6.x.
Change-Id: Ic5dbc1a5871d0e0ff184dc4f3b11e92166c92f65
Reviewed-on: http://gerrit.openafs.org/12106
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Before commit e4a8a7a38d an error message
was printed if looking up a server hostname failed. Restore this, and
also print a message in the now detected case that the lookup returns
loopback addresses only.
Change-Id: Idf7c3133ab5c83e081335ba1dc8fcbddb7da7329
Reviewed-on: http://gerrit.openafs.org/12097
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Fix a minor bug in which an unnecessary syslog connection is opened when
the BosLog is not present (typically, the first time the bosserver is
started) or when the BosLog is a named pipe, even if the -syslog option
was not given.
This bug was introduced in commit bdc7e43117,
which added checks to avoid renaming logs when they are named pipes.
lstat() and S_ISFIFO are provided by libroken, so do not need to be hidden
behind conditional compilation.
Change-Id: I828534be69949fe017cc7dbed1b6798aa4c0ba17
Reviewed-on: http://gerrit.openafs.org/11686
Reviewed-by: Perry Ruiter <pruiter@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
A continuation entry (struct contentry) contains 39 id elements, however
a regular entry (struct prentry) contains only 10 id elements.
Attempting to access more than 10 elements of a regular entry is
undefined behavior.
Use a stuct contentry when when processing continuation entries in
prdb_check. This is done to safely traverse the id arrays of the
continuation entries. Use the new pr_PrintContEntry to print
continuation entries.
The undefined behavior manfests as a segmentation violation in
WalkNextChain() when built with GCC 4.8 with optimization enabled.
Change-Id: I7613345ee6b7b232c5a0645f4f302c3eac0cdc15
Reviewed-on: http://gerrit.openafs.org/11742
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Continuation entries may not be in owner chains. Fix the
comments in WalkOwnerChain (which were probably copied from
WalkNextChain) and add a check and error message for
continuation entries found on owner chains.
Change-Id: I8c49378478cf6a3d31317ff90a52fe1e74517dd3
Reviewed-on: http://gerrit.openafs.org/11751
Reviewed-by: Daria Phoebe Brashear <shadow@your-file-system.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
A continuation entry (struct contentry) contains 39 id elements, however
a regular entry (struct prentry) contains only 10 id elements. Attempting
to access more than 10 elements of a regular entry is undefined
behavior.
Add a new function to safely print continuation entries and change
pr_PrintEntry to avoid accessing the entries array out of bounds.
The pr_PrintEntry function is at this time only used by the prdb_check
and ptclient debugging utilities.
Change-Id: Ie836983c8a5970a9495b87d0627ba6c05d117a9b
Reviewed-on: http://gerrit.openafs.org/11750
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Document the built-in version sub-command which displays
the OpenAFS version string. This sub-command is provided
by the cmd library.
Document the switch style -version option provided by the cmd
library for the initcmd based commands: afsmonitor, scout,
xstat_fs_test, and xstat_cm_test.
Change-Id: Id421d2c68a5c49a2b1a5abb2f3e9ca64ea36cd3e
Reviewed-on: http://gerrit.openafs.org/11161
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Commit 9b0d5f274f added a return code to
BlobScan to allow afs_readdir to return an error when afs_newslot failed
to allocate a buffer. Unfortunately, that change introduced a false
EIO error.
Originally, BlobScan would return a blob number of 0 to indicate the end
of the file has been reached while traversing the directory blobs.
Restore that behavior by changing the cache manager's DRead function to
return ENOENT instead of the generic EIO error to indicate the page to
be read is out of bounds, and change BlobScan to return a blob of zero
to indicate to callers the last blob has been reached. All callers
already check for a blob number of zero, which is out of range.
Change-Id: I5baae8e5377dd49dcca6765b7a4ddc89cca70738
Reviewed-on: http://gerrit.openafs.org/12058
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Tested-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Commit d1d411576c added the vos remaddrs
subcommand, but unfortunately stole the common parameters from
setaddrs. Fix this bug and remove the extra blank line between
the subcommand syntax and the common params macro.
Change-Id: I1171bfadec08ac34679204f0a9245d76c468cafa
Reviewed-on: http://gerrit.openafs.org/12093
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
In src/libuafs, "make" with a large number of jobs (e.g., "make -j16")
can fail because some of the LT_objs depend on make_h_tree having been
called already.
Make "h" (the libuafs header subdirectory) a dependency of all of
LT_objs.
Change-Id: Ie005dbb1f1b0a794c703147062615808a45956dc
Reviewed-on: http://gerrit.openafs.org/12079
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
CVE-2015-7762:
The CMU/Transarc/IBM definition of rx_AckDataSize(nAcks) was mistakenly
computed from sizeof(struct rx_ackPacket) and inadvertently added three
octets to the computed ack data size due to C language alignment rules.
When constructing ack packets these three octets are not assigned a
value before writing them to the network.
Beginning with AFS 3.3, IBM extended the ACK packet with the "maxMTU" ack
trailer value which was appended to the packet according to the
rx_AckDataSize() computation. As a result the three unassigned octets
were unintentionally cemented into the ACK packet format.
In OpenAFS commit 4916d4b422 Nickolai
Zeldovich <kolya@mit.edu> noticed that the size produced by the
rx_AckDataSize(nAcks) macro was dependent upon the compiler and processor
architecture. The rx_AckDataSize() macro was altered to explicitly
expose the three octets that are included in the computation.
Unfortunately, the failure to initialize the three octets went unnoticed.
The Rx implementation maintains a pool of packet buffers that are reused
during the lifetime of the process. When an ACK packet is constructed
three octets from a previously received or transmitted packets will be
leaked onto the network. These octets can include data from a
received packet that was encrypted on the wire and then decrypted.
If the received encrypted packet is a duplicate or if it is outside the
valid window, the decrypted packet will be used immediately to construct
an ACK packet.
CVE-2015-7763:
In OpenAFS commit c7f9307c35 the ACK packet
was further extended in an attempt to detect the path MTU between two
peers. When the ACK reason is RX_ACK_PING a variable number of octets is
appended to the ACK following the ACK trailers.
The implementation failed to initialize all of the padding region.
A variable amount of data from previous packets can be leaked onto the
network. The padding region can include data from a received packet
that was encrypted on the wire and then decrypted.
OpenAFS 1.5.75 through 1.5.78 and all 1.6.x releases (including release
candidates) are vulnerable.
Credits:
Thanks to John Stumpo for identifying both vulnerabilities.
Thanks to Simon Wilkinson for patch development.
Thanks to Ben Kaduk for managing the security release cycle.
Change-Id: I29e47610e497c0ea94033450f434da11c367027c
cm_BPlusDirLookup() and cm_Lookup() can return CM_ERROR_INEXACT_MATCH
which is not a fatal error. Instead it is an indication that the returned
cm_scache object was not a case sensitive match. Do not fail the request
and do not leak the cm_scache reference.
Change-Id: Ieef3ce1ac96a8794859b5b9c530545d4fdd26bd5
Reviewed-on: http://gerrit.openafs.org/12057
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
cm_Lookup() must not mask a CM_ERROR_AMBIGUOUS_FILENAME error by
converting it to CM_ERROR_BPLUS_NOMATCH. Doing so results in the
redirector believing that the object does not exist instead of
there being a STATUS_OBJECT_NAME_COLLISION.
Change-Id: Iaa84d50271c234a84e11dd58d78ef90f5d224032
Reviewed-on: http://gerrit.openafs.org/11930
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
The redirector is supposed to reject access to file objects if there
is no case exact match and multiple entries match in a case insensitive
comparison. The check was only present in the AFSLocateNameEntry()
function and not elsewhere.
Fix the AFSLocateNameEntry() call and addd the missing checks.
Change-Id: I15aba954179fa85e28b348989779bc05122c0037
Reviewed-on: http://gerrit.openafs.org/11929
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Although pioctl operations are delivered through the redirector the
contents of the operations are opaque to the redirector. Therefore,
the cm_req must not be initialized as a redirector operation. If they
are the necessary invalidation notifications for symlink and mount point
operations will not be delivered.
Change-Id: I48c2d89d2b2e0fc3f0ef56e7731108a8c51e1674
Reviewed-on: http://gerrit.openafs.org/12062
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
clientServiceProviderKeyExists() must use AFSREG_CLT_SVC_PROVIDER_SUBKEY
since it is a relative path from HKEY_LOCAL_MACHINE.
Change-Id: I975d594bfe69c563f692978057c08b834d54b8b1
Reviewed-on: http://gerrit.openafs.org/12059
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
If cm_noIPAddrs == 0, then no servers will be probed. If
syscfg_GetIFInfo() fails then cm_noIPAddrs is set to 0. Therefore,
also set cm_LanAdapterChangeDetected to non-zero if syscfg_GetIFInfo()
fails so that the interface info can be queried again prior to a server
probe attempt.
Change-Id: I6639441fa6266671cfb875256eb23c3b018e67c9
Reviewed-on: http://gerrit.openafs.org/12055
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Do not blindly install a network provider for the service since at
least one end user organization does not install the service's network
provider.
Change-Id: I15a528ff34ffd3e060fdbd93545af3857592c835
Reviewed-on: http://gerrit.openafs.org/12051
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Tested-by: Jeffrey Altman <jaltman@your-file-system.com>
If we open a handle, close it.
Change-Id: I1a5b2308a91f3c66791e65f76ca17ae52d34789f
Reviewed-on: http://gerrit.openafs.org/12050
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Tested-by: Jeffrey Altman <jaltman@your-file-system.com>
In cm_CheckServersMulti() if cm_ConnByServer() fails or if cm_noIPAddr is
zero then a cm_server.pingCount will be leaked. This can result in
servers being marked down and never restored to an up state.
This change adds the necessary pingCount decrement and moves the
assignment of the cm_server_t pointer to serversp[] to make it clear
that the cm_server_t will not be in the array if a failure occurs.
Only objects in the array will have the pingCount decremented after
the RPCs are issued.
Change-Id: I18895c848039e4131a674d814019f236a1b0e5b5
Reviewed-on: http://gerrit.openafs.org/12048
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Update the log messages to use modern formatting specifiers for
volume ids and inodes in the volume dump and restore code.
Change-Id: Ic2844e389e5951d2f2bbbc31a86c2342f2e8d848
Reviewed-on: http://gerrit.openafs.org/11701
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
In 0a0927497c an extra "pingCount"
format parameter was added in cm_DumpServers(). Remove it.
Change-Id: I79c2212c11319d7f94f963214d90b0530a978ab5
Reviewed-on: http://gerrit.openafs.org/12046
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
