Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[getpass] Implement a "strict" mode for getuser() #90789

Open
eryksun opened this issue Feb 4, 2022 · 4 comments
Open

[getpass] Implement a "strict" mode for getuser() #90789

eryksun opened this issue Feb 4, 2022 · 4 comments
Labels
3.13 new features, bugs and security fixes OS-windows stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@eryksun
Copy link
Contributor

eryksun commented Feb 4, 2022

BPO 46631
Nosy @pfmoore, @tjguk, @zware, @eryksun, @zooba

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2022-02-04.05:55:34.124>
labels = ['type-feature', 'library', 'OS-windows', '3.11']
title = 'Implement a "strict" mode for getpass.getuser()'
updated_at = <Date 2022-02-05.03:39:16.470>
user = 'https://github.com/eryksun'

bugs.python.org fields:

activity = <Date 2022-02-05.03:39:16.470>
actor = 'eryksun'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)', 'Windows']
creation = <Date 2022-02-04.05:55:34.124>
creator = 'eryksun'
dependencies = []
files = []
hgrepos = []
issue_num = 46631
keywords = []
message_count = 2.0
messages = ['412495', '412556']
nosy_count = 5.0
nosy_names = ['paul.moore', 'tim.golden', 'zach.ware', 'eryksun', 'steve.dower']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'needs patch'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue46631'
versions = ['Python 3.11']

@eryksun
Copy link
Contributor Author

eryksun commented Feb 4, 2022

The implementation of getpass.getuser() checks the environment variables "LOGNAME" (login name), "USER", "LNAME", and "USERNAME", in that order. On Windows, "LOGNAME", "USER", and "LNAME" have no conventional usage. There should be a strict mode that restricts getuser() to check only "USERNAME" on Windows and only "LOGNAME" on POSIX1. If the login environment variable isn't defined, it should fall back on using the system API, based on the user ID on POSIX and the logon ID on Windows.

For the fallback on Windows, the _winapi module could implement GetCurrentProcessToken(), GetTokenInformation(), and LsaGetLogonSessionData(). For the TokenStatistics token information, it can return a dict that for now just contains "AuthenticationId" (i.e. the login ID). For LsaGetLogonSessionData(), return a dict that for now just contains "UserName". Note that GetCurrentProcessToken() returns a pseudohandle (-4), which should not be closed.

For example, assuming that _winapi wraps the required functions:

    def getuser(strict=False):
        """Get the username from the environment or password database.

        First try various environment variables. If strict, check only LOGNAME
        on POSIX and only USERNAME on Windows. As a fallback, on POSIX get the
        user name from the password database, and on Windows get the user name
        from the logon session of the current process.
        """
        posix = sys.platform != 'win32'

        if strict:
            names = ('LOGNAME',) if posix else ('USERNAME',)
        else:
            names = ('LOGNAME', 'USER', 'LNAME', 'USERNAME')

        for name in names:
            if user := os.environ.get(name):
                return user

        if posix:
            import pwd
            return pwd.getpwuid(os.getuid())[0]

        import _winapi
        logon_id = _winapi.GetTokenInformation(
                        _winapi.GetCurrentProcessToken(),
                        _winapi.TokenStatistics)['AuthenticationId']
        return _winapi.LsaGetLogonSessionData(logon_id)['UserName']

Like WinAPI GetUserNameW(), the above fallback returns the logon user name instead of the account name of the token user. As far as I know, the user name and the account name only differ for the builtin service account logons "SYSTEM" (999) and "NETWORK SERVICE" (996), for which the user name is the machine security principal (i.e. the machine's NETBIOS name plus "$").

Unlike GetUserNameW(), the above code uses the process token instead of the effective token, which is like using getuid() on POSIX. In contrast, GetUserNameW() is like using geteuid() on POSIX. An effective option could be supported for the latter. Calling getuser(effective=True) would skip checking the environment variable and get the effective user name from the system. On Windows, it would use GetCurrentThreadEffectiveToken() instead of GetCurrentProcessToken(). On POSIX, it would use geteuid() instead of getuid().

Footnotes

  1. https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html

@eryksun eryksun added 3.11 bug and security fixes stdlib Python modules in the Lib dir OS-windows type-feature A feature request or enhancement labels Feb 4, 2022
@eryksun
Copy link
Contributor Author

eryksun commented Feb 5, 2022

Here's an example for the suggested changes to _winapi.

Include these headers:

    #include <ntsecapi.h> // LsaGetLogonSessionData
    #include <subauth.h> // STATUS_SUCCESS

Add these argument-clinic macros to _winapi_functions:

_WINAPI_GETCURRENTPROCESSTOKEN_METHODDEF
_WINAPI_GETTOKENINFORMATION_METHODDEF
_WINAPI_LSAGETLOGONSESSIONDATA_METHODDEF

Add TokenStatistics in winapi_exec():

WINAPI_CONSTANT(F_DWORD, TokenStatistics);

Add the wrapped functions:

/*[clinic input]
_winapi.GetCurrentProcessToken -> HANDLE

Return a handle for the access token of the current process.
[clinic start generated code]*/

static HANDLE
_winapi_GetCurrentProcessToken_impl(PyObject *module)
/*[clinic end generated code: output=cf8e8e20dd41dd6e input=73a282cf3718af9e]*/
{
    return GetCurrentProcessToken();
}
/*[clinic input]
_winapi.GetTokenInformation

    handle: HANDLE
    information_class: unsigned_long
    /

Get information from an access token.
[clinic start generated code]*/

static PyObject *
_winapi_GetTokenInformation_impl(PyObject *module, HANDLE handle,
                                 unsigned long information_class)
/*[clinic end generated code: output=caecec0a25658348 input=b277ad2414f1b03e]*/
{
    if (information_class != TokenStatistics) {
        return PyErr_Format(
                    PyExc_NotImplementedError,
                    "Unsupported information class: %d",
                    information_class);
    }

    DWORD returned_size;
    TOKEN_STATISTICS info;

    if (!GetTokenInformation(handle, information_class, &info, sizeof(info),
            &returned_size)) {
        return PyErr_SetFromWindowsErr(0);
    }

    PyObject *result = PyDict_New();
    if (!result) {
        return NULL;
    }

    PyObject *value = PyLong_FromUnsignedLongLong(
                        (((uint64_t)info.AuthenticationId.HighPart) << 32) +
                        info.AuthenticationId.LowPart);
    if (!value) {
        goto error;
    }

    if (PyDict_SetItemString(result, "AuthenticationId", value) < 0) {
        Py_DECREF(value);
        goto error;
    }

    Py_DECREF(value);
    return result;

error:
    Py_CLEAR(result);
    return NULL;
}
/*[clinic input]
_winapi.LsaGetLogonSessionData

    logon_id: unsigned_long_long
    /
Get data for the logon session identified by logon_id.
[clinic start generated code]*/

static PyObject *
_winapi_LsaGetLogonSessionData_impl(PyObject *module,
                                    unsigned long long logon_id)
/*[clinic end generated code: output=680ac7725ef34527 input=01ff4216b89d01ef]*/
{
    SECURITY_LOGON_SESSION_DATA *pdata;
    LUID logon_luid;
    logon_luid.HighPart = logon_id >> 32;
    logon_luid.LowPart = logon_id & 0xFFFFFFFF;

    NTSTATUS status = LsaGetLogonSessionData(&logon_luid, &pdata);
    if (status != STATUS_SUCCESS) {
        return PyErr_SetFromWindowsErr(LsaNtStatusToWinError(status));
    }

    PyObject *result = PyDict_New();
    if (!result) {
        goto error;
    }

    PyObject *value = PyUnicode_FromWideChar(pdata->UserName.Buffer,
                            pdata->UserName.Length / sizeof(WCHAR));
    if (!value) {
        goto error;
    }

    if (PyDict_SetItemString(result, "UserName", value) < 0) {
        Py_DECREF(value);
        goto error;
    }

    Py_DECREF(value);
    LsaFreeReturnBuffer(pdata);
    return result;

error:
    LsaFreeReturnBuffer(pdata);
    Py_CLEAR(result);
    return NULL;
}

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
@eryksun eryksun changed the title Implement a "strict" mode for getpass.getuser() [getpass] Implement a "strict" mode for getuser() Aug 8, 2022
@eryksun eryksun added 3.12 bugs and security fixes and removed 3.11 bug and security fixes labels Oct 24, 2022
@gpshead
Copy link
Member

gpshead commented Nov 12, 2022

I suggest not adding a strict mode but just improving the behavior without that conditional.

@zooba
Copy link
Member

zooba commented Nov 15, 2022

We've dropped non-standard environment variables before, and I'm not sure it's worth it. People have definitely found a need to override what some Python library thinks their username is, and chances are the lie is more useful than the truth.

@erlend-aasland erlend-aasland added 3.13 new features, bugs and security fixes and removed 3.12 bugs and security fixes labels Jan 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.13 new features, bugs and security fixes OS-windows stdlib Python modules in the Lib dir type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

4 participants