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.

Sign up for GitHub

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

Jump to bottom

Backport #10460 to branch/v8 #10617

Merged
merged 4 commits into from Mar 3, 2022
Merged

Backport #10460 to branch/v8 #10617

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d8b9260
Fix utmp accounting on system with bad glibc default (#10460)
xacrimon Feb 25, 2022
ea335be
remove racy errno
xacrimon Feb 25, 2022
18fc004
Merge branch 'branch/v8' into joel/v8/backport-10460
xacrimon Mar 3, 2022
684875e
Merge branch 'branch/v8' into joel/v8/backport-10460
xacrimon Mar 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
37 changes: 27 additions & 10 deletions lib/srv/uacc/uacc.h
View file
Open in desktop
Expand Up @@ -26,6 +26,14 @@ limitations under the License.
#include <stdlib.h>
#include <limits.h>

// Sometimes the _UTMP_PATH and _WTMP_PATH macros from glibc are bad, this seems to depend on distro.
// I asked around on IRC, no one really knows why. I suspect it's another
// archaic remnant of old Unix days and that a cleanup is long overdue.
//
// In the meantime, we just try to resolve from these paths instead.
#define UACC_UTMP_PATH "/var/run/utmp"
#define UACC_WTMP_PATH "/var/run/wtmp"

int UACC_UTMP_MISSING_PERMISSIONS = 1;
int UACC_UTMP_WRITE_ERROR = 2;
int UACC_UTMP_READ_ERROR = 3;
Expand All @@ -35,6 +43,12 @@ int UACC_UTMP_FAILED_TO_SELECT_FILE = 6;
int UACC_UTMP_OTHER_ERROR = 7;
int UACC_UTMP_PATH_DOES_NOT_EXIST = 8;

// This is a bit of a hack, but it seems cleaner than doing it any other way when we're dealing with CGO FFI.
// We use this string pointer to store a potential error message from glibc in certain cases.
//
// At first glance this may seem racy, however this pointer is protected by the mutex that wraps all uacc logic on the Go side.
char* UACC_PATH_ERR;

// I initially attempted to use the login/logout BSD functions but ran into a string of unexpected behaviours such as
// errno being set to undocument values along with wierd return values in certain cases. They also modify the utmp database
// in a way we don't want. We want to insert a USER_PROCESS entry directly before we do PAM/cgroup setup and launch the shell
Expand Down Expand Up @@ -78,18 +92,15 @@ static int check_abs_path_err(const char* buffer) {

// check for GNU extension errors
if (errno == EACCES || errno == ENOENT) {
UACC_PATH_ERR = (char*)malloc(PATH_MAX);
strcpy(UACC_PATH_ERR, buffer);
return UACC_UTMP_OTHER_ERROR;
}

// no error was found
return 0;
}

// Allow the Go side to read errno.
static int get_errno() {
return errno;
}

// The max byte length of the C string representing the TTY name.
static int max_len_tty_name() {
return UT_LINESIZE;
Expand All @@ -98,8 +109,9 @@ static int max_len_tty_name() {
// Low level C function to add a new USER_PROCESS entry to the database.
// This function does not perform any argument validation.
static int uacc_add_utmp_entry(const char *utmp_path, const char *wtmp_path, const char *username, const char *hostname, const int32_t remote_addr_v6[4], const char *tty_name, const char *id, int32_t tv_sec, int32_t tv_usec) {
UACC_PATH_ERR = NULL;
char resolved_utmp_buffer[PATH_MAX];
const char* file = get_absolute_path_with_fallback(&resolved_utmp_buffer[0], utmp_path, _PATH_UTMP);
const char* file = get_absolute_path_with_fallback(&resolved_utmp_buffer[0], utmp_path, UACC_UTMP_PATH);
int status = check_abs_path_err(file);
if (status != 0) {
return status;
Expand All @@ -121,6 +133,7 @@ static int uacc_add_utmp_entry(const char *utmp_path, const char *wtmp_path, con
errno = 0;
setutent();
if (errno > 0) {
endutent();
return UACC_UTMP_FAILED_OPEN;
}
if (pututline(&entry) == NULL) {
Expand All @@ -129,7 +142,7 @@ static int uacc_add_utmp_entry(const char *utmp_path, const char *wtmp_path, con
}
endutent();
char resolved_wtmp_buffer[PATH_MAX];
const char* wtmp_file = get_absolute_path_with_fallback(&resolved_wtmp_buffer[0], wtmp_path, _PATH_WTMP);
const char* wtmp_file = get_absolute_path_with_fallback(&resolved_wtmp_buffer[0], wtmp_path, UACC_WTMP_PATH);
status = check_abs_path_err(wtmp_file);
if (status != 0) {
return status;
Expand All @@ -141,8 +154,9 @@ static int uacc_add_utmp_entry(const char *utmp_path, const char *wtmp_path, con
// Low level C function to mark a database entry as DEAD_PROCESS.
// This function does not perform string argument validation.
static int uacc_mark_utmp_entry_dead(const char *utmp_path, const char *wtmp_path, const char *tty_name, int32_t tv_sec, int32_t tv_usec) {
UACC_PATH_ERR = NULL;
char resolved_utmp_buffer[PATH_MAX];
const char* file = get_absolute_path_with_fallback(&resolved_utmp_buffer[0], utmp_path, _PATH_UTMP);
const char* file = get_absolute_path_with_fallback(&resolved_utmp_buffer[0], utmp_path, UACC_UTMP_PATH);
int status = check_abs_path_err(file);
if (status != 0) {
return status;
Expand Down Expand Up @@ -174,6 +188,7 @@ static int uacc_mark_utmp_entry_dead(const char *utmp_path, const char *wtmp_pat
errno = 0;
setutent();
if (errno != 0) {
endutent();
return UACC_UTMP_FAILED_OPEN;
}
if (pututline(&entry) == NULL) {
Expand All @@ -182,7 +197,7 @@ static int uacc_mark_utmp_entry_dead(const char *utmp_path, const char *wtmp_pat
}
endutent();
char resolved_wtmp_buffer[PATH_MAX];
const char* wtmp_file = get_absolute_path_with_fallback(&resolved_wtmp_buffer[0], wtmp_path, _PATH_WTMP);
const char* wtmp_file = get_absolute_path_with_fallback(&resolved_wtmp_buffer[0], wtmp_path, UACC_WTMP_PATH);
status = check_abs_path_err(wtmp_file);
if (status != 0) {
return status;
Expand All @@ -194,8 +209,9 @@ static int uacc_mark_utmp_entry_dead(const char *utmp_path, const char *wtmp_pat
// Low level C function to check the database for an entry for a given user.
// This function does not perform string argument validation.
static int uacc_has_entry_with_user(const char *utmp_path, const char *user) {
UACC_PATH_ERR = NULL;
char resolved_utmp_buffer[PATH_MAX];
const char* file = get_absolute_path_with_fallback(&resolved_utmp_buffer[0], utmp_path, _PATH_UTMP);
const char* file = get_absolute_path_with_fallback(&resolved_utmp_buffer[0], utmp_path, UACC_UTMP_PATH);
int status = check_abs_path_err(file);
if (status != 0) {
return status;
Expand All @@ -206,6 +222,7 @@ static int uacc_has_entry_with_user(const char *utmp_path, const char *user) {
errno = 0;
setutent();
if (errno != 0) {
endutent();
return UACC_UTMP_FAILED_OPEN;
}
struct utmp *entry = getutent();
Expand Down
49 changes: 24 additions & 25 deletions lib/srv/uacc/uacc_linux.go
View file
Open in desktop
Expand Up @@ -101,27 +101,22 @@ func Open(utmpPath, wtmpPath string, username, hostname string, remote [4]int32,
microsFraction := (C.int32_t)((timestamp.UnixNano() % int64(time.Second)) / int64(time.Microsecond))

accountDb.Lock()
status := C.uacc_add_utmp_entry(cUtmpPath, cWtmpPath, cUsername, cHostname, &cIP[0], cTtyName, cIDName, secondsElapsed, microsFraction)
accountDb.Unlock()
defer accountDb.Unlock()
status, errno := C.uacc_add_utmp_entry(cUtmpPath, cWtmpPath, cUsername, cHostname, &cIP[0], cTtyName, cIDName, secondsElapsed, microsFraction)

switch status {
case C.UACC_UTMP_MISSING_PERMISSIONS:
return trace.AccessDenied("missing permissions to write to utmp/wtmp")
case C.UACC_UTMP_WRITE_ERROR:
return trace.AccessDenied("failed to add entry to utmp database")
case C.UACC_UTMP_FAILED_OPEN:
code := C.get_errno()
return trace.AccessDenied("failed to open user account database, code: %d", code)
return trace.AccessDenied("failed to open user account database, code: %d", errno)
case C.UACC_UTMP_FAILED_TO_SELECT_FILE:
return trace.BadParameter("failed to select file")
case C.UACC_UTMP_PATH_DOES_NOT_EXIST:
return trace.NotFound("user accounting files are missing from the system, running in a container?")
default:
if status != 0 {
return trace.Errorf("unknown error with errno %d", C.get_errno())
}

return nil
return decodeUnknownError(int(status))
}
}

Expand Down Expand Up @@ -159,8 +154,8 @@ func Close(utmpPath, wtmpPath string, tty *os.File) error {
microsFraction := (C.int32_t)((timestamp.UnixNano() % int64(time.Second)) / int64(time.Microsecond))

accountDb.Lock()
status := C.uacc_mark_utmp_entry_dead(cUtmpPath, cWtmpPath, cTtyName, secondsElapsed, microsFraction)
accountDb.Unlock()
defer accountDb.Unlock()
status, errno := C.uacc_mark_utmp_entry_dead(cUtmpPath, cWtmpPath, cTtyName, secondsElapsed, microsFraction)

switch status {
case C.UACC_UTMP_MISSING_PERMISSIONS:
Expand All @@ -170,18 +165,13 @@ func Close(utmpPath, wtmpPath string, tty *os.File) error {
case C.UACC_UTMP_READ_ERROR:
return trace.AccessDenied("failed to read and search utmp database")
case C.UACC_UTMP_FAILED_OPEN:
code := C.get_errno()
return trace.AccessDenied("failed to open user account database, code: %d", code)
return trace.AccessDenied("failed to open user account database, code: %d", errno)
case C.UACC_UTMP_FAILED_TO_SELECT_FILE:
return trace.BadParameter("failed to select file")
case C.UACC_UTMP_PATH_DOES_NOT_EXIST:
return trace.NotFound("user accounting files are missing from the system, running in a container?")
default:
if status != 0 {
return trace.Errorf("unknown error with code %d", status)
}

return nil
return decodeUnknownError(int(status))
}
}

Expand All @@ -201,24 +191,33 @@ func UserWithPtyInDatabase(utmpPath string, username string) error {
defer C.free(unsafe.Pointer(cUsername))

accountDb.Lock()
status := C.uacc_has_entry_with_user(cUtmpPath, cUsername)
accountDb.Unlock()
defer accountDb.Unlock()
status, errno := C.uacc_has_entry_with_user(cUtmpPath, cUsername)

switch status {
case C.UACC_UTMP_FAILED_OPEN:
code := C.get_errno()
return trace.AccessDenied("failed to open user account database, code: %d", code)
return trace.AccessDenied("failed to open user account database, code: %d", errno)
case C.UACC_UTMP_ENTRY_DOES_NOT_EXIST:
return trace.NotFound("user not found")
case C.UACC_UTMP_FAILED_TO_SELECT_FILE:
return trace.BadParameter("failed to select file")
case C.UACC_UTMP_PATH_DOES_NOT_EXIST:
return trace.NotFound("user accounting files are missing from the system, running in a container?")
default:
if status != 0 {
return trace.Errorf("unknown error with code %d", status)
}
return decodeUnknownError(int(status))
}
}

func decodeUnknownError(status int) error {
if status == 0 {
return nil
}

if C.UACC_PATH_ERR != nil {
data := C.GoString(C.UACC_PATH_ERR)
C.free(unsafe.Pointer(C.UACC_PATH_ERR))
return trace.Errorf("unknown error with code %d and data %v", status, data)
}

return trace.Errorf("unknown error with code %d", status)
}