openafs/ChangeLog

commit b151451269ec41b5723484596e7dd40f9ab8824a (HEAD -> openafs-stable-1_8_x, origin/openafs-stable-1_8_x)
Author: Andrew Deason <adeason@sinenomine.net>
Date:   Tue Nov 12 20:29:24 2024 -0600

    ptserver: Add xdr_namelist to liboafs_prot.la.sym
    
    Commit 1f5e1ef9e3 (OPENAFS-SA-2024-003: Run xdr_free for retried RPCs)
    added a couple of references to xdr_namelist, which currently causes a
    build failure on AIX:
    
        /bin/sh ../../libtool --quiet --mode=link --tag=CC   xlc_r [...]  -o pts pts.o  ../../src/ptserver/liboafs_prot.la [...]
        ld: 0711-317 ERROR: Undefined symbol: xdr_namelist
        ld: 0711-345 Use the -bloadmap or -bnoquiet option to obtain more information.
        make: 1254-004 The error code from the last command is 8.
    
    To avoid this, add xdr_namelist to liboafs_prot.la.sym.
    
    Reviewed-on: https://gerrit.openafs.org/15954
    Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
    Tested-by: BuildBot <buildbot@rampaginggeek.com>
    Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
    Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
    (cherry picked from commit 4f82b5bd49a3c83c990d64d06cb6389969826208)
    
    Change-Id: I8a7272d1b94bd02295ef63b70a4247a4cf6e70f6
    Reviewed-on: https://gerrit.openafs.org/15955
    Tested-by: BuildBot <buildbot@rampaginggeek.com>
    Reviewed-by: Andrew Deason <adeason@sinenomine.net>
    Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
    Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
    Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

commit c1beae2622fe6fbdda2353a7da2090fc23595617
Author: Benjamin Kaduk <kaduk@mit.edu>
Date:   Fri Nov 8 14:03:53 2024 -0800

    Make OpenAFS 1.8.13
    
    Update version strings for the 1.8.13 release.
    
    Change-Id: Ic7f75226f3ba0f51f17c8e123c8cdbdab3ff6c7f
    Reviewed-on: https://gerrit.openafs.org/15949
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>

commit 7ad61adb706bd53be287f8620ac67720434b3c24
Author: Benjamin Kaduk <kaduk@mit.edu>
Date:   Fri Nov 8 13:57:28 2024 -0800

    Update NEWS for OpenAFS 1.8.13
    
    Change-Id: I8e25f6d4719f403b07a8faad733d858a8872620f
    Reviewed-on: https://gerrit.openafs.org/15948
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>

commit 37e585f0841803cdf3a1f99770034890ba162d7c
Author: Andrew Deason <adeason@sinenomine.net>
Date:   Thu Oct 15 21:07:17 2020 -0500

    OPENAFS-SA-2024-003: xdr: Initialize memory for INOUT args
    
    CVE-2024-10397
    
    Currently, there are a few callers of RPCs that specify some data for
    an INOUT parameter, but do not initialize the memory for that data.
    This can result in the uninitialized memory being sent to the peer
    when the argument is processed as an IN argument. Simply clear the
    relevant data before running the RPC to avoid this.
    
    The relevant RPCs and arguments are:
    
    - For RMTSYS_Pioctl, the 'OutData' argument.
    
    - For BUDB_GetVolumes, the 'volumes' argument.
    -- via DBLookupByVolume -> bcdb_LookupVolume -> ubik_BUDB_GetVolumes
    -- and via bc_Restorer -> bcdb_FindVolumes -> ubik_BUDB_GetVolumes
    
    - For KAA_Authenticate_old / KAA_Authenticate, this can happen with
      the 'answer' argument in ka_Authenticate if KAA_AuthenticateV2 or
      KAA_Authenticate return RXGEN_OPCODE, but the server manages to
      populate oanswer.SeqLen with non-zero.
    
    For all of these, make sure the memory is blanked before running the
    relevant RPC. For ka_Authenticate, reset oanswer.SeqLen to 0 to avoid
    sending any data, but still blank 'answer' and 'answer_old' just to be
    safe.
    
    FIXES 135043
    
    Reviewed-on: https://gerrit.openafs.org/15925
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>
    (cherry picked from commit c4e28c2afe743aa323be57ef3b0faec13027e678)
    
    Change-Id: If44320c1efde98c53eed88099cd978ef89f4c0d8
    Reviewed-on: https://gerrit.openafs.org/15947
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>

commit 4871f8ad2775e97bb85ff7efc33a4ad8d3f6d9d1
Author: Andrew Deason <adeason@sinenomine.net>
Date:   Fri Oct 16 10:55:15 2020 -0500

    OPENAFS-SA-2024-003: sys: Don't over-copy RMTSYS_Pioctl output data
    
    CVE-2024-10397
    
    Here, 'OutData' only has OutData.rmtbulk_len bytes in it. We know that
    OutData.rmtbulk_len is at most data->out_size, but it could be
    smaller. So, only copy OutData.rmtbulk_len bytes, not data->out_size,
    since data->out_size could be more than the number of bytes we have
    allocated in OutData.
    
    FIXES 135043
    
    Reviewed-on: https://gerrit.openafs.org/15924
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>
    (cherry picked from commit f31a79d749abc8e64a8d9ac748bb2b5457875099)
    
    Change-Id: Ic05751d05c7c8862770188131110cc602c9b93b7
    Reviewed-on: https://gerrit.openafs.org/15946
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>

commit 25ad3931d5c03ead625a96e6b626febeb3e20453
Author: Andrew Deason <adeason@sinenomine.net>
Date:   Fri Oct 16 10:52:03 2020 -0500

    OPENAFS-SA-2024-003: Run xdr_free for retried RPCs
    
    CVE-2024-10397
    
    A few areas of code retry the same RPC, like so:
    
        do {
            code = VL_SomeRPC(rxconn, &array_out);
        } while (some_condition);
        xdr_free((xdrproc_t) xdr_foo, &array_out);
    
    Or try a different version/variant of an RPC (e.g.
    VLDB_ListAttributesN2 -> VLDB_ListAttributes).
    
    If the first RPC call causes the output array to be allocated with
    length N, then the subsequent RPC calls may fail if the server
    responds with an array larger than N.
    
    Furthermore, if the subsequent call responds with an array smaller
    than N, then when we xdr_free the array, our length will be smaller
    than the actual number of allocated elements. That results in two
    potential issues:
    
    - We'll fail to free the elements at the end of the array. This is
      only a problem if each element in the array also uses
      dynamically-allocated memory (e.g. each element contains a string or
      another array). Fortunately, there are only a few such structures in
      any of our RPC-L definitions: SysNameList and CredInfos. And neither
      of those are used in such a retry loop, so this isn't a problem.
    
    - We'll give the wrong length to osi_free when freeing the array
      itself. This only matters for KERNEL, and only on some platforms
      (such as Solaris), since the length given to osi_free is ignored
      everywhere else.
    
    To avoid these possible issues, change the relevant retry loops to
    free our xdr-allocated arrays on every iteration of the loop, like
    this:
    
        do {
            xdr_free((xdrproc_t) xdr_foo, &array_out);
            code = VL_SomeRPC(rxconn, &array_out);
        } while (some_condition);
        xdr_free((xdrproc_t) xdr_foo, &array_out);
    
    Or like this:
    
        do {
            code = VL_SomeRPC(rxconn, &array_out);
            xdr_free((xdrproc_t) xdr_foo, &array_out);
        } while (some_condition);
    
    FIXES 135043
    
    Reviewed-on: https://gerrit.openafs.org/15923
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>
    (cherry picked from commit 1f5e1ef9e35f6b5e8693c91199c976d5e030c0d0)
    
    Change-Id: I77ce3a904d502784cbf356e113972dfab838256e
    Reviewed-on: https://gerrit.openafs.org/15945
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>

commit a82212ab20f0635a40c52648a52a1e9eaccc4937
Author: Andrew Deason <adeason@sinenomine.net>
Date:   Thu Oct 15 20:30:14 2020 -0500

    OPENAFS-SA-2024-003: xdr: Ensure correct string length in xdr_string
    
    CVE-2024-10397
    
    Currently, if a caller calls an RPC with a string output argument,
    like so:
    
    {
        char *str = NULL;
        code = RXAFS_SomeCall(&str);
        /* do something with 'str' */
        xdr_free((xdrproc_t) xdr_string, &str);
    }
    
    Normally, xdr_free causes xdr_string to call osi_free, specifying the
    same size that we allocated for the string. However, since we only
    have a char*, the amount of space allocated for the string is not
    recorded separately, and so xdr_string calculates the size of the
    buffer to free by using strlen().
    
    This works for well-formed strings, but if we fail to decode the
    payload of the string, or if our peer gave us a string with a NUL byte
    in the middle of it, then strlen() may be significantly less than the
    actual allocated size. And so in this case, the size given to osi_free
    will be wrong.
    
    The size given to osi_free is ignored in userspace, and for KERNEL on
    many platforms like Linux and DARWIN. However, it is notably not
    ignored for KERNEL on Solaris and some other less supported platforms
    (HPUX, Irix, NetBSD). At least on Solaris, an incorrect size given to
    osi_free can cause a system panic or possibly memory corruption.
    
    To avoid this, change xdr_string during XDR_DECODE to make sure that
    strlen() of the string always reflects the allocated size. If we fail
    to decode the string's payload, replace the payload with non-NUL bytes
    (fill it with 'z', an arbitrary choice). And if we do successfully
    decode the payload, check if the strlen() is wrong (that is, if the
    payload contains NUL '\0' bytes), and fail if so, also filling the
    payload with 'z'. This is only strictly needed in KERNEL on certain
    platforms, but do it everywhere so our behavior is consistent.
    
    FIXES 135043
    
    Reviewed-on: https://gerrit.openafs.org/15922
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>
    (cherry picked from commit 7d0675e6c6a2f3200a3884fbe46b3ef8ef9ffd24)
    
    Change-Id: Ieb8827474a7458ce80176b14ce87f3402aed7a86
    Reviewed-on: https://gerrit.openafs.org/15944
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>

commit 0ff2cd9e0f5656e8327c5fe47935998de3669678
Author: Andrew Deason <adeason@sinenomine.net>
Date:   Thu Oct 15 23:18:53 2020 -0500

    OPENAFS-SA-2024-003: Check sanity on lengths of RPC returned arrays
    
    CVE-2024-10397
    
    Various RPCs return a variable-length array in an OUT argument, but
    are only supposed to return specific sizes. A few instances of this
    include the following (but this is not an exhaustive list):
    
    - AFSVolListOneVolume should only return a single volintInfo.
    
    - PR_NameToID should return the same number of IDs as names given.
    
    - VL_GetAddrsU should return the same number of addresses as the
      'nentries' OUT argument.
    
    Some callers of these RPCs just assume that the server has not
    violated these rules. If the server responds with a nonsensical array
    size, this could cause us to read beyond the end of the array, or
    cause a NULL dereference or other errors.
    
    For example, some callers of VL_GetAddrsU will iterate over 'nentries'
    addresses, even if the 'blkaddrs' OUT argument contains fewer entries.
    Or with AFSVolListOneVolume, some callers assume that at least 1
    volintInfo has been returned; if 0 have been returned, we can try to
    access a NULL array.
    
    To avoid all of this, add various sanity checks on the relevant
    returned lengths of these RPCs. For most cases, if the lengths are not
    sane, return an internal error from the appropriate subsystem (or
    RXGEN_CC_UNMARSHAL if there isn't one). For VL_GetAddrsU, if
    'nentries' is too long, just set it to the length of the returned
    array.
    
    FIXES 135043
    
    Reviewed-on: https://gerrit.openafs.org/15921
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>
    (cherry picked from commit c732715e4ee78ed1e2414c813ae5a4b3574107a0)
    
    Change-Id: I2cfc0723f4c3a2692238fa1e59145aceee17e0d6
    Reviewed-on: https://gerrit.openafs.org/15943
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>

commit d253a52d3b59bd691eae8863ea2f06d99ad18550
Author: Andrew Deason <adeason@sinenomine.net>
Date:   Sun Oct 4 23:04:06 2020 -0500

    OPENAFS-SA-2024-003: xdr: Prevent XDR_DECODE buffer overruns
    
    CVE-2024-10397
    
    When making an RPC call from a client, output arguments that use
    arrays (or array-like objects like strings and opaques) can be
    allocated by XDR, like so:
    
    {
        struct idlist ids;
    
        ids.idlist_val = NULL;
        ids.idlist_len = 0;
        code = PR_NameToID(rxconn, names, &ids);
        /* data inside ids.idlist_val[...] */
        xdr_free((xdrproc_t) xdr_idlist, &ids);
    }
    
    With this approach, during XDR_DECODE, xdr_array() reads in the number
    of array elements from the peer, then allocates enough memory to hold
    that many elements, and then reads in the array elements.
    
    Alternatively, the caller can provide preallocated memory, like so:
    
    {
        struct idlist ids;
        afs_int32 ids_buf[30];
    
        ids.idlist_val = ids_buf;
        ids.idlist_len = 30;
        code = PR_NameToID(rxconn, names, &ids);
        /* data inside ids.idlist_val[...] */
    }
    
    With this approach, during XDR_DECODE, xdr_array() reads in the number
    of array elements from the peer, and then reads in the array elements
    into the supplied buffer. However, in this case, xdr_array() never
    checks that the number of array elements will actually fit into the
    supplied buffer; the _len field provided by the caller is just ignored.
    In this example, if the ptserver responds with 50 elements for the 'ids'
    output argument, xdr_array() will write 50 afs_int32's into
    'ids.idlist_val', going beyond the end of the 30 elements that are
    actually allocated.
    
    It's also possible, and in fact very easy, to use xdr-allocated
    buffers and then reuse them as a preallocated buffer, possibly
    accidentally. For example:
    
    {
        struct idlist ids;
    
        ids.idlist_val = NULL;
        ids.idlist_len = 0;
        while (some_condition) {
            code = PR_NameToID(rxconn, names, &ids);
        }
    }
    
    In this case, the first call to PR_NameToID can cause the buffer for
    'ids' to be allocated by XDR, which will then be reused by the
    subsequent calls to PR_NameToId. Note that this can happen even if the
    first PR_NameToID call fails; the call can be aborted after the output
    array is allocated.
    
    Retrying an RPC in this way is effectively what all ubik_Call*
    codepaths do (including all ubik_* wrappers, e.g. ubik_PR_NameToID).
    Or some callers retry effectively the same RPC when falling back to
    earlier versions (e.g. VL_ListAttributesN2 -> VL_ListAttributesN).
    
    To prevent this for arrays and opaques, change xdr_array (and
    xdr_bytes) to check if the _len field for preallocated buffers is
    large enough, and return failure if it's not.
    
    Also perform the same check for the ka_CBS and ka_BBS structures. These
    are mostly the same as opaques, but they have custom serialization
    functions in src/kauth/kaaux.c. ka_BBS also has two lengths: the actual
    length of bytes, and a 'max' length. ka_CBS isn't used for any RPC
    output arguments, but fix it for consistency.
    
    For strings, the situation is complicated by the fact that callers
    cannot pass in how much space was allocated for the string, since
    callers only provide a char**. So for strings, just refuse to use a
    preallocated buffer at all, and return failure if one is provided.
    
    Note that for some callers using preallocated arrays or strings, the
    described buffer overruns are not possible, since the preallocated
    buffers are larger than the max length specified in the relevant
    RPC-L. For example, afs_DoBulkStat() allocates AFSCBMAX entries for
    the output args for RXAFS_InlineBulkStatus, which is the max length
    specified in the RPC-L, so a buffer overrun is impossible. But since
    it is so easy to allow a buffer overrun, enforce the length checks for
    everyone.
    
    FIXES 135043
    
    Reviewed-on: https://gerrit.openafs.org/15920
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>
    (cherry picked from commit 13413eceed80d106cbed5ffb91c4dfbc8cccf55c)
    
    Change-Id: I1010d2fa309d4a441ebaf285168c2e7e887753b9
    Reviewed-on: https://gerrit.openafs.org/15942
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>

commit c18640c6b98b10cd6f78c63195ff822689cb5348
Author: Andrew Deason <adeason@sinenomine.net>
Date:   Thu Jun 13 15:30:50 2024 -0500

    OPENAFS-SA-2024-003: xdr: Set _len for prealloc'd opaque/array OUT args
    
    CVE-2024-10397
    
    Currently, a few RPCs with arrays or opaque OUT arguments are called
    with preallocated memory for the arg, but also provide a _len of 0 (or
    an uninitialized _len). This makes it impossible for the xdr routine to
    tell whether we have allocated enough space to actually hold the
    response from the server.
    
    To help this situation, either specify an appropriate _len for the
    preallocated value (cm_IoctlGetACL, fsprobe_LWP), or don't provide a
    preallocated buffer at all and let xdr allocate a buffer for us
    (PGetAcl).
    
    Note that this commit doesn't change xdr to actually check the value of
    the given _len; but now a future commit can do so without breaking
    callers.
    
    FIXES 135043
    
    Reviewed-on: https://gerrit.openafs.org/15919
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>
    (cherry picked from commit b2b1110ddd9e19670dbc6a3217dc2a74af432f82)
    
    Change-Id: Ibdee49b79da1476c4e606bcad5fb3d08eb259ad7
    Reviewed-on: https://gerrit.openafs.org/15941
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>

commit 40440c3eb628ff1772588bdc99d7496292097bbd
Author: Andrew Deason <adeason@sinenomine.net>
Date:   Thu Jun 13 15:28:38 2024 -0500

    OPENAFS-SA-2024-003: xdr: Avoid prealloc'd string OUT args
    
    CVE-2024-10397
    
    Currently, several callers call RPCs with string OUT arguments, and
    provide preallocated memory for those arguments. This can easily allow a
    response from the server to overrun the allocated buffer, stomping over
    stack or heap memory.
    
    We could simply make our preallocated buffers larger than the maximum
    size that the RPC allows, but relying on that is error prone, and
    there's no way for XDR to check if a string buffer is large enough.
    
    Instead, to make sure we don't overrun a given preallocated buffer,
    avoid giving a preallocated buffer to such RPCs, and let XDR allocate
    the memory for us.
    
    Specifically, this commit changes several callers to
    RXAFS_GetVolumeStatus(), and one caller of BOZO_GetInstanceParm(), to
    avoid passing in a preallocated string buffer.
    
    All other callers of RPCs with string OUT args already let XDR allocate
    the buffers for them.
    
    FIXES 135043
    
    Reviewed-on: https://gerrit.openafs.org/15918
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>
    (cherry picked from commit 00a1b266af51a828a022c23e7bb006a39740eaad)
    
    Change-Id: Ib174d008eaf1fd10d42702bcdb607e45b26acf58
    Reviewed-on: https://gerrit.openafs.org/15940
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>

commit fec84e347768080e4370e5aeb05886bfe19ae54b
Author: Michael Meffie <mmeffie@sinenomine.net>
Date:   Fri Mar 10 17:51:17 2023 -0500

    xdr: Avoid xdr_string maxsize check when freeing
    
    The maxsize argument in xdr_string() is garbage when called by
    xdr_free(), since xdr_free() only passes the XDR handle and the xdr
    string to be freed. Sometimes the size check fails and xdr_string()
    returns early, without freeing the string and without setting the object
    pointer to NULL.
    
    Usually this just results in leaking the string's memory. But since
    commit 9ae5b599c7 (bos: Let xdr allocate rpc output strings), many
    callers in bos.c rely on xdr_free(xdr_string) to set the given string
    to NULL; if this doesn't happen, subsequent calls to BOZO_ RPCs can
    corrupt memory, often causing the 'bos' process to segfault.
    
    We only need the maxsize check when encoding or decoding, so avoid
    accessing the maxsize agument when the op mode is XDR_FREE.
    
    In general, xdr_free() can only safely be used on xdr 2-argument xdr
    functions, so must be avoided when freeing xdr opaque, byte, and union
    types.
    
    This change makes it safe to use xdr_free() to free xdr strings, but in
    the future, we should provide a typesafe and less fragile function for
    freeing xdr strings returned from RPCs.  Currently, xdr_free(xdr_string)
    is only called by the bos client and the tests.
    
    Reviewed-on: https://gerrit.openafs.org/15343
    Tested-by: BuildBot <buildbot@rampaginggeek.com>
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    (cherry picked from commit bbb1e8adfed6804ac6fbae0a073dc6927096e16a)
    
    Change-Id: I1f190d28acab5fa1621919f283571fcacb495ce4
    Reviewed-on: https://gerrit.openafs.org/15939
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>

commit 21941c0ab2d28fa3a074f46e4d448d518a7c1b8a
Author: Andrew Deason <adeason@sinenomine.net>
Date:   Tue Nov 5 23:40:24 2024 -0600

    OPENAFS-SA-2024-002: Avoid uninitialized memory when parsing ACLs
    
    CVE-2024-10396
    
    Several places in the tree parse ACLs using sscanf() calls that look
    similar to this:
    
        sscanf(str, "%d dfs:%d %s", &nplus, &dfs, cell);
        sscanf(str, "%100s %d", tname, &trights);
    
    Some callers check whether the scanf() returns negative or 0, but some
    callers do not check the return code at all. If only some of the fields
    are present in the sscanf()'d string (because, for instance, the ACL is
    malformed), some of the arguments are left alone, and may be set to
    garbage if the relevant variable was never initialized.
    
    If the parsed ACL is copied to another ACL, this can result in the
    copied ACL containing uninitialized memory.
    
    To avoid this, make sure all of the variables passed to sscanf() and
    similar calls are initialized before parsing. This commit does not
    guarantee that the results make sense, but at least the results do not
    contain uninitialized memory.
    
    Reviewed-on: https://gerrit.openafs.org/15917
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>
    (cherry picked from commit ac602a0a5624b0f0ab04df86f618d09f2a4ad063)
    
    Change-Id: I00245c12993683eb3b58d51cf77742f758bac120
    Reviewed-on: https://gerrit.openafs.org/15938
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>

commit a9ede52673b8c8abbfc2577ac6987a8a5686206f
Author: Benjamin Kaduk <kaduk@mit.edu>
Date:   Mon Nov 4 20:50:50 2024 -0800

    OPENAFS-SA-2024-002: make VIOCGETAL consumers stay within string bounds
    
    CVE-2024-10396
    
    After the preceding commits, the data returned by the VIOCGETAL
    pioctl (a RXAFS_FetchAcl wrapper) will safely be NUL-terminated.
    However, the callers that attempt to parse the ACL string make
    assumptions that the returned data will be properly formatted,
    and implement a "skip to next line" functionality (under various
    names) that blindly increments a char* until it finds a newline
    character, which can read past the end of even a properly
    NUL-terminated string if there is not a newline where one is
    expected.
    
    Adjust the various "skip to next line" functionality to keep
    the current string pointer at the trailing NUL if the end of the
    string is reached while searching for a newline.
    
    Reviewed-on: https://gerrit.openafs.org/15916
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>
    (cherry picked from commit a4ecb050540528a1bff840ff08d21f99e6ef3fbf)
    
    Change-Id: Id2d8c0164cfaa7d03a9e37b29ff58b88cf815483
    Reviewed-on: https://gerrit.openafs.org/15937
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>

commit a96a3160f5425125588f39f5ac612df3ef9b9a8a
Author: Benjamin Kaduk <kaduk@mit.edu>
Date:   Mon Nov 4 20:50:50 2024 -0800

    OPENAFS-SA-2024-002: verify FetchACL returned only a string
    
    CVE-2024-10396
    
    Supplement the previous commit by additionally verifying that
    the returned ACL string occupies the entire XDR opaque, rejecting
    any values returned that have an internal NUL prior to the end
    of the opaque.
    
    Reviewed-on: https://gerrit.openafs.org/15915
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>
    (cherry picked from commit 7e13414e8ea995d438cde3e60988225f3ab4cbcd)
    
    Change-Id: I107f89e3d8a5c3c5cd67f6296742bfca7cace0e1
    Reviewed-on: https://gerrit.openafs.org/15936
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>

commit 64068705b15661a8d4e0b9f9f2ad4aec34ed51a7
Author: Benjamin Kaduk <kaduk@mit.edu>
Date:   Mon Nov 4 20:33:16 2024 -0800

    OPENAFS-SA-2024-002: verify FetchACL returned a valid string
    
    CVE-2024-10396
    
    Analogously to how a call to RXAFS_StoreACL() with a malformed
    ACL string can cause a fileserver to perform invalid memory operations,
    a malformed ACL string returned in response to a call to RXAFS_FetchACL()
    can cause a client to perform invalid memory operations.
    
    Modify all the in-tree callers of the RPC to verify that the ACL
    data, which is conveyed as an XDR 'opaque' but whose contents
    are actually expected to be a string, is a valid C string.  If
    a zero-length opaque or one without a trailing NUL is received,
    treat that as an error response from the fileserver rather than
    returning success.
    
    The Unix cache manager's pioctl handler already has logic to cope with a
    zero-length reply by emitting a single NUL byte to userspace.  This
    special-casing seems to have been in place from the original IBM import,
    though it does so by confusingly "skipping over" a NUL byte already put
    in place.  For historical compatibility, preserve that behavior rather
    than treating the zero-length reply as an error as we do for the other
    callers.  It seems likely that this location should treat a zero-length
    reply as an error just as the other call sites do, but that can be done
    as a later change.
    
    Reviewed-on: https://gerrit.openafs.org/15914
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>
    (cherry picked from commit 0b1ccb0dbc3b7673558eceff3d672971f5bb0197)
    
    Change-Id: Ifbce762d76641f08b5fc5e79b4c8dad07c1a135a
    Reviewed-on: https://gerrit.openafs.org/15935
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>

commit bb01d76a2095baa65880bdc5d504e7a198958265
Author: Andrew Deason <adeason@sinenomine.net>
Date:   Wed Aug 21 00:41:49 2024 -0500

    OPENAFS-SA-2024-002: viced: Avoid unchecked ACL in StoreACL audit log
    
    CVE-2024-10396
    
    Currently in SRXAFS_StoreACL, if CallPreamble() or check_acl() fail, we
    will jump to Bad_StoreACL, which will pass the ACL string from the
    client to osi_auditU. Since check_acl() hasn't yet checked if the given
    ACL contains a NUL byte, the ACL may be an unterminated string. If
    auditing is enabled, this can cause garbage to be logged to the audit
    log, or cause the fileserver to crash.
    
    To avoid this, set 'rawACL' to NULL at first, only setting it to the
    actual ACL string after check_acl() has succeeded. This ensures that all
    code accessing 'rawACL' is guaranteed to be using a terminated string.
    
    This may mean that we pass a NULL AUD_ACL to osi_auditU. Our auditing
    code explicitly checks for and handles handles NULL strings, so this is
    fine.
    
    FIXES 135445
    
    Reviewed-on: https://gerrit.openafs.org/15913
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>
    (cherry picked from commit c9eae1e8b26144063e5d1db23d47ee82c4b9ef3a)
    
    Change-Id: Ieda6f910d875c4b5179011e5e93e5694d3f4ce47
    Reviewed-on: https://gerrit.openafs.org/15934
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>

commit ee020f7cba7d82bc3d4b468210b5052af53c5db5
Author: Andrew Deason <adeason@sinenomine.net>
Date:   Wed Aug 21 00:29:34 2024 -0500

    OPENAFS-SA-2024-002: viced: Introduce 'rawACL' in StoreACL
    
    CVE-2024-10396
    
    Change our StoreACL implementation to refer to the 'AccessList' argument
    via a new local variable called 'rawACL'. This makes it clearer to
    users that the data is a string, and makes it easier for future commits
    to make sure we don't access the 'AccessList' argument in certain
    situations.
    
    Update almost all users in StoreACL to refer to 'rawACL' instead of
    'AccessList'. Change the name of 'AccessList' to 'uncheckedACL' to make
    sure we don't miss any users. Update our check_acl() call to use
    'uncheckedACL' (and not 'rawACL'), because it must use an AFSOpaque to
    check the ACL.
    
    Change RXStore_AccessList() and printableACL() to accept a plain char*
    instead of a struct AFSOpaque.
    
    This commit should not incur any noticeable behavior change. Technically
    printableACL() is changed to run strlen() on the given string, but this
    should not cause any noticeable change in behavior:
    
    This change could cause printableACL() to process less of the string
    than before, if the string contains a NUL byte before the end of the
    AFSOpaque buffer. But this doesn't matter, since the all of our code
    after this treats the ACL as a plain string, and so doesn't look at any
    data beyond the first NUL. It's not possible for printableACL() to
    process more data than before, because check_acl() has already checked
    that the ACL string contains a NUL byte, so we must process
    AFSOpaque_len bytes or fewer.
    
    FIXES 135445
    
    Reviewed-on: https://gerrit.openafs.org/15912
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>
    (cherry picked from commit eb8b93a971c6293cdfbf8cd3d9a6351a8cb76f81)
    
    [1.8: printableACL() does not exist in this branch.]
    
    Change-Id: I65b518acab26be0bb1854c29e46c90e5fee52d41
    Reviewed-on: https://gerrit.openafs.org/15933
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>

commit d66caf8c04878724001839317637445708edef2c
Author: Andrew Deason <adeason@sinenomine.net>
Date:   Tue Sep 19 15:55:42 2023 -0500

    OPENAFS-SA-2024-002: acl: Error on missing newlines when parsing ACL
    
    CVE-2024-10396
    
    In acl_Internalize_pr(), each line in an ACL granting rights (positive
    or negative) is sscanf()'d with "%63s\t%d\n", and then we try to
    advance 'nextc' beyond the next newline character.
    
    However, sscanf()'ing "%63s\t%d\n" does not guarantee that there is a
    newline in the given string. Whitespace characters in sscanf() are not
    matched exactly, and may match any amount of whitespace (including
    none at all). For example, a string like "foo 4" may be parsed by
    sscanf(), but does not contain any newlines.
    
    If this happens, strchr(nextc, '\n') will return NULL, and we'll
    advance 'nextc' to 0x1, causing a segfault when we next try to
    dereference 'nextc'.
    
    To avoid this, check if 'nextc' is NULL after the strchr() call, and
    return an error if so.
    
    FIXES 135445
    
    Reviewed-on: https://gerrit.openafs.org/15911
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>
    (cherry picked from commit 96ab2c6f8a614d597a523b45871c5f64a50a7040)
    
    Change-Id: I666dfb2c401410865c1f98d9db1b342b52c8f628
    Reviewed-on: https://gerrit.openafs.org/15932
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>

commit 1e6e813188ecce62eb7af19385d911f63469bdb6
Author: Andrew Deason <adeason@sinenomine.net>
Date:   Tue Sep 19 15:44:08 2023 -0500

    OPENAFS-SA-2024-002: acl: Do not parse beyond end of ACL
    
    CVE-2024-10396
    
    The early parsing code in acl_Internalize_pr() tries to advance
    'nextc' to go beyond the first two newlines in the given ACL string.
    But if the given ACL string has no newlines, or only 1 newline, then
    'nextc' will point beyond the end of the ACL string, potentially
    pointing to garbage.
    
    Intuitively, it may look like the ACL string must contain at least 2
    newlines because we have sscanf()'d the string with "%d\n%\d".
    However, whitespace characters in sscanf() are not matched exactly
    like non-whitespace characters are; a sequence of whitespace
    characters matches any amount of whitespace (including none). So, a
    string like "1 2" will be parsed by "%d\n%d\n", but will not contain
    any newline characters.
    
    Usually this should result in a parse error from acl_Internalize_pr(),
    but if the garbage happens to parse successfully, this could result in
    unrelated memory getting stored to the ACL.
    
    To fix this, don't advance 'nextc' if we're already at the end of the
    ACL string.
    
    FIXES 135445
    
    Reviewed-on: https://gerrit.openafs.org/15910
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>
    (cherry picked from commit 35d218c1d17973c1412ea5dff1e23d9aae50c4c7)
    
    Change-Id: I7a7d136676e548adba5fa8d0003b5f8342332a86
    Reviewed-on: https://gerrit.openafs.org/15931
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>

commit a07e50726df09c49dfe7b953c3e49eb98f310c09
Author: Andrew Deason <adeason@sinenomine.net>
Date:   Mon Sep 18 16:14:07 2023 -0500

    OPENAFS-SA-2024-002: viced: Free ACL on acl_Internalize_pr error
    
    CVE-2024-10396
    
    Currently, we don't free 'newACL' if acl_Internalize_pr() fails. If
    acl_Internalize_pr() has already allocated 'newACL', then the memory
    associated with newACL will be leaked. This can happen if parsing the
    given ACL fails at any point after successfully parsing the first
    couple of lines in the ACL.
    
    Change acl_FreeACL() to make freeing a NULL acl a no-op, to make it
    easier to make sure the acl has been freed.
    
    FIXES 135445
    
    Reviewed-on: https://gerrit.openafs.org/15909
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>
    (cherry picked from commit f4dfc2d7183f126bc4a45b5cabc78c3de020925f)
    
    Change-Id: If1554aa899542761ec6e6611394f2ee4f9379f22
    Reviewed-on: https://gerrit.openafs.org/15930
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>

commit f74f960a18f559e683d6a1f5104e43c3ca93ecb8
Author: Andrew Deason <adeason@sinenomine.net>
Date:   Mon Sep 18 16:13:57 2023 -0500

    OPENAFS-SA-2024-002: viced: Refuse ACLs without '\0' in SRXAFS_StoreACL
    
    CVE-2024-10396
    
    Currently, the fileserver treats the ACL given in RXAFS_StoreACL as a
    string, even though it is technically an AFSOpaque and could be not
    NUL-terminated.
    
    We give the ACL opaque/string to acl_Internalize_pr() to parse, which
    will run off the end of the allocated buffer if the given ACL does not
    contain a '\0' character. Usually this will result in a parse error
    since we'll encounter garbage, but if the partially-garbage ACL
    happens to parse successfully, some uninitialized data could make it
    into the stored ACL.
    
    In addition, if the given ACL is an opaque of length 0, we'll still
    give the opaque pointer to acl_Internalize_pr(). In this case, the
    pointer will point to &memZero, which happens to contain a NUL byte,
    and so is treated like an empty string (which is not a valid ACL). But
    the fact that this causes no problems is somewhat a coincidence, and
    so should also be avoided.
    
    To avoid both of these situations, just check if the given ACL string
    contains a NUL byte. If it doesn't, or if it has length 0, refuse to
    look at it and abort the call with EINVAL.
    
    FIXES 135445
    
    Reviewed-on: https://gerrit.openafs.org/15908
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>
    (cherry picked from commit e15decb318797f1d471588dc669c3e3b26f1b8b3)
    
    Change-Id: I0f447310db5a988b21e19bb5158bb564d4ea3d94
    Reviewed-on: https://gerrit.openafs.org/15929
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>

commit 57b655e4837d8660ebcc25d95efb09118adaff07
Author: Andrew Deason <adeason@sinenomine.net>
Date:   Fri Jan 10 12:40:15 2020 -0600

    OPENAFS-SA-2024-001: afs: Throttle PAG creation in afs_genpag()
    
    CVE-2024-10394
    
    Currently, we only throttle PAG creation in afs_setpag(). But there
    are several callers that call setpag() directly, not via afs_setpag;
    notably _settok_setParentPag in afs_pioctl.c. When setpag() is called
    with a PAG value of -1, it generates a new PAG internally without any
    throttling. So, those callers effectively bypass the PAG throttling
    mechanism, which allows a calling user to create PAGs without any
    delay.
    
    To avoid this, move our afs_pag_wait call from afs_setpag() to
    afs_genpag(), which all code uses to generate a new PAG value. This
    ensures that PAG creation is always throttled for unprivileged users.
    
    FIXES 135062
    
    Reviewed-on: https://gerrit.openafs.org/15907
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>
    (cherry picked from commit 0358648dbed7656e7bda30f6f0ea6e8e01bf6527)
    
    Change-Id: I7f8f475a913c6f62ca2c7a6fb00239e51a8a8c62
    Reviewed-on: https://gerrit.openafs.org/15928
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>

commit 20c22347b41eea2ebbdc0ab15f16c822af44df51
Author: Andrew Deason <adeason@sinenomine.net>
Date:   Fri Jan 10 12:01:50 2020 -0600

    OPENAFS-SA-2024-001: afs: Introduce afs_genpag()
    
    CVE-2024-10394
    
    Currently, several areas in the code call genpag() to generate a new
    PAG id, but the signature of genpag() is very limited. To allow for
    the code in genpag() to return errors and to examine the calling
    user's credentials, introduce a new function, afs_genpag(), that does
    the same thing as genpag(), but accepts creds and allows errors to be
    returned.
    
    Convert all existing callers to use afs_genpag() and to handle any
    errors, though no errors are ever returned in this commit on its own.
    
    To ensure there are no old callers of genpag() left around, change the
    existing genpag() to be called genpagval(), and declare it static.
    
    FIXES 135062
    
    Reviewed-on: https://gerrit.openafs.org/14090
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>
    (cherry picked from commit f701f704c7bc93cf5fd7cffaaa043cef6a99e77f)
    
    Change-Id: I675d6cb111ca74638a3b856a3c989dcb2fe6d534
    Reviewed-on: https://gerrit.openafs.org/15927
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>