Replace various types used for auth_info

The loadable sesman authentication modules use different types for the
authentication handle returned from auth_userpass(). The PAM module
uses a pointer, and the other modules use (effectively) a boolean. Within
sesman itself, a long or tbus (intptr_t) is used.

This PR replaces all of these types with a pointer to an incomplete type.

Consequently:-
- A single better-labelled type is used it all places within sesman so
  it's more obvious what's being handled.
- There is no need to cast the authentication handle within the PAM
  module to a long and back again.
- The compiler can check function signatures between auth.h and the
  various verify modules.
This commit is contained in:
matt335672 2022-09-11 13:18:53 +01:00
parent e1697879ec
commit 660ac303f0
9 changed files with 173 additions and 140 deletions

View File

@ -27,59 +27,71 @@
#ifndef AUTH_H
#define AUTH_H
/**
* Opaque type used to represent an authentication handle
*/
struct auth_info;
/**
*
* @brief Validates user's password
* @param user user's login name
* @param pass user's password
* @param client_ip IP address of connecting client (or ""/NULL if not known)
* @return non-zero handle on success, 0 on failure
* @param[out] errorcode from result
* @return auth handle on success, NULL on failure
*
*/
long
struct auth_info *
auth_userpass(const char *user, const char *pass,
const char *client_ip, int *errorcode);
/**
*
* @brief FIXME
* @param in_val
* @param in_display
* @brief Starts a session
* @param auth_info. Auth handle created by auth_userpass
* @param display_num Display number
* @return 0 on success, 1 on failure
*
*/
int
auth_start_session(long in_val, int in_display);
auth_start_session(struct auth_info *auth_info, int display_num);
/**
*
* @brief FIXME
* @param in_val
* @brief Stops a session previously started with auth_start_session()
* @param auth_info. Auth handle created by auth_userpass
* @return 0 on success, 1 on failure
*
*/
int
auth_stop_session(long in_val);
auth_stop_session(struct auth_info *auth_info);
/**
*
* @brief FIXME
* @param in_val
* @brief Deallocates an auth handle and releases all resources
* @param auth_info. Auth handle created by auth_userpass
* @return 0 on success, 1 on failure
*
*/
int
auth_end(long in_val);
auth_end(struct auth_info *auth_info);
/**
*
* @brief FIXME
* @param in_val
* @brief Sets up the environment for a session started
* with auth_start_session()
*
* This call is only effective for PAM-based environments. It must be made
* after the context has been switched to the logged-in user.
*
* @param auth_info. Auth handle created by auth_userpass
* @return 0 on success, 1 on failure
*
*/
int
auth_set_env(long in_val);
auth_set_env(struct auth_info *auth_info);
#define AUTH_PWD_CHG_OK 0
@ -90,8 +102,9 @@ auth_set_env(long in_val);
/**
*
* @brief FIXME
* @param in_val
* @brief WIP - Checks to see if the password for a user needs changing
*
* @param user - Username to check
* @return 0 on success, 1 on failure
*
*/
@ -100,8 +113,10 @@ auth_check_pwd_chg(const char *user);
/**
*
* @brief FIXME
* @param in_val
* @brief WIP - Changes the password for a user
*
* @param user Username to check
* @param newpwd New password for user
* @return 0 on success, 1 on failure
*
*/

View File

@ -70,13 +70,13 @@ process_gateway_request(struct trans *trans)
if (rv == 0)
{
int errorcode = 0;
tbus data;
struct auth_info *auth_info;
LOG(LOG_LEVEL_INFO, "Received authentication request for user: %s",
username);
data = auth_userpass(username, password, ip_addr, &errorcode);
if (data)
auth_info = auth_userpass(username, password, ip_addr, &errorcode);
if (auth_info != NULL)
{
if (1 == access_login_allowed(username))
{
@ -97,7 +97,7 @@ process_gateway_request(struct trans *trans)
log_authfail_message(username, ip_addr);
}
rv = scp_send_gateway_response(trans, errorcode);
auth_end(data);
auth_end(auth_info);
}
return rv;
}
@ -123,7 +123,7 @@ process_create_session_request(struct trans *trans)
if (rv == 0)
{
tbus data;
struct auth_info *auth_info;
struct session_item *s_item;
int errorcode = 0;
bool_t do_auth_end = 1;
@ -133,8 +133,8 @@ process_create_session_request(struct trans *trans)
SCP_SESSION_TYPE_TO_STR(sp.type),
sp.username);
data = auth_userpass(sp.username, password, sp.ip_addr, &errorcode);
if (data)
auth_info = auth_userpass(sp.username, password, sp.ip_addr, &errorcode);
if (auth_info != NULL)
{
s_item = session_get_bydata(&sp);
if (s_item != 0)
@ -154,7 +154,7 @@ process_create_session_request(struct trans *trans)
s_item->pid);
}
session_reconnect(display, sp.username, data);
session_reconnect(display, sp.username, auth_info);
}
else
{
@ -175,7 +175,7 @@ process_create_session_request(struct trans *trans)
"username %s", sp.username);
}
display = session_start(data, &sp, &guid);
display = session_start(auth_info, &sp, &guid);
/* if the session started up ok, auth_end will be called on
sig child */
@ -190,7 +190,7 @@ process_create_session_request(struct trans *trans)
if (do_auth_end)
{
auth_end(data);
auth_end(auth_info);
}
rv = scp_send_create_session_response(trans, errorcode, display, &guid);
@ -214,13 +214,13 @@ process_list_sessions_request(struct trans *trans)
{
enum scp_list_sessions_status status;
int errorcode = 0;
tbus data;
struct auth_info *auth_info;
LOG(LOG_LEVEL_INFO,
"Received request to list sessions for user %s", username);
data = auth_userpass(username, password, NULL, &errorcode);
if (data)
auth_info = auth_userpass(username, password, NULL, &errorcode);
if (auth_info != NULL)
{
struct scp_session_info *info = NULL;
unsigned int cnt = 0;
@ -241,7 +241,7 @@ process_list_sessions_request(struct trans *trans)
{
status = E_SCP_LS_AUTHENTICATION_FAIL;
}
auth_end(data);
auth_end(auth_info);
if (rv == 0)
{

View File

@ -438,7 +438,7 @@ session_start_chansrv(const char *username, int display)
/******************************************************************************/
int
session_start(long data,
session_start(struct auth_info *auth_info,
const struct session_parameters *s,
struct guid *guid)
{
@ -546,7 +546,7 @@ session_start(long data,
g_exit(1);
}
auth_start_session(data, display);
auth_start_session(auth_info, display);
sesman_close_all();
g_sprintf(geometry, "%dx%d", s->width, s->height);
g_sprintf(depth, "%d", s->bpp);
@ -615,7 +615,7 @@ session_start(long data,
g_cfg->env_values);
if (x_server_running(display))
{
auth_set_env(data);
auth_set_env(auth_info);
if (s->directory != 0)
{
if (s->directory[0] != 0)
@ -932,8 +932,8 @@ session_start(long data,
LOG(LOG_LEVEL_INFO,
"Calling auth_stop_session and auth_end from pid %d",
g_getpid());
auth_stop_session(data);
auth_end(data);
auth_stop_session(auth_info);
auth_end(auth_info);
LOG(LOG_LEVEL_INFO,
"Terminating X server (pid %d) on display %d",
@ -976,7 +976,7 @@ session_start(long data,
temp->item->width = s->width;
temp->item->height = s->height;
temp->item->bpp = s->bpp;
temp->item->data = data;
temp->item->auth_info = auth_info;
g_strncpy(temp->item->start_ip_addr, s->ip_addr,
sizeof(temp->item->start_ip_addr) - 1);
g_strncpy(temp->item->name, s->username, 255);
@ -1001,7 +1001,8 @@ session_start(long data,
/******************************************************************************/
int
session_reconnect(int display, const char *username, long data)
session_reconnect(int display, const char *username,
struct auth_info *auth_info)
{
int pid;
@ -1018,7 +1019,7 @@ session_reconnect(int display, const char *username, long data)
display,
g_cfg->env_names,
g_cfg->env_values);
auth_set_env(data);
auth_set_env(auth_info);
if (g_file_exist(g_cfg->reconnect_sh))
{

View File

@ -59,7 +59,7 @@ struct session_item
int width;
int height;
int bpp;
long data;
struct auth_info *auth_info;
/* status info */
unsigned char status;
@ -114,12 +114,13 @@ session_get_bydata(const struct session_parameters *params);
*
*/
int
session_start(long authdata,
session_start(struct auth_info *auth_info,
const struct session_parameters *params,
struct guid *guid);
int
session_reconnect(int display, const char *username, long data);
session_reconnect(int display, const char *username,
struct auth_info *auth_info);
/**
*

View File

@ -28,7 +28,10 @@
#include <config_ac.h>
#endif
#include "sesman.h"
#include "arch.h"
#include "auth.h"
#include "log.h"
#include "os_calls.h"
#include "string_calls.h"
#include <stdio.h>
@ -48,9 +51,17 @@ auth_crypt_pwd(const char *pwd, const char *pln, char *crp);
static int
auth_account_disabled(struct spwd *stp);
/*
* Need a complete type for struct auth_info, even though we're
* not really using it if this module (UNIX authentication) is selected */
struct auth_info
{
char dummy;
};
/******************************************************************************/
/* returns boolean */
long
/* returns non-NULL for success */
struct auth_info *
auth_userpass(const char *user, const char *pass,
const char *client_ip, int *errorcode)
{
@ -58,12 +69,14 @@ auth_userpass(const char *user, const char *pass,
const char *epass;
struct passwd *spw;
struct spwd *stp;
/* Need a non-NULL pointer to return to indicate success */
static struct auth_info success = {0};
spw = getpwnam(user);
if (spw == 0)
{
return 0;
return NULL;
}
if (g_strncmp(spw->pw_passwd, "x", 3) == 0)
@ -73,13 +86,13 @@ auth_userpass(const char *user, const char *pass,
if (stp == 0)
{
return 0;
return NULL;
}
if (1 == auth_account_disabled(stp))
{
LOG(LOG_LEVEL_INFO, "account %s is disabled", user);
return 0;
return NULL;
}
encr = stp->sp_pwdp;
@ -92,15 +105,15 @@ auth_userpass(const char *user, const char *pass,
epass = crypt(pass, encr);
if (epass == 0)
{
return 0;
return NULL;
}
return (strcmp(encr, epass) == 0);
return (strcmp(encr, epass) == 0) ? &success : NULL;
}
/******************************************************************************/
/* returns error */
int
auth_start_session(long in_val, int in_display)
auth_start_session(struct auth_info *auth_info, int display_num)
{
return 0;
}
@ -108,21 +121,21 @@ auth_start_session(long in_val, int in_display)
/******************************************************************************/
/* returns error */
int
auth_stop_session(long in_val)
auth_stop_session(struct auth_info *auth_info)
{
return 0;
}
/******************************************************************************/
int
auth_end(long in_val)
auth_end(struct auth_info *auth_info)
{
return 0;
}
/******************************************************************************/
int
auth_set_env(long in_val)
auth_set_env(struct auth_info *auth_info)
{
return 0;
}

View File

@ -28,7 +28,8 @@
#include <config_ac.h>
#endif
#include "sesman.h"
#include "arch.h"
#include "auth.h"
#define _XOPEN_SOURCE
#include <stdio.h>
@ -43,34 +44,49 @@
#define SECS_PER_DAY (24L*3600L)
#endif
/*
* Need a complete type for struct auth_info, even though we're
* not really using it if this module (BSD authentication) is selected */
struct auth_info
{
char dummy;
};
/******************************************************************************/
/* returns boolean */
long
/* returns non-NULL for success */
struct auth_info *
auth_userpass(const char *user, const char *pass,
const char *client_ip, int *errorcode)
{
int ret = auth_userokay(user, NULL, "auth-xrdp", pass);
/* Need a non-NULL pointer to return to indicate success */
static struct auth_info success = {0};
struct auth_info *ret = NULL;
if (auth_userokay(user, NULL, "auth-xrdp", pass))
{
ret = &success;
}
return ret;
}
/******************************************************************************/
/* returns error */
int
auth_start_session(long in_val, int in_display)
auth_start_session(struct auth_info *auth_info, int display_num)
{
return 0;
}
/******************************************************************************/
int
auth_end(long in_val)
auth_end(struct auth_info *auth_info)
{
return 0;
}
/******************************************************************************/
int
auth_set_env(long in_val)
auth_set_env(struct auth_info *auth_info)
{
return 0;
}
@ -89,33 +105,7 @@ auth_change_pwd(const char *user, const char *newpwd)
}
int
auth_stop_session(long in_val)
{
return 0;
}
/**
*
* @brief Password encryption
* @param pwd Old password
* @param pln Plaintext new password
* @param crp Crypted new password
*
*/
static int
auth_crypt_pwd(const char *pwd, const char *pln, char *crp)
{
return 0;
}
/**
*
* @return 1 if the account is disabled, 0 otherwise
*
*/
static int
auth_account_disabled(struct spwd *stp)
auth_stop_session(struct auth_info *auth_info)
{
return 0;
}

View File

@ -29,6 +29,7 @@
#endif
#include "arch.h"
#include "auth.h"
#include "os_calls.h"
#include "string_calls.h"
@ -76,6 +77,14 @@ struct user_info
const char *pass;
};
/*
* Need a complete type for struct auth_info, even though we're
* not really using it if this module (Kerberos authentication) is selected */
struct auth_info
{
char dummy;
};
/******************************************************************************/
/* returns boolean */
static int
@ -399,8 +408,8 @@ cleanup:
}
/******************************************************************************/
/* returns boolean */
long
/* returns non-NULL for success */
struct auth_info *
auth_userpass(const char *user, const char *pass,
const char *client_ip, int *errorcode)
{
@ -408,7 +417,9 @@ auth_userpass(const char *user, const char *pass,
struct k5_data k5;
struct user_info u_info;
int got_k5;
int authed_k5;
/* Need a non-NULL pointer to return to indicate success */
static struct auth_info success = {0};
struct auth_info *auth_info = NULL;
g_memset(&opts, 0, sizeof(opts));
opts.action = INIT_PW;
@ -416,22 +427,24 @@ auth_userpass(const char *user, const char *pass,
g_memset(&u_info, 0, sizeof(u_info));
u_info.name = user;
u_info.pass = pass;
authed_k5 = 0;
got_k5 = k5_begin(&opts, &k5, &u_info);
if (got_k5)
{
authed_k5 = k5_kinit(&opts, &k5, &u_info);
if (k5_kinit(&opts, &k5, &u_info))
{
auth_info = &success;
}
k5_end(&k5);
}
return authed_k5;
return auth_info;
}
/******************************************************************************/
/* returns error */
int
auth_start_session(long in_val, int in_display)
auth_start_session(struct auth_info *auth_info, int display_num)
{
return 0;
}
@ -439,21 +452,21 @@ auth_start_session(long in_val, int in_display)
/******************************************************************************/
/* returns error */
int
auth_stop_session(long in_val)
auth_stop_session(struct auth_info *auth_info)
{
return 0;
}
/******************************************************************************/
int
auth_end(long in_val)
auth_end(struct auth_info *auth_info)
{
return 0;
}
/******************************************************************************/
int
auth_set_env(long in_val)
auth_set_env(struct auth_info *auth_info)
{
return 0;
}

View File

@ -46,7 +46,7 @@ struct t_user_pass
char pass[MAX_BUF];
};
struct t_auth_info
struct auth_info
{
struct t_user_pass user_pass;
int session_opened;
@ -209,19 +209,19 @@ get_service_name(char *service_name)
}
/******************************************************************************/
/* returns long, zero is no go
Stores the detailed error code in the errorcode variable*/
/* returns non-NULL for success
* Detailed error code is in the errorcode variable */
long
struct auth_info *
auth_userpass(const char *user, const char *pass,
const char *client_ip, int *errorcode)
{
int error;
struct t_auth_info *auth_info;
struct auth_info *auth_info;
char service_name[256];
get_service_name(service_name);
auth_info = g_new0(struct t_auth_info, 1);
auth_info = g_new0(struct auth_info, 1);
g_strncpy(auth_info->user_pass.user, user, MAX_BUF - 1);
g_strncpy(auth_info->user_pass.pass, pass, MAX_BUF - 1);
auth_info->pamc.conv = &verify_pam_conv;
@ -238,7 +238,7 @@ auth_userpass(const char *user, const char *pass,
pam_strerror(auth_info->ph, error));
pam_end(auth_info->ph, error);
g_free(auth_info);
return 0;
return NULL;
}
if (client_ip != NULL && client_ip[0] != '\0')
@ -270,7 +270,7 @@ auth_userpass(const char *user, const char *pass,
pam_strerror(auth_info->ph, error));
pam_end(auth_info->ph, error);
g_free(auth_info);
return 0;
return NULL;
}
/* From man page:
The pam_acct_mgmt function is used to determine if the users account is
@ -290,23 +290,21 @@ auth_userpass(const char *user, const char *pass,
pam_strerror(auth_info->ph, error));
pam_end(auth_info->ph, error);
g_free(auth_info);
return 0;
return NULL;
}
return (long)auth_info;
return auth_info;
}
/******************************************************************************/
/* returns error */
int
auth_start_session(long in_val, int in_display)
auth_start_session(struct auth_info *auth_info, int display_num)
{
struct t_auth_info *auth_info;
int error;
char display[256];
g_sprintf(display, ":%d", in_display);
auth_info = (struct t_auth_info *)in_val;
g_sprintf(display, ":%d", display_num);
error = pam_set_item(auth_info->ph, PAM_TTY, display);
if (error != PAM_SUCCESS)
@ -342,12 +340,10 @@ auth_start_session(long in_val, int in_display)
/******************************************************************************/
/* returns error */
int
auth_stop_session(long in_val)
auth_stop_session(struct auth_info *auth_info)
{
struct t_auth_info *auth_info;
int error;
auth_info = (struct t_auth_info *)in_val;
error = pam_close_session(auth_info->ph, 0);
if (error != PAM_SUCCESS)
{
@ -363,13 +359,9 @@ auth_stop_session(long in_val)
/* returns error */
/* cleanup */
int
auth_end(long in_val)
auth_end(struct auth_info *auth_info)
{
struct t_auth_info *auth_info;
auth_info = (struct t_auth_info *)in_val;
if (auth_info != 0)
if (auth_info != NULL)
{
if (auth_info->ph != 0)
{
@ -396,18 +388,15 @@ auth_end(long in_val)
/* returns error */
/* set any pam env vars */
int
auth_set_env(long in_val)
auth_set_env(struct auth_info *auth_info)
{
struct t_auth_info *auth_info;
char **pam_envlist;
char **pam_env;
char item[256];
char value[256];
int eq_pos;
auth_info = (struct t_auth_info *)in_val;
if (auth_info != 0)
if (auth_info != NULL)
{
/* export PAM environment */
pam_envlist = pam_getenvlist(auth_info->ph);

View File

@ -29,6 +29,7 @@
#endif
#include "arch.h"
#include "auth.h"
#include "os_calls.h"
#include "string_calls.h"
@ -36,9 +37,17 @@
#define SERVICE "xrdp"
/*
* Need a complete type for struct auth_info, even though we're
* not really using it if this module (PAM userpass) is selected */
struct auth_info
{
char dummy;
};
/******************************************************************************/
/* returns boolean */
long
/* returns non-NULL for success */
struct auth_info *
auth_userpass(const char *user, const char *pass,
const char *client_ip, int *errorcode)
{
@ -47,13 +56,15 @@ auth_userpass(const char *user, const char *pass,
struct pam_conv conv = {pam_userpass_conv, &userpass};
const void *template1;
int status;
/* Need a non-NULL pointer to return to indicate success */
static struct auth_info success = {0};
userpass.user = user;
userpass.pass = pass;
if (pam_start(SERVICE, user, &conv, &pamh) != PAM_SUCCESS)
{
return 0;
return NULL;
}
status = pam_authenticate(pamh, 0);
@ -61,7 +72,7 @@ auth_userpass(const char *user, const char *pass,
if (status != PAM_SUCCESS)
{
pam_end(pamh, status);
return 0;
return NULL;
}
status = pam_acct_mgmt(pamh, 0);
@ -69,7 +80,7 @@ auth_userpass(const char *user, const char *pass,
if (status != PAM_SUCCESS)
{
pam_end(pamh, status);
return 0;
return NULL;
}
status = pam_get_item(pamh, PAM_USER, &template1);
@ -77,21 +88,21 @@ auth_userpass(const char *user, const char *pass,
if (status != PAM_SUCCESS)
{
pam_end(pamh, status);
return 0;
return NULL;
}
if (pam_end(pamh, PAM_SUCCESS) != PAM_SUCCESS)
{
return 0;
return NULL;
}
return 1;
return &success;
}
/******************************************************************************/
/* returns error */
int
auth_start_session(long in_val, int in_display)
auth_start_session(struct auth_info *auth_info, int display_num)
{
return 0;
}
@ -99,21 +110,21 @@ auth_start_session(long in_val, int in_display)
/******************************************************************************/
/* returns error */
int
auth_stop_session(long in_val)
auth_stop_session(struct auth_info *auth_info)
{
return 0;
}
/******************************************************************************/
int
auth_end(long in_val)
auth_end(struct auth_info *auth_info)
{
return 0;
}
/******************************************************************************/
int
auth_set_env(long in_val)
auth_set_env(struct auth_info *auth_info)
{
return 0;
}