When reading a callback state dump, check the return values from
read(2) instead of ignoring them. This adds a new static function,
ReadBytes(), which handles reading a requested number of bytes from a
file and bailing if there is an error.
Reviewed-on: http://gerrit.openafs.org/9989
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Derrick Brashear <shadow@your-file-system.com>
(cherry picked from commit c854cb31ff)
Change-Id: Ieffa11c6049eceb23bc6bf9169454529df3ef766
Reviewed-on: https://gerrit.openafs.org/13505
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
When writing a callback state dump, test the return values from
write(2), but don't do anything based on the test. This avoids
compiler warnings when building on Ubuntu 12.10, with gcc 4.7.2 and
eglibc 2.15-0ubuntu20.1. This adds a new macro, WriteBytes(), which
handles writing a requested number of bytes to a file and ignoring
errors.
Reviewed-on: http://gerrit.openafs.org/9991
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Derrick Brashear <shadow@your-file-system.com>
(cherry picked from commit e16bef6136)
Change-Id: Ib88d8354f68f26fb001e3b650791236e42789f31
Reviewed-on: https://gerrit.openafs.org/13504
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Tested-by: Stephan Wiesand <stephan.wiesand@desy.de>
Recent versions of gcc generate a format overflow warning on the dfstring
buffer in fs.c. Increase the size of the buffer to avoid a possible buffer
overflow.
fs.c: In function ‘AclToString’:
fs.c:770:30: error: ‘%s’ directive writing up to 1024 bytes
into a region of size between 13 and 23 [-Werror=format-overflow=]
sprintf(dfsstring, " dfs:%d %s", acl->dfs, acl->cell);
^~
fs.c:770:2: note: ‘sprintf’ output between 8 and 1042 bytes into
a destination of size 30
sprintf(dfsstring, " dfs:%d %s", acl->dfs, acl->cell);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Reviewed-on: https://gerrit.openafs.org/12917
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
(cherry picked from commit c84f36a9b8)
Reviewed-on: https://gerrit.openafs.org/13099
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
(cherry picked from commit e1b71e8845)
Change-Id: I332d283c787f35b0d5d72e1c9e8de1145e177719
Reviewed-on: https://gerrit.openafs.org/13499
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Tested-by: Stephan Wiesand <stephan.wiesand@desy.de>
Recent versions of gcc generate an overflow warning in the butc DUMPNAME macro
when copying values into the finishedMsg1 buffer. Increase the size of the
destination buffer to avoid a possible buffer overflow.
dump.c:88:24: error: ‘%s’ directive writing up to 63 bytes into
a region of size 50 [-Werror=format-overflow=]
sprintf(dumpname, "%s (DumpId %u)", name, dbDumpId);
^
dump.c:1294:5: note: in expansion of macro ‘DUMPNAME’
DUMPNAME(finishedMsg1, nodePtr->dumpSetName, dparams.databaseDumpId);
^~~~~~~~
dump.c:88:6: note: ‘sprintf’ output between 12 and 84 bytes into
a destination of size 50
sprintf(dumpname, "%s (DumpId %u)", name, dbDumpId);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
dump.c:1294:5: note: in expansion of macro ‘DUMPNAME’
DUMPNAME(finishedMsg1, nodePtr->dumpSetName, dparams.databaseDumpId);
^~~~~~~~
Reviewed-on: https://gerrit.openafs.org/12916
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
(cherry picked from commit cec45d5944)
Reviewed-on: https://gerrit.openafs.org/13097
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
(cherry picked from commit dba69c2a19)
Change-Id: Idb1bc17619018073d14f6047cad404283898a005
Reviewed-on: https://gerrit.openafs.org/13498
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Tested-by: Stephan Wiesand <stephan.wiesand@desy.de>
Fixes these warnings:
namei_ops.c: In function 'namei_copy_on_write':
namei_ops.c:1328:31: warning: 'snprintf' output may be truncated before the last format character [-Wformat-truncation=]
snprintf(path, sizeof(path), "%s-tmp", name.n_path);
^~~~~~~~
namei_ops.c:1328:2: note: 'snprintf' output between 5 and 260 bytes into a destination of size 259
snprintf(path, sizeof(path), "%s-tmp", name.n_path);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
vol_split.c: In function 'split_volume':
vol_split.c:576:22: warning: 'sprintf' may write a terminating nul past the end of the destination [-Wformat-overflow=]
sprintf(symlink, "#%s", V_name(newvol));
^~~~~
vol_split.c:576:5: note: 'sprintf' output between 2 and 33 bytes into a destination of size 32
sprintf(symlink, "#%s", V_name(newvol));
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Reviewed-on: https://gerrit.openafs.org/12722
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
(cherry picked from commit 0a9a6b57ce)
Reviewed-on: https://gerrit.openafs.org/12723
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
(cherry picked from commit 688b357086)
Change-Id: I44c8e8e8a8049da4b8a1f4c2cfe6c7245661b7b7
Reviewed-on: https://gerrit.openafs.org/13497
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Tested-by: Stephan Wiesand <stephan.wiesand@desy.de>
With gcc 7.3, we start getting several warnings like the following:
vutil.c: In function ‘VWalkVolumeHeaders’:
vutil.c:860:34: error: ‘%s’ directive output may be truncated writing up to 255 bytes into a region of size 63 [-Werror=format-truncation=]
snprintf(name, VMAXPATHLEN, "%s" OS_DIRSEP "%s", partpath, dentry->d_name);
Most or all of these truncations should be okay, but increase the size
of the relevant buffers so we can build with warning checking turned
on.
Reviewed-on: https://gerrit.openafs.org/13274
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
(cherry picked from commit 2daa413e3e)
Reviewed-on: https://gerrit.openafs.org/13459
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
(cherry picked from commit 232bd12b07)
Change-Id: I64e489cb1b349fc89a304cd8ef2fa68730e29512
Reviewed-on: https://gerrit.openafs.org/13496
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Tested-by: Stephan Wiesand <stephan.wiesand@desy.de>
This commit updates the code imported from heimdal to
3fe55728404c602884f16126e9cc60fc5a3d8f20 (switch-from-svn-to-git-2993-g3fe5572)
[This commit has been backported to OpenAFS 1.6.x branch, which imports
only krb5/config_file.c]
Reviewed-on: http://gerrit.openafs.org/7612
Tested-by: Jeffrey Altman <jaltman@your-file-system.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
(cherry picked from commit 9c0b7be87d)
Change-Id: I99d0b15168d6309ee04ac0f7d2bde27c21ba4382
Reviewed-on: https://gerrit.openafs.org/13495
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Tested-by: Stephan Wiesand <stephan.wiesand@desy.de>
Update all three copies in the tree, and the rpm specfile.
Reviewed-on: https://gerrit.openafs.org/13134
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
(cherry picked from commit 4a2b5101af)
Reviewed-on: https://gerrit.openafs.org/13409
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
(cherry picked from commit eca7ade855)
[stephan.wiesand@desy.de: added fourth instance still present on 1.6.x]
Change-Id: Ie06fb1f36f60fe1a60342431130d13d66ede8aff
Reviewed-on: https://gerrit.openafs.org/13418
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
-----BEGIN PGP SIGNATURE-----
iQHGBAABCgAsFiEE2WGV4E2ARf9BYP0XKNmm82TrdRIFAluX5VAOHGthZHVrQG1p
dC5lZHUACgkQKNmm82TrdRKUiAweNfmtCQ4U2KWR1lgnNr4ZNBuUif++aMnIb+et
lWiaZGuLnDfFcOHxzDvFlHo6JTuZLITZJuZjzL2+QaFyYnwUh+Tnd/skRifXMGq7
/sQCWFpLzVJ07Kun6scapS6OcHtN3Has00HXlgpo6yW4M0K24RYV+GaibnXUiV8I
TCJOglZb3TO5M4/KsY1pRcZEoNv1ps38X/tqd0HKovmnO5Ybjo9seNtr52YTcD7V
KJbQ+qCjbNBsfOPv//jLQIlqo3zE4aor7OaMCtmrdBb7IMPspGqX8mfpWm7qV1sd
8F9O4J+zQmrzHwGI767MLC868fvFwu1dMJ1HFTMUMHloLQuLsvVsnZO6vrevpeGb
RI2KaKHgIBbkCdFi4kAZ5uGi4+2lSN9sKCnnL4IIvsinOxM/8+jIFH3NJL04UaR+
higM7Us9y79dOuT7/eGCI5bHnJKeZKbWoIFl2IJws3g/F4gA3lr51HBAaZv4hUI7
H19Z6uAUju9xttnFBxdvzoiVAuGk0Od8Aw==
=Lysx
-----END PGP SIGNATURE-----
Merge tag 'openafs-stable-1_6_23' into openafs-stable-1_6_x
Join the history of the security release into the
1.6.x stable release branch.
Change-Id: I9de0a04c89b079b205475eed93ac8e4a7f46797b
Commit 9ebff4c6ca introduced audit
framework support for several butc-related data types, but had
a typo ('$d' for '%d') in a couple of places, that was not reported
by compiler format-string checking. Fix the typo to properly print
all the auditable data.
(cherry picked from commit d5816fd6cd)
(cherry picked from commit 9060181820)
(cherry picked from commit 0cdb370f18)
Change-Id: I0d1cb15d02225a8557da09ed72efbc5103e1ec1b
Commit 9ebff4c6ca introduced audit
framework support for several butc-related data types, but had
a typo ('$d' for '%d') in a couple of places, that was not reported
by compiler format-string checking. Fix the typo to properly print
all the auditable data.
(cherry picked from commit d5816fd6cd)
(cherry picked from commit 9060181820)
Change-Id: I49a8fff424e0a8cc7bdcdcca2878e614d6e70f20
Use the standard routine to pick a client security object, instead of
always assuming rxnull. Respect -localauth as well as being able to
use the current user's tokens, but also provide a -nobutcauth argument
to fall back to the historical rxnull behavior (but only for the connections
to butc; vldb and budb connections are not affected).
(cherry picked from commit 345ee34236)
(cherry picked from commit ed217df4b2)
(cherry picked from commit 3f06dd4f73)
Use the standard routine to pick a client security object, instead of
always assuming rxnull. Respect -localauth as well as being able to
use the current user's tokens, but also provide a -nobutcauth argument
to fall back to the historical rxnull behavior (but only for the connections
to butc; vldb and budb connections are not affected).
(cherry picked from commit 345ee34236)
(cherry picked from commit ed217df4b2)
The butc -localauth option is available to use the cell-wide key to
authenticate to the vlserver and buserver, which in normal deployments
will require incoming connections to be authenticated as a superuser.
In such cases, the cell-wide key is also available for use in
authenticating incoming connections to the butc, which would otherwise
have been completely unauthenticated.
Because of the security hazards of allowing unauthenticaed inbound
RPCs, especially ones that manipulate backup information and are allowed
to initiate outboud RPCs authenticated as the superuser, default to
not allowing unauthenticated inbound RPCs at all. Provide an opt-out
command-line argument for deployments that require this functionality
and have configured their network environment (firewall/etc.) appropriately.
(cherry picked from commit 1b199eeafa)
(cherry picked from commit fa04588907)
Change-Id: Ib796fd4d61cc5d2e98f1b1e787f3267456b0ffe8
Make the actual implementations into helper functions, with the RPC
stubs calling the helpers and doing the auditing on the results, akin
to most other server programs in the tree. This relies on support for
some additional types having been added to the audit framework.
(cherry picked from commit c43169fd36)
(cherry picked from commit 6f8c0c8134)
(cherry picked from commit 23f3f2e0d9)
Change-Id: Icb4a9ca3cce81b088268655a648823f3e8260f0a
Add support for several complex butc types to enable butc auditing.
(cherry picked from commit 41d2dd569a)
(cherry picked from commit 049b7eafe1)
Change-Id: I6662f028e300afaa5e2586db1a590f9ea8ec3139
This local stub was present in the original IBM import and is unused.
It will conflict with the real audit code once we start adding auditing
to the TC_ RPCs, so remove it now.
(cherry picked from commit 50216dbbc3)
(cherry picked from commit 7eb650a6ed)
(cherry picked from commit cf69365f04)
Change-Id: Idf9d3dfa040cdd34437d1c97ce27a1225a356993
RPCs with unbounded arrays as inputs are susceptible to remote
denial-of-service (DOS) attacks. A malicious client may submit an RPC
request with an arbitrarily large array, forcing the server to expend
large amounts of network bandwidth, cpu cycles, and heap memory to
unmarshal the input.
Instead, issue an error message and stop rxgen when it detects an RPC
defined with an unbounded input array. Thus we will detect the problem
at build time and prevent any future unbounded input arrays.
(cherry picked from commit a4c1d5c48d)
(cherry picked from commit 2cf5cfa856)
(cherry picked from commit 289a5643e7)
Change-Id: If5222aab9ce700ba8d9520e5e2e81e66e1b87fd1
Several AFSVol* RPCs are defined with an unbounded XDR "string" as
input.
RPCs with unbounded arrays as inputs are susceptible to remote
denial-of-service (DOS) attacks. A malicious client may submit an
AFSVol* request with an arbitrarily large string, forcing the volserver
to expend large amounts of network bandwidth, cpu cycles, and heap
memory to unmarshal the input.
Instead, give each input "string" an appropriate size.
Volume names are inherently capped to 32 octets (including trailing NUL)
by the protocol, but there is less clearly a hard limit on partition names.
The Vol_PartitionInfo{,64} functions accept a partition name as input and
also return a partition name in the output structure; the output values
have wire-protocol limits, so larger values could not be retrieved by clients,
but for denial-of-service purposes, a more generic PATH_MAX-like value seems
appropriate. We have several varying sources of such a limit in the tree, but
pick 4k as the least-restrictive.
[kaduk@mit.edu: use a larger limit for pathnames and expand on PATH_MAX in
commit message]
(cherry picked from commit 8b92d015cc)
(cherry picked from commit fe41fa565b)
(cherry picked from commit 39b675e243)
Change-Id: Idad0b0abf582b356042245398e1317a610ff321e
AFSVolForwardMultiple is defined with an input parameter that is defined
to XDR as an unbounded array of replica structs:
typedef replica manyDests<>;
RPCs with unbounded arrays as inputs are susceptible to remote
denial-of-service (DOS) attacks. A malicious client may submit an
AFSVolForwardMultiple request with an arbitrarily large array, forcing
the volserver to expend large amounts of network bandwidth, cpu cycles,
and heap memory to unmarshal the input.
Even though AFSVolForwardMultiple requires superuser authorization, this
attack is exploitable by non-authorized actors because XDR unmarshalling
happens long before any authorization checks can occur.
Add a bounding constant (NMAXNSERVERS 13) to the manyDests input array.
This constant is derived from the current OpenAFS vldb implementation, which
is limited to 13 replica sites for a given volume by the layout (size) of the
serverNumber, serverPartition, and serverFlags fields.
[kaduk@mit.edu: explain why this constant is used]
(cherry picked from commit 97b0ee4d9c)
(cherry picked from commit fac3749f0d)
(cherry picked from commit ea30e64d1b)
Change-Id: Ib2e5d4cc660e0a278b9dbd10ac2db656239e1302
BUDB_SaveText is defined with an input parameter that is defined to XDR
as an unbounded array of chars:
typedef char charListT<>;
RPCs with unbounded arrays as inputs are susceptible to remote
denial-of-service (DOS) attacks. A malicious client may submit a
BUDB_SaveText request with an arbitrarily large array, forcing the budb
server to expend large amounts of network bandwidth, cpu cycles, and
heap memory to unmarshal the input.
Modify the XDR definition of charListT so it is bounded. This typedef
is shared (as an OUT parameter) by BUDB_GetText and BUDB_DumpDB, but
fortunately all in-tree callers of the client routines specify the same
maximum length of 1024.
Note: However, SBUDB_SaveText server implementation seems to allow for up to
BLOCK_DATA_SIZE (2040) = BLOCKSIZE (2048) - sizeof(struct blockHeader)
(8), and it's unknown if any out-of-tree callers exist. Since we do not need a
tight bound in order to avoid the DoS, use a somewhat higher maximum of
4096 bytes to leave a safety margin.
[kaduk@mit.edu: bump the margin to 4096; adjust commit message to match]
(cherry picked from commit 124445c0c4)
(cherry picked from commit 87f199c141)
(cherry picked from commit c5c3a858b2)
Change-Id: I6802e76a5f6e39e31ece66d1ff00ed11b47b6c36
VL_RegisterAddrs is defined with an input argument of type bulkaddrs,
which is defined to XDR as an unbounded array of afs_uint32 (IPv4 addresses):
typedef afs_uint32 bulkaddrs<>
The <> with no value instructs rxgen to build client and server stubs
that allow for a maximum size of "~0u" or 0xFFFFFFFF.
Ostensibly the bulkaddrs array is unbounded to allow it to be shared
among VL_RegisterAddrs, VL_GetAddrs, and VL_GetAddrsU. The VL_GetAddrs*
RPCs use bulkaddrs as an output array with a maximum size of MAXSERVERID
(254). VL_RegisterAddrss uses bulkaddrs as an input array, with a
nominal size of VL_MAXIPADDRS_PERMH (16).
However, RPCs with unbounded array inputs are susceptible to remote
denial-of-service attacks. That is, a malicious client may send a
VL_RegisterAddrs request with an arbitrarily long array, forcing the
vlserver to expend large amounts of network bandwidth, cpu cycles, and
heap memory to unmarshal the argument. Even though VL_RegisterAddrs
requires superuser authorization, this attack is exploitable by
non-authorized actors because XDR unmarshalling happens long before any
authorization checks can occur.
Because all uses of the type that our implementation support have fixed
bounds on valid data (whether input or output), apply an arbitrary
implementation limit (larger than any valid structure would be), to
prevent this class of attacks in the XDR decoder.
[kaduk@mit.edu: limit the bulkaddrs type instead of introducing a new type]
(cherry picked from commit 7629209219)
(cherry picked from commit 4218dc0a2d)
(cherry picked from commit 38f401ae7e)
Change-Id: Ib0798af007af14a2a91ae280c0f28838f33d1a65
In STC_ReadLabel, the interaction with the tape device is
synchronous, so there is no need to allocate a task ID for status
monitoring. However, we do need to initialize the output value,
to avoid writing stack garbage on the wire.
(cherry picked from commit f5a80115f8)
(cherry picked from commit 418b2ab56c)
(cherry picked from commit babbb2824a)
Change-Id: Ie18bbe7542a23d2ce952cfcd5288ee0aa43bb71f
VOTE_Debug and VOTE_XDebug (udebug) both leave a single field
uninitialized if there is no current transaction. This leaks the memory
contents of the ubik server over the wire.
struct ubik_debug
- 4 bytes in member writeTrans
In common code to both RPCs, ensure that writeTrans is always
initialized.
[kaduk@mit.edu: switch to memset]
(cherry picked from commit 7a7c1f751c)
(cherry picked from commit 0ee86cc3f9)
(cherry picked from commit 9db5fcf460)
Change-Id: I1c9fc9a6a8bb8aed04f814e4da041af3f49a7401
KAM_ListEntry (kas list) does not initialize its output correctly. It
leaks kaserver memory contents over the wire:
struct kaindex
- up to 64 bytes for member name
- up to 64 bytes for member instance
Initialize the buffer.
[kaduk@mit.edu: move initialization to top of server routine]
(cherry picked from commit b604ee7add)
(cherry picked from commit c912830e9c)
(cherry picked from commit 04fb009f15)
Change-Id: I613b1f46b913d4208bac15eb92274127da14e9c9
TC_ScanStatus (backup status) and TC_GetStatus (internal backup status
watcher) do not initialize their output buffers. They leak memory
contents over the wire:
struct tciStatusS
- up to 64 bytes in member taskName (TC_MAXNAMELEN 64)
- up to 64 bytes in member volumeName "
Initialize the buffers.
[kaduk@mit.edu: move initialization to top of server routines]
(cherry picked from commit be0142707c)
(cherry picked from commit 43b3efd4f8)
(cherry picked from commit a41b75a13b)
Change-Id: Ibe35ca06eb663399f0b9e14d7487d91553cd67c8
TC_ReadLabel (backup readlabel) does not initialize its output buffer
completely. It leaks butc memory contents over the wire:
struct tc_tapeLabel
- up to 32 bytes from member afsname (TC_MAXTAPELEN 32)
- up to 32 bytes from member pname (TC_MAXTAPELEN 32)
Initialize the buffer.
[kaduk@mit.edu: move initialization to the RPC stub]
(cherry picked from commit 52f4d63148)
(cherry picked from commit b7e53b9e97)
(cherry picked from commit 3e0294543d)
Change-Id: I4e8ab1b94d36e9904a9505cd7f0e97cc6fb3a40f
The following budb RPCs do not initialize their output correctly.
This leaks buserver memory contents over the wire:
BUDB_FindLatestDump (backup dump)
BUDB_FindDump (backup volrestore, diskrestore, volsetrestore)
BUDB_GetDumps (backup dumpinfo)
BUDB_FindLastTape (backup dump)
struct budb_dumpEntry
- up to 32 bytes in member volumeSetName
- up to 256 bytes in member dumpPath
- up to 32 bytes in member name
- up to 32 bytes in member tape.tapeServer
- up to 32 bytes in member tape.format
- up to 256 bytes in member dumper.name
- up to 128 bytes in member dumper.instance
- up to 256 bytes in member dumper.cell
Initialize the buffer in common routine FillDumpEntry.
(cherry picked from commit e967714711)
(cherry picked from commit 6f26a945ad)
(cherry picked from commit b4543ae233)
Change-Id: I713f967eebc1286764b9658ff4ddccb65f456480
RXAFSCB_TellMeAboutYourself does not completely initialize its output
buffers. This leaks kernel memory over the wire:
struct interfaceAddr
Unix cache manager (libafs)
- up to 124 bytes in array addr_in ((AFS_MAX_INTERFACE_ADDR 32 * 4) - 4))
- up to 124 bytes in array subnetmask "
- up to 124 bytes in array mtu "
Windows cache manager
- 64 bytes in array addr_in ((AFS_MAX_INTERFACE_ADDR 32 - CM_MAXINTERFACE_ADDR 16)* 4)
- 64 bytes in array subnetmask "
- 64 bytes in array mtu "
The following implementations of SRXAFSCB_TellMeAboutYourself are not susceptible:
- fsprobe
- libafscp
- xstat_fs_test
Initialize the buffer.
(cherry picked from commit 211b6d6a43)
(cherry picked from commit a6557ffa64)
(cherry picked from commit 0dbbcc9ac6)
Change-Id: Ic977c8a473df12f64d2865cd68f1f42744b57d9e
RXAFSCB_GetLock (cmdebug) does not correctly initialize its output.
This leaks kernel memory over the wire:
struct AFSDBLock
- up to 14 bytes for member name (16 - '<cellname>\0')
Initialize the buffer.
(cherry picked from commit b52eb11a08)
(cherry picked from commit 3dea4adaa3)
(cherry picked from commit f0c4f8d899)
Change-Id: I3935968bacb8e063fd1fdd2fc52efd2258a5eb99
PR_ListEntries (pts listentries) does not properly initialize its output
buffers. This leaks ptserver memory over the wire:
struct prlistentries
- up to 62 bytes for each entry name (PR_MAXNAMELEN 64 - 'a\0')
Initialize the buffer, and remove the now redundant memset for the
reserved fields.
(cherry picked from commit 9d1aeb5d76)
(cherry picked from commit e19ad4cdde)
(cherry picked from commit 7ee2586168)
Change-Id: I42d32876ddf8fa98744620fdf75b4e0783b93aba
AFSVolMonitor (vos status) does not properly initialize its output
buffers. This leaks information from volserver memory:
struct transDebugInfo
- up to 29 bytes in member lastProcName (30-'\0')
- 16 bytes in members readNext, tranmitNext, lastSendTime,
lastReceiveTime
Initialize the buffers. This must be done on a per-buffer basis inside
the loop, since realloc is used to expand the storage if needed,
and there is not a standard realloc API to zero the newly allocated storage.
[kaduk@mit.edu: update commit message]
(cherry picked from commit 26924fd508)
(cherry picked from commit 2d22756de7)
(cherry picked from commit 37cbe68577)
Change-Id: I1eab9e35207fed5d151c70962c00b6fa8ac7da58
AFSVolPartitionInfo and AFSVolPartitionInfo64 (vos partinfo) do not
properly initialize their reply buffers. This leaks the contents of
volserver memory over the wire:
AFSVolPartitionInfo (struct diskPartition)
- up to 24 bytes in member name (32-'/vicepa\0'))
- up to 12 bytes in member devName (32-'/vicepa/Lock/vicepa\0'))
AFSVolPartitionInfo64 (struct diskPartition64)
- up to 248 bytes in member name (256-'/vicepa\0'))
- up to 236 bytes in member devName (256-'/vicepa/Lock/vicepa\0')
Initialize the output buffers.
[kaduk@mit.edu: move memset to top-level function scope of RPC handlers]
(cherry picked from commit 76e62c1de8)
(cherry picked from commit 28edf734db)
(cherry picked from commit f1c9c0160e)
Change-Id: I48348b326f0933a0fcb556425f085abad36d3bea
SPR_IDToName does not completely initialize the return array of names,
and thus leaks information from ptserver memory:
- up to 62 bytes per requested id (PR_MAXNAMELEN 64 - 'a\0')
Use calloc to ensure that all memory sent on the wire is initialized,
preventing the information leak.
[kaduk@mit.edu: switch to calloc; update commit message]
(cherry picked from commit 70b0136d55)
(cherry picked from commit c8c8682bb0)
(cherry picked from commit 40343287fb)
Change-Id: I793ccc2f3595344e72e9b4ba948a2266f1c4c0a5
The butc -localauth option is available to use the cell-wide key to
authenticate to the vlserver and buserver, which in normal deployments
will require incoming connections to be authenticated as a superuser.
In such cases, the cell-wide key is also available for use in
authenticating incoming connections to the butc, which would otherwise
have been completely unauthenticated.
Because of the security hazards of allowing unauthenticaed inbound
RPCs, especially ones that manipulate backup information and are allowed
to initiate outboud RPCs authenticated as the superuser, default to
not allowing unauthenticated inbound RPCs at all. Provide an opt-out
command-line argument for deployments that require this functionality
and have configured their network environment (firewall/etc.) appropriately.
(cherry picked from commit 1b199eeafa)
Change-Id: I914f867bf3a328de0e994f999b5e106a6efe71b5
Make the actual implementations into helper functions, with the RPC
stubs calling the helpers and doing the auditing on the results, akin
to most other server programs in the tree. This relies on support for
some additional types having been added to the audit framework.
(cherry picked from commit c43169fd36)
(cherry picked from commit 6f8c0c8134)
Change-Id: I7535b60d730975ab9ebbb4a8cb7de45fca29c985
Add support for several complex butc types to enable butc auditing.
(cherry picked from commit 41d2dd569a)
Change-Id: Ib26984b614d4a44d9baa33552c0b7d3de736c97d
This local stub was present in the original IBM import and is unused.
It will conflict with the real audit code once we start adding auditing
to the TC_ RPCs, so remove it now.
(cherry picked from commit 50216dbbc3)
(cherry picked from commit 7eb650a6ed)
Change-Id: I87e7df4ff40d3a233de9df4c7edad8e9d35aebf4
RPCs with unbounded arrays as inputs are susceptible to remote
denial-of-service (DOS) attacks. A malicious client may submit an RPC
request with an arbitrarily large array, forcing the server to expend
large amounts of network bandwidth, cpu cycles, and heap memory to
unmarshal the input.
Instead, issue an error message and stop rxgen when it detects an RPC
defined with an unbounded input array. Thus we will detect the problem
at build time and prevent any future unbounded input arrays.
(cherry picked from commit a4c1d5c48d)
(cherry picked from commit 2cf5cfa856)
Change-Id: I6b740d0ad75ac8a6c8333cf164220691a04c144b
Several AFSVol* RPCs are defined with an unbounded XDR "string" as
input.
RPCs with unbounded arrays as inputs are susceptible to remote
denial-of-service (DOS) attacks. A malicious client may submit an
AFSVol* request with an arbitrarily large string, forcing the volserver
to expend large amounts of network bandwidth, cpu cycles, and heap
memory to unmarshal the input.
Instead, give each input "string" an appropriate size.
Volume names are inherently capped to 32 octets (including trailing NUL)
by the protocol, but there is less clearly a hard limit on partition names.
The Vol_PartitionInfo{,64} functions accept a partition name as input and
also return a partition name in the output structure; the output values
have wire-protocol limits, so larger values could not be retrieved by clients,
but for denial-of-service purposes, a more generic PATH_MAX-like value seems
appropriate. We have several varying sources of such a limit in the tree, but
pick 4k as the least-restrictive.
[kaduk@mit.edu: use a larger limit for pathnames and expand on PATH_MAX in
commit message]
(cherry picked from commit 8b92d015cc)
(cherry picked from commit fe41fa565b)
Change-Id: I0ba32b30d260e0e1f66cad0f383441a446f7b7bc
AFSVolForwardMultiple is defined with an input parameter that is defined
to XDR as an unbounded array of replica structs:
typedef replica manyDests<>;
RPCs with unbounded arrays as inputs are susceptible to remote
denial-of-service (DOS) attacks. A malicious client may submit an
AFSVolForwardMultiple request with an arbitrarily large array, forcing
the volserver to expend large amounts of network bandwidth, cpu cycles,
and heap memory to unmarshal the input.
Even though AFSVolForwardMultiple requires superuser authorization, this
attack is exploitable by non-authorized actors because XDR unmarshalling
happens long before any authorization checks can occur.
Add a bounding constant (NMAXNSERVERS 13) to the manyDests input array.
This constant is derived from the current OpenAFS vldb implementation, which
is limited to 13 replica sites for a given volume by the layout (size) of the
serverNumber, serverPartition, and serverFlags fields.
[kaduk@mit.edu: explain why this constant is used]
(cherry picked from commit 97b0ee4d9c)
(cherry picked from commit fac3749f0d)
Change-Id: I57a0aa15b5a92a111f835b7a58f7495376e3e63b
BUDB_SaveText is defined with an input parameter that is defined to XDR
as an unbounded array of chars:
typedef char charListT<>;
RPCs with unbounded arrays as inputs are susceptible to remote
denial-of-service (DOS) attacks. A malicious client may submit a
BUDB_SaveText request with an arbitrarily large array, forcing the budb
server to expend large amounts of network bandwidth, cpu cycles, and
heap memory to unmarshal the input.
Modify the XDR definition of charListT so it is bounded. This typedef
is shared (as an OUT parameter) by BUDB_GetText and BUDB_DumpDB, but
fortunately all in-tree callers of the client routines specify the same
maximum length of 1024.
Note: However, SBUDB_SaveText server implementation seems to allow for up to
BLOCK_DATA_SIZE (2040) = BLOCKSIZE (2048) - sizeof(struct blockHeader)
(8), and it's unknown if any out-of-tree callers exist. Since we do not need a
tight bound in order to avoid the DoS, use a somewhat higher maximum of
4096 bytes to leave a safety margin.
[kaduk@mit.edu: bump the margin to 4096; adjust commit message to match]
(cherry picked from commit 124445c0c4)
(cherry picked from commit 87f199c141)
Change-Id: If62f472932f08b6211f829a4b2bfbeb85585886b
VL_RegisterAddrs is defined with an input argument of type bulkaddrs,
which is defined to XDR as an unbounded array of afs_uint32 (IPv4 addresses):
typedef afs_uint32 bulkaddrs<>
The <> with no value instructs rxgen to build client and server stubs
that allow for a maximum size of "~0u" or 0xFFFFFFFF.
Ostensibly the bulkaddrs array is unbounded to allow it to be shared
among VL_RegisterAddrs, VL_GetAddrs, and VL_GetAddrsU. The VL_GetAddrs*
RPCs use bulkaddrs as an output array with a maximum size of MAXSERVERID
(254). VL_RegisterAddrss uses bulkaddrs as an input array, with a
nominal size of VL_MAXIPADDRS_PERMH (16).
However, RPCs with unbounded array inputs are susceptible to remote
denial-of-service attacks. That is, a malicious client may send a
VL_RegisterAddrs request with an arbitrarily long array, forcing the
vlserver to expend large amounts of network bandwidth, cpu cycles, and
heap memory to unmarshal the argument. Even though VL_RegisterAddrs
requires superuser authorization, this attack is exploitable by
non-authorized actors because XDR unmarshalling happens long before any
authorization checks can occur.
Because all uses of the type that our implementation support have fixed
bounds on valid data (whether input or output), apply an arbitrary
implementation limit (larger than any valid structure would be), to
prevent this class of attacks in the XDR decoder.
[kaduk@mit.edu: limit the bulkaddrs type instead of introducing a new type]
(cherry picked from commit 7629209219)
(cherry picked from commit 4218dc0a2d)
Change-Id: Ic3112ebe13cf3550dce03537670896457e00b3b9
In STC_ReadLabel, the interaction with the tape device is
synchronous, so there is no need to allocate a task ID for status
monitoring. However, we do need to initialize the output value,
to avoid writing stack garbage on the wire.
(cherry picked from commit f5a80115f8)
(cherry picked from commit 418b2ab56c)
Change-Id: Iaf89d857734bc05a06ebdbd86074b5eadbf100dd
VOTE_Debug and VOTE_XDebug (udebug) both leave a single field
uninitialized if there is no current transaction. This leaks the memory
contents of the ubik server over the wire.
struct ubik_debug
- 4 bytes in member writeTrans
In common code to both RPCs, ensure that writeTrans is always
initialized.
[kaduk@mit.edu: switch to memset]
(cherry picked from commit 7a7c1f751c)
(cherry picked from commit 0ee86cc3f9)
Change-Id: I7fcde3970e6c6d46c8ac1caecd76fa9cb832807c
KAM_ListEntry (kas list) does not initialize its output correctly. It
leaks kaserver memory contents over the wire:
struct kaindex
- up to 64 bytes for member name
- up to 64 bytes for member instance
Initialize the buffer.
[kaduk@mit.edu: move initialization to top of server routine]
(cherry picked from commit b604ee7add)
(cherry picked from commit c912830e9c)
Change-Id: I51229a121cbc4e428169635e8fc46321fb52b813
TC_ScanStatus (backup status) and TC_GetStatus (internal backup status
watcher) do not initialize their output buffers. They leak memory
contents over the wire:
struct tciStatusS
- up to 64 bytes in member taskName (TC_MAXNAMELEN 64)
- up to 64 bytes in member volumeName "
Initialize the buffers.
[kaduk@mit.edu: move initialization to top of server routines]
(cherry picked from commit be0142707c)
(cherry picked from commit 43b3efd4f8)
Change-Id: I03ebbf76a9e22d15b774e04deb0f2750625c3646
TC_ReadLabel (backup readlabel) does not initialize its output buffer
completely. It leaks butc memory contents over the wire:
struct tc_tapeLabel
- up to 32 bytes from member afsname (TC_MAXTAPELEN 32)
- up to 32 bytes from member pname (TC_MAXTAPELEN 32)
Initialize the buffer.
[kaduk@mit.edu: move initialization to the RPC stub]
(cherry picked from commit 52f4d63148)
(cherry picked from commit b7e53b9e97)
Change-Id: I606fcf5afdb176cb4a2ca7bff0a56761b7ae2d48
