get_{local,remote}_address() in winbindd_dual_pam_auth()

[Stefan Metzmacher]

Hi Gary,

in commit c8b7b7918b49f3598706190975a82be258aa9c44
Author:     Gary Lockyer <gary at catalyst.net.nz>
AuthorDate: Mon Jan 28 15:31:46 2019 +1300
Commit:     Andrew Bartlett <abartlet at samba.org>
CommitDate: Wed Feb 20 06:03:09 2019 +0100

    winbind: Log PAM and NTLM authentications.

    Generate JSON authentication messages for winbind PAM_AUTH and
    PAM_AUTH_CRAP requests.  The logon_id in these messages can be used to
    link them to the SamLogon messages.

    Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

You added get_local_address(), get_remote_address(), which take a socket filedescriptor
in order to the addresses from the socket.

However all you pass in is the socket fd that's created in
fork_domain_child() via socketpair(AF_UNIX, SOCK_STREAM, 0, fdpair).

I'm wondering how this could ever be useful in any way?

Also in _winbind_SamLogon() your passing down
p->{local,remote}_address from pipes_struct, which were not used
before in winbindd and are both NULL.
It means that make_user_info() in winbindd_dual_auth_passdb() will always
return NT_STATUS_NO_MEMORY. Luckily that code path is not triggered from auth_winbind.

Before we used tsocket_address_inet_from_strings(frame, "ip", "127.0.0.1", 0)
only in winbindd_dual_auth_passdb().

You are also using the wrong pid for client_pid, it's state->pid instead
of state->request->pid, state->pid comes from fork_domain_child():
state.cli.pid = getpid(); and is the pid of the winbindd parent process.

Maybe winbindd's parent should check request->pid against SO_PEERCRED/SO_PEERID?
before using it for auth logging?

How did you tested this (beside making sure it compiles)?

What would be the correct way out of this?

metze

-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20210512/cf684f07/OpenPGP_signature.sig>