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

xrdp_sec.c: add token login for autologon clients #2392

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from

Conversation

galeksandrp
Copy link
Contributor

Make token login available for clients with RDP_LOGON_AUTO flag, such as mstsc.exe 6.2.22000.282 (Win11 22621.608).

Make `token login` available for clients with `RDP_LOGON_AUTO` flag,
such as `mstsc.exe` `6.2.22000.282` (`Win11` `22621.608`).

xrdp_sec.c: reformat code to make astyle happy
@matt335672
Copy link
Member

Hi @galeksandrp

Thanks for this.

I've been looking at this area of the code from line 1002 to line 1035

https://github.com/galeksandrp/xrdp/blob/22e48dd91bb0e85d5f9eb7229d31b5de71bd0c49/libxrdp/xrdp_sec.c#L1002-L1035

I think it could have done with some refactoring before your change. Whereas I'm happy your change works OK, the code we've ended up with is even more complicated, and it took me a while to understand.

The relevant part of the spec is [MS-RDPBCGR] 2.2.1.11.1.1. Importantly the password field is not optional, but in the resulting code, it's read in two places which seems unnecessary.

I think this would work. It's a bit simpler:-

    if (unicode_utf16_in(s, len_password, self->rdp_layer->client_info.password, sizeof(self->rdp_layer->client_info.password) - 1) != 0)
    {
        LOG(LOG_LEVEL_ERROR, "ERROR reading password");
        return 1;
    }

    if (self->rdp_layer->client_info.enable_token_login
            && len_user > 0
            && len_password == 0
            && (sep = g_strchr(self->rdp_layer->client_info.username, '\x1f')) != NULL)
    {
        LOG(LOG_LEVEL_DEBUG, "Client supplied a Logon token. Overwriting password with logon token.");
        g_strncpy(self->rdp_layer->client_info.password, sep + 1,
                  sizeof(self->rdp_layer->client_info.password) - 1);
        self->rdp_layer->client_info.username[sep - self->rdp_layer->client_info.username] = '\0';
        self->rdp_layer->client_info.rdp_autologin = 1;
    }
    else if (self->rdp_layer->client_info.require_credentials &&
             !(flags & RDP_LOGON_AUTO))
    {
        LOG(LOG_LEVEL_ERROR, "Server is configured to require that the "
            "client enable auto logon with credentials, but the client did "
            "not request auto logon.");
        return 1; /* credentials on cmd line is mandatory */
    }

I haven't tested this yet. What do you think? Have I missed something?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants