VirtualBox

Ignore:
Timestamp:
Jan 3, 2024 5:24:32 PM (14 months ago)
Author:
vboxsync
svn:sync-xref-src-repo-rev:
160930
Message:

Guest Properties/VBoxService: Deal with long(er) user name / domain name combinations on Windows guests. See comments for details. bugref:10575

Location:
trunk/src/VBox/Additions/common/VBoxService
Files:
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/src/VBox/Additions/common/VBoxService/VBoxServicePropCache.cpp

    r98103 r102753  
    3636
    3737#include <VBox/VBoxGuestLib.h>
     38#include <VBox/HostServices/GuestPropertySvc.h> /* For GUEST_PROP_MAX_VALUE_LEN */
    3839#include "VBoxServiceInternal.h"
    3940#include "VBoxServiceUtils.h"
    … …  
    214215 *
    215216 * @returns VBox status code.
    216  *
     217 * @retval  VERR_BUFFER_OVERFLOW if the property name or value exceeds the limit.
    217218 * @param   pCache          The property cache.
    218219 * @param   pszName         The property name.
    … …  
    227228
    228229    Assert(pCache->uClientID);
     230
     231    if (RTStrNLen(pszName, GUEST_PROP_MAX_NAME_LEN) > GUEST_PROP_MAX_NAME_LEN - 1 /* Terminator */)
     232        return VERR_BUFFER_OVERFLOW;
    229233
    230234    /*
    … …  
    240244        if (!pszValue)
    241245            return VERR_NO_STR_MEMORY;
     246        if (RTStrNLen(pszValue, GUEST_PROP_MAX_VALUE_LEN) > GUEST_PROP_MAX_VALUE_LEN - 1 /* Terminator */)
     247        {
     248            RTStrFree(pszValue);
     249            return VERR_BUFFER_OVERFLOW;
     250        }
    242251    }
    243252

  • trunk/src/VBox/Additions/common/VBoxService/VBoxServiceVMInfo-win.cpp

    r99828 r102753  
    3737#include <wtsapi32.h>        /* For WTS* calls. */
    3838#include <psapi.h>           /* EnumProcesses. */
     39#include <sddl.h>            /* For ConvertSidToStringSidW. */
    3940#include <Ntsecapi.h>        /* Needed for process security information. */
    4041
    … …  
    941942
    942943
     944/**
     945 * Destroys an allocated SID.
     946 *
     947 * @param   pSid                SID to dsetroy. The pointer will be invalid on return.
     948 */
     949static void vgsvcVMInfoWinUserSidDestroy(PSID pSid)
     950{
     951    RTMemFree(pSid);
     952    pSid = NULL;
     953}
     954
     955
     956/**
     957 * Looks up and returns a SID for a given user.
     958 *
     959 * @returns VBox status code.
     960 * @param   pszUser             User to look up a SID for.
     961 * @param   ppSid               Where to return the allocated SID.
     962 *                              Must be destroyed with vgsvcVMInfoWinUserSidDestroy().
     963 */
     964static int vgsvcVMInfoWinUserSidLookup(const char *pszUser, PSID *ppSid)
     965{
     966    RTUTF16 *pwszUser = NULL;
     967    size_t cwUser = 0;
     968    int rc = RTStrToUtf16Ex(pszUser, RTSTR_MAX, &pwszUser, 0, &cwUser);
     969    AssertRCReturn(rc, rc);
     970
     971    PSID pSid = NULL;
     972    DWORD cbSid = 0;
     973    DWORD cbDomain = 0;
     974    SID_NAME_USE enmSidUse = SidTypeUser;
     975    if (!LookupAccountNameW(NULL, pwszUser, pSid, &cbSid, NULL, &cbDomain, &enmSidUse))
     976    {
     977        DWORD const dwErr = GetLastError();
     978        if (dwErr == ERROR_INSUFFICIENT_BUFFER)
     979        {
     980            pSid = (PSID)RTMemAllocZ(cbSid);
     981            if (pSid)
     982            {
     983                PRTUTF16 pwszDomain = (PRTUTF16)RTMemAllocZ(cbDomain * sizeof(RTUTF16));
     984                if (pwszDomain)
     985                {
     986                    if (LookupAccountNameW(NULL, pwszUser, pSid, &cbSid, pwszDomain, &cbDomain, &enmSidUse))
     987                    {
     988                        if (IsValidSid(pSid))
     989                            *ppSid = pSid;
     990                        else
     991                            rc = VERR_INVALID_PARAMETER;
     992                    }
     993                    else
     994                        rc = RTErrConvertFromWin32(GetLastError());
     995                }
     996                else
     997                    rc = VERR_NO_MEMORY;
     998            }
     999            else
     1000                rc = VERR_NO_MEMORY;
     1001        }
     1002        else
     1003            rc = RTErrConvertFromWin32(dwErr);
     1004    }
     1005    else
     1006        rc = RTErrConvertFromWin32(GetLastError());
     1007
     1008    if (RT_FAILURE(rc))
     1009        vgsvcVMInfoWinUserSidDestroy(pSid);
     1010
     1011    return rc;
     1012}
     1013
     1014
     1015/**
     1016 * Fallback function in case writing the user name failed within vgsvcVMInfoWinUserUpdateF().
     1017 *
     1018 * This uses the following approach:
     1019 *   - only use the user name as part of the property name from now on
     1020 *   - write the domain name into a separate "Domain" property
     1021 *   - write the (full) SID into a  separate "SID" property
     1022 *
     1023 * @returns VBox status code.
     1024 * @retval  VERR_BUFFER_OVERFLOW if the final property name length exceeds the maximum supported length.
     1025 * @param   pCache                  Pointer to guest property cache to update user in.
     1026 * @param   pszUser                 Name of guest user to update.
     1027 * @param   pszDomain               Domain of guest user to update. Optional.
     1028 * @param   pszKey                  Key name of guest property to update.
     1029 * @param   pszValueFormat          Guest property value to set. Pass NULL for deleting
     1030 *                                  the property.
     1031 * @param   va                      Variable arguments.
     1032 */
     1033int vgsvcVMInfoWinUserUpdateFallbackV(PVBOXSERVICEVEPROPCACHE pCache, const char *pszUser, const char *pszDomain,
     1034                                      WCHAR *pwszSid, const char *pszKey, const char *pszValueFormat, va_list va)
     1035{
     1036    int rc = VGSvcUserUpdateF(pCache, pszUser, NULL /* pszDomain */, "Domain", pszDomain);
     1037    if (RT_SUCCESS(rc))
     1038        rc = VGSvcUserUpdateF(pCache, pszUser, NULL /* pszDomain */, "SID", "%ls", pwszSid);
     1039
     1040    /* Last but no least, write the actual guest property value we initially were called for.
     1041     * We always do this, no matter of what the outcome from above was. */
     1042    int rc2 = VGSvcUserUpdateV(pCache, pszUser, NULL /* pszDomain */, pszKey, pszValueFormat, va);
     1043    if (RT_SUCCESS(rc))
     1044        rc2 = rc;
     1045
     1046    return rc;
     1047}
     1048
     1049
     1050/**
     1051 * Wrapper function for VGSvcUserUpdateF() that deals with too long guest property names.
     1052 *
     1053 * @return  VBox status code.
     1054 * @retval  VERR_BUFFER_OVERFLOW if the final property name length exceeds the maximum supported length.
     1055 * @param   pCache                  Pointer to guest property cache to update user in.
     1056 * @param   pszUser                 Name of guest user to update.
     1057 * @param   pszDomain               Domain of guest user to update. Optional.
     1058 * @param   pszKey                  Key name of guest property to update.
     1059 * @param   pszValueFormat          Guest property value to set. Pass NULL for deleting
     1060 *                                  the property.
     1061 * @param   ...                     Variable arguments.
     1062 */
     1063int vgsvcVMInfoWinUserUpdateF(PVBOXSERVICEVEPROPCACHE pCache, const char *pszUser, const char *pszDomain,
     1064                              const char *pszKey, const char *pszValueFormat, ...)
     1065{
     1066    va_list va;
     1067    va_start(va, pszValueFormat);
     1068
     1069    /* First, try to write stuff as we always did, to not break older VBox versions. */
     1070    int rc = VGSvcUserUpdateV(pCache, pszUser, pszDomain, pszKey, pszValueFormat, va);
     1071    if (rc == VERR_BUFFER_OVERFLOW)
     1072    {
     1073        /**
     1074         * If the constructed property name was too long, we have to be a little more creative here:
     1075         *
     1076         *   - only use the user name as part of the property name from now on
     1077         *   - write the domain name into a separate "Domain" property
     1078         *   - write the (full) SID into a  separate "SID" property
     1079         */
     1080        PSID pSid;
     1081        rc = vgsvcVMInfoWinUserSidLookup(pszUser, &pSid); /** @todo Shall we cache this? */
     1082        if (RT_SUCCESS(rc))
     1083        {
     1084            WCHAR *pwszSid;
     1085            if (ConvertSidToStringSidW(pSid, &pwszSid)) /** @todo Ditto. */
     1086            {
     1087                rc = vgsvcVMInfoWinUserUpdateFallbackV(pCache, pszUser, pszDomain, pwszSid, pszKey, pszValueFormat, va);
     1088                if (RT_FAILURE(rc))
     1089                {
     1090                    /**
     1091                     * If using the sole user name as a property name still is too long or something else failed,
     1092                     * at least try to look up the user's RID (relative identifier). Note that the RID always is bound to the
     1093                     * to the authority that issued the SID.
     1094                     */
     1095                    int const cSubAuth = *GetSidSubAuthorityCount(pSid);
     1096                    if (cSubAuth > 1)
     1097                    {
     1098                        DWORD const dwUserRid = *GetSidSubAuthority(pSid, cSubAuth - 1);
     1099                        char  szUserRid[16 + 1];
     1100                        if (RTStrPrintf2(szUserRid, sizeof(szUserRid), "%ld", dwUserRid) > 0)
     1101                            rc = vgsvcVMInfoWinUserUpdateFallbackV(pCache, szUserRid, pszDomain, pwszSid, pszKey,
     1102                                                                   pszValueFormat, va);
     1103                        else
     1104                            rc = VERR_BUFFER_OVERFLOW;
     1105                    }
     1106                    /* else not much else we can do then. */
     1107                }
     1108
     1109                LocalFree(pwszSid);
     1110                pwszSid = NULL;
     1111            }
     1112            else
     1113                rc = RTErrConvertFromWin32(GetLastError());
     1114
     1115            vgsvcVMInfoWinUserSidDestroy(pSid);
     1116        }
     1117        else
     1118            VGSvcError("Looking up SID for user '%s' (domain '%s') failed with %Rrc\n", pszUser, pszDomain, rc);
     1119    }
     1120    va_end(va);
     1121    return rc;
     1122}
     1123
     1124
    9431125static int vgsvcVMInfoWinWriteLastInput(PVBOXSERVICEVEPROPCACHE pCache, const char *pszUser, const char *pszDomain)
    9441126{
    … …  
    9811163                              : VBoxGuestUserState_Idle;
    9821164
    983                     rc = VGSvcUserUpdateF(pCache, pszUser, pszDomain, "UsageState",
    984                                           userState == VBoxGuestUserState_InUse ? "InUse" : "Idle");
    985 
     1165                    rc = vgsvcVMInfoWinUserUpdateF(pCache, pszUser, pszDomain, "UsageState",
     1166                                                   userState == VBoxGuestUserState_InUse ? "InUse" : "Idle");
    9861167                    /*
    9871168                     * Note: vboxServiceUserUpdateF can return VINF_NO_CHANGE in case there wasn't anything
    … …  
    9961177                        /* Also write the user's current idle time, if there is any. */
    9971178                        if (userState == VBoxGuestUserState_Idle)
    998                             rc = vgsvcUserUpdateF(pCache, pszUser, pszDomain, "IdleTimeMs", "%RU32", ipcReply.cSecSinceLastInput);
     1179                            rc = vgsvcVMInfoWinUserUpdateF(pCache, pszUser, pszDomain, "IdleTimeMs", "%RU32",
     1180                                                           ipcReply.cSecSinceLastInput);
    9991181                        else
    1000                             rc = vgsvcUserUpdateF(pCache, pszUser, pszDomain, "IdleTimeMs", NULL /* Delete property */);
    1001 
     1182                            rc = vgsvcVMInfoWinUserUpdateF(pCache, pszUser, pszDomain, "IdleTimeMs",
     1183                                                           NULL /* Delete property */);
    10021184                        if (RT_SUCCESS(rc))
    10031185#endif
    … …  
    10261208
    10271209                    /* Overwrite rc from above. */
    1028                     rc = VGSvcUserUpdateF(pCache, pszUser, pszDomain, "UsageState", "Idle");
     1210                    rc = vgsvcVMInfoWinUserUpdateF(pCache, pszUser, pszDomain, "UsageState", "Idle");
    10291211
    10301212                    fReportToHost = rc == VINF_SUCCESS;

  • trunk/src/VBox/Additions/common/VBoxService/VBoxServiceVMInfo.cpp

    r102716 r102753  
    108108#include <VBox/version.h>
    109109#include <VBox/VBoxGuestLib.h>
     110#include <VBox/HostServices/GuestPropertySvc.h> /* For GUEST_PROP_MAX_VALUE_LEN */
    110111#include "VBoxServiceInternal.h"
    111112#include "VBoxServiceUtils.h"
    … …  
    145146static const char              *g_pszPropCacheValNetCount = "/VirtualBox/GuestInfo/Net/Count";
    146147/** A guest user's guest property root key. */
    147 static const char              *g_pszPropCacheValUser = "/VirtualBox/GuestInfo/User/";
     148static const char              *g_pszPropCacheKeyUser = "/VirtualBox/GuestInfo/User";
    148149/** The VM session ID. Changes whenever the VM is restored or reset. */
    149150static uint64_t                 g_idVMInfoSession;
    … …  
    419420 * Updates a per-guest user guest property inside the given property cache.
    420421 *
    421  * @return  IPRT status code.
     422 * @return  VBox status code.
     423 * @retval  VERR_BUFFER_OVERFLOW if the final property name length exceeds the maximum supported length.
    422424 * @param   pCache                  Pointer to guest property cache to update user in.
    423425 * @param   pszUser                 Name of guest user to update.
    … …  
    436438    /* pszValueFormat is optional. */
    437439
     440    /** Historically we limit guest property names to 64 characters (see GUEST_PROP_MAX_NAME_LEN, including terminator).
     441     *  So we need to make sure the stuff we want to write as a value fits into that space. See bugref{10575}. */
     442
     443    /* Try to write things the legacy way first. */
     444    char szName[GUEST_PROP_MAX_NAME_LEN];
     445    AssertCompile(GUEST_PROP_MAX_NAME_LEN == 64); /* Can we improve stuff once we (ever) raise this limit? */
     446    ssize_t const cchVal = pszDomain
     447                         ? RTStrPrintf2(szName, sizeof(szName), "%s/%s@%s/%s", g_pszPropCacheKeyUser, pszUser, pszDomain, pszKey)
     448                         : RTStrPrintf2(szName, sizeof(szName), "%s/%s/%s",    g_pszPropCacheKeyUser, pszUser, pszKey);
     449
     450    /* Did we exceed the length limit? Tell the caller to try again with some more sane values. */
     451    if (cchVal < 0)
     452        return VERR_BUFFER_OVERFLOW;
     453
    438454    int rc = VINF_SUCCESS;
    439455
    440     /** @todo r=bird: This is just asking for trouble with long names, esp. when
    441      * domain names are included.  The max property name limit is 64 characters,
    442      * so it is really easy to run into the limit here. E.g.
    443      *      "/VirtualBox/GuestInfo/User/Administrator@DGV-W10X64-TEST/UsageState"
    444      * is too long and will be rejected by the host servce (assert in strict builds)
    445      * as Dmitrii just observed.
    446      *
    447      * A slightly more managable design here would've been to use the user ID rather
    448      * than the user name. Not sure how this would work for domains, though, these
    449      * can probably be pretty long as well. Any numeric/fixed-sized IDs for domains?
    450      * Since these mistakes have been made already, perhaps use the UID as a
    451      * fallback?
    452      *
    453      * Also, since we're working a max length limit here, WTF do we use
    454      * RTStrAPrintf?  We could use a fixed buffer size and RTStrPrintf2 and detect
    455      * the problem here before we even get to the host side.
    456      */
    457     char *pszName;
    458     if (pszDomain)
    459     {
    460         if (RTStrAPrintf(&pszName, "%s%s@%s/%s", g_pszPropCacheValUser, pszUser, pszDomain, pszKey) < 0)
    461             rc = VERR_NO_MEMORY;
    462     }
    463     else
    464     {
    465         if (RTStrAPrintf(&pszName, "%s%s/%s", g_pszPropCacheValUser, pszUser, pszKey) < 0)
    466             rc = VERR_NO_MEMORY;
    467     }
    468 
    469456    char *pszValue = NULL;
    470     if (   RT_SUCCESS(rc)
    471         && pszValueFormat)
     457    if (pszValueFormat)
    472458    {
    473459        va_list va;
    … …  
    482468
    483469    if (RT_SUCCESS(rc))
    484         rc = VGSvcPropCacheUpdate(pCache, pszName, pszValue);
     470        rc = VGSvcPropCacheUpdate(pCache, szName, pszValue);
    485471    if (rc == VINF_SUCCESS) /* VGSvcPropCacheUpdate will also return VINF_NO_CHANGE. */
    486472    {
    487473        /** @todo Combine updating flags w/ updating the actual value. */
    488         rc = VGSvcPropCacheUpdateEntry(pCache, pszName,
     474        rc = VGSvcPropCacheUpdateEntry(pCache, szName,
    489475                                       VGSVCPROPCACHE_FLAGS_TEMPORARY | VGSVCPROPCACHE_FLAGS_TRANSIENT,
    490476                                       NULL /* Delete on exit */);
    … …  
    492478
    493479    RTStrFree(pszValue);
    494     RTStrFree(pszName);
     480    return rc;
     481}
     482
     483
     484/**
     485 * Updates a per-guest user guest property inside the given property cache.
     486 *
     487 * @return  VBox status code.
     488 * @retval  VERR_BUFFER_OVERFLOW if the final property name length exceeds the maximum supported length.
     489 * @param   pCache                  Pointer to guest property cache to update user in.
     490 * @param   pszUser                 Name of guest user to update.
     491 * @param   pszDomain               Domain of guest user to update. Optional.
     492 * @param   pszKey                  Key name of guest property to update.
     493 * @param   pszFormat               Format string to set. Pass NULL for deleting the property.
     494 * @param   va                      Format arguments.
     495 */
     496int VGSvcUserUpdateV(PVBOXSERVICEVEPROPCACHE pCache, const char *pszUser, const char *pszDomain,
     497                     const char *pszKey, const char *pszFormat, va_list va)
     498{
     499    char *psz = NULL;
     500    if (pszFormat) /* Might be NULL to delete a property. */
     501    {
     502        if (RTStrAPrintfV(&psz, pszFormat, va) < 0)
     503            return VERR_NO_MEMORY;
     504    }
     505    int const rc = VGSvcUserUpdateF(pCache, pszUser, pszDomain, pszKey, psz);
     506    RTStrFree(psz);
    495507    return rc;
    496508}

  • trunk/src/VBox/Additions/common/VBoxService/VBoxServiceVMInfo.h

    r98103 r102753  
    3535extern int VGSvcUserUpdateF(PVBOXSERVICEVEPROPCACHE pCache, const char *pszUser, const char *pszDomain,
    3636                            const char *pszKey, const char *pszValueFormat, ...);
     37extern int VGSvcUserUpdateV(PVBOXSERVICEVEPROPCACHE pCache, const char *pszUser, const char *pszDomain,
     38                            const char *pszKey, const char *pszFormat, va_list va);
    3739
     40extern int vgsvcVMInfoWinUserUpdateF(PVBOXSERVICEVEPROPCACHE pCache, const char *pszUser, const char *pszDomain,
     41                                     const char *pszKey, const char *pszValueFormat, ...);
    3842
    3943extern uint32_t g_uVMInfoUserIdleThresholdMS;
Note: See TracChangeset for help on using the changeset viewer.

© 2024 Oracle Support Privacy / Do Not Sell My Info Terms of Use Trademark Policy Automated Access Etiquette