Skip to content

Commit

Permalink
feat: Improving secure file error messages
Browse files Browse the repository at this point in the history 
Further improvement to the error messages from the POSIX secure file. This will provide the user with better information about how to correct the situation and when possible this function will also provide recommendations for commands to run (chown or chmod) as necessary.

[Seagate/openSeaChest#161]

Signed-off-by: Tyler Erickson <tyler.erickson@seagate.com>
  • Loading branch information
@vonericsen
vonericsen committed Oct 28, 2024
1 parent ee46bc8 commit a5dc85f
Showing 1 changed file with 29 additions and 19 deletions.
48 changes: 29 additions & 19 deletions src/posix_secure_file.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,22 +190,22 @@ static bool internal_OS_Is_Directory_Secure(const char* fullpath, unsigned int n
if (!fullpath || fullpath[0] != '/')
{
/* Handle error */
set_dir_security_output_error_message(outputError, "Full path does not start with /\n");
set_dir_security_output_error_message(outputError, "Error: Full path must start with \"/\".\n");
return false;
}

if (num_symlinks > MAX_SYMLINKS_IN_PATH)
{
/* Could be a symlink loop */
/* Handle error */
set_dir_security_output_error_message(outputError, "Too many symlinks (must be less than %d links)\n", MAX_SYMLINKS_IN_PATH);
set_dir_security_output_error_message(outputError, "Error: Too many symbolic links (must be fewer than %d links)\n", MAX_SYMLINKS_IN_PATH);
return false;
}

if (!(path_copy = strdup(fullpath)))
{
/* Handle error */
set_dir_security_output_error_message(outputError, "Cannot strdup path (out of memory?)\n");
set_dir_security_output_error_message(outputError, "Error: Unable to duplicate fullpath to path copy: %s (Out of memory)\n", fullpath);
return false;
}

Expand All @@ -226,7 +226,7 @@ static bool internal_OS_Is_Directory_Secure(const char* fullpath, unsigned int n
if (num_of_dirs == SSIZE_MAX)
{
/* out of room to compare this many directories deep */
set_dir_security_output_error_message(outputError, "Too many directories deep to check permissions\n");
set_dir_security_output_error_message(outputError, "Error: The path contains too many directories. Please reduce the number of directories in the path and try again. Must be less than %zd directories in the path.\n", SSIZE_MAX);
return false;
}
/* Now num_of_dirs indicates # of dirs we must check */
Expand All @@ -235,22 +235,22 @@ static bool internal_OS_Is_Directory_Secure(const char* fullpath, unsigned int n
if (!(dirs = C_CAST(char**, safe_malloc(C_CAST(size_t, num_of_dirs) * sizeof(char*)))))
{
/* Handle error */
set_dir_security_output_error_message(outputError, "Cannot allocate memory for all directories needed to be checked\n");
set_dir_security_output_error_message(outputError, "Error: Unable to allocate memory for directory validation. Please ensure sufficient memory is available and try again.\n");
return false;
}

if (!(dirs[num_of_dirs - 1] = strdup(fullpath)))
{
/* Handle error */
set_dir_security_output_error_message(outputError, "Cannot strdup fullpath into dirs array: %s\n", fullpath);
set_dir_security_output_error_message(outputError, "Error: Unable to duplicate fullpath into dirs array: %s (Out of memory)\n", fullpath);
safe_free(dirs);
return false;
}

if (!(path_copy = strdup(fullpath)))
{
/* Handle error */
set_dir_security_output_error_message(outputError, "Cannot strdup fullpath to path copy: %s\n", fullpath);
set_dir_security_output_error_message(outputError, "Error: Unable to duplicate fullpath to path copy: %s (Out of memory)\n", fullpath);
safe_free(dirs);
return false;
}
Expand All @@ -263,7 +263,7 @@ static bool internal_OS_Is_Directory_Secure(const char* fullpath, unsigned int n
if (!(dirs[i] = strdup(path_parent)))
{
/* Handle error */
set_dir_security_output_error_message(outputError, "Cannot dup path parent: %s\n", path_parent);
set_dir_security_output_error_message(outputError, "Error: Unable to duplicate path parent: %s (Out of memory)\n", path_parent);
secure = false;
break;
}
Expand Down Expand Up @@ -296,7 +296,7 @@ static bool internal_OS_Is_Directory_Secure(const char* fullpath, unsigned int n
if (lstat(dirs[i], &buf) != 0)
{
/* Handle error */
set_dir_security_output_error_message(outputError, "Failed to read file status for %s\n", dirs[i]);
set_dir_security_output_error_message(outputError, "Error: Failed to read file status for %s. This operation is necessary to retrieve ownership and permission details. Please check the path and ensure you have the required permissions.\n", dirs[i]);
secure = false;
break;
}
Expand All @@ -309,31 +309,31 @@ static bool internal_OS_Is_Directory_Secure(const char* fullpath, unsigned int n
{
/* Handle error */
secure = false;
set_dir_security_output_error_message(outputError, "Bad link size for %s\n", dirs[i]);
set_dir_security_output_error_message(outputError, "Error: Invalid link size for %s. The size of the symbolic link is negative, which indicates a potential filesystem issue.\n", dirs[i]);
break;
}
linksize = buf.st_size + 1;
if (!(link = C_CAST(char*, safe_malloc(C_CAST(size_t, linksize)))))
{
/* Handle error */
secure = false;
set_dir_security_output_error_message(outputError, "link cannot allocate to read origin location\n");
set_dir_security_output_error_message(outputError, "Error: Unable to allocate memory to read the link for %s. (Out of memory)\n", dirs[i]);
break;
}

r = readlink(dirs[i], link, C_CAST(size_t, linksize));
if (r == -1)
{
/* Handle error */
set_dir_security_output_error_message(outputError, "readlink failed: %s\n", dirs[i]);
set_dir_security_output_error_message(outputError, "Error: Failed to read the symbolic link for %s. Please check the path and ensure the link exists and is accessible.\n", dirs[i]);
secure = false;
safe_free(&link);
break;
}
else if (r >= linksize)
{
/* Handle truncation error */
set_dir_security_output_error_message(outputError, "link truncated: %s\n", dirs[i]);
set_dir_security_output_error_message(outputError, "Error: The symbolic link for %s is truncated. The link is too long to be read completely.\n", dirs[i]);
secure = false;
safe_free(&link);
break;
Expand Down Expand Up @@ -361,7 +361,7 @@ static bool internal_OS_Is_Directory_Secure(const char* fullpath, unsigned int n
if (!S_ISDIR(buf.st_mode))
{
/* Not a directory */
set_dir_security_output_error_message(outputError, "not a directory, cannot verify %s for secure path\n", dirs[i]);
set_dir_security_output_error_message(outputError, "Error: %s is not a directory. Cannot verify for secure path.\n", dirs[i]);
secure = false;
break;
}
Expand All @@ -383,19 +383,29 @@ static bool internal_OS_Is_Directory_Secure(const char* fullpath, unsigned int n
#if defined (_DEBUG)
printf("UID detected: %u\n", sudouid);
#endif
if (sudouid != ROOT_UID_VAL && buf.st_uid != sudouid)
if (sudouid != ROOT_UID_VAL)
{
if (buf.st_uid != sudouid)
{
/* Directory is owned by someone besides user or root */
secure = false;
set_dir_security_output_error_message(outputError, "Error: Directory (%s) owned by someone other than user or root. Owner: %u Current User: %u. Recommended action: \"chown %u:%u %s\"\n", dirs[i], buf.st_uid, sudouid, sudouid, sudouid, dirs[i]);
break;
}
}
else
{
/* Directory is owned by someone besides user or root */
secure = false;
set_dir_security_output_error_message(outputError, "Directory (%s) owned by someone other than user or root. Owner: %u Current User: %u\n", dirs[i], buf.st_uid, sudouid);
set_dir_security_output_error_message(outputError, "Error: Directory (%s) owned by someone other than user or root. Owner: %u Current User: %u. Recommended action: \"chown %u:%u %s\"\n", dirs[i], buf.st_uid, my_uid, my_uid, my_uid, dirs[i]);
break;
}
}
else
{
/* Directory is owned by someone besides user or root */
secure = false;
set_dir_security_output_error_message(outputError, "Directory (%s) owned by someone other than user or root. Owner: %u Current User: %u\n", dirs[i], buf.st_uid, my_uid);
set_dir_security_output_error_message(outputError, "Error: Directory (%s) owned by someone other than user or root. Owner: %u Current User: %u. Recommended action: \"chown %u:%u %s\"\n", dirs[i], buf.st_uid, my_uid, my_uid, my_uid, dirs[i]);
break;
}
}
Expand All @@ -407,11 +417,11 @@ static bool internal_OS_Is_Directory_Secure(const char* fullpath, unsigned int n
secure = false;
if (buf.st_mode & S_IWGRP)
{
set_dir_security_output_error_message(outputError, "Directory (%s) writable by others. Disable write permissions for groups\n", dirs[i]);
set_dir_security_output_error_message(outputError, "Error: Directory (%s) writable by group. Disable write permissions for groups. Recommended action: \"chmod u=rwx,g=rx,o=rx %s\"\n", dirs[i], dirs[i]);
}
else
{
set_dir_security_output_error_message(outputError, "Directory (%s) writable by others. Disable write permissions for others\n", dirs[i]);
set_dir_security_output_error_message(outputError, "Error: Directory (%s) writable by others. Disable write permissions for others. Recommended action: \"chmod u=rwx,g=rx,o=rx %s\"\n", dirs[i], dirs[i]);
}
break;
}
Expand Down

0 comments on commit a5dc85f

Please sign in to comment.