Skip to content

Commit

Permalink
Fixed issue where the work folder was not properly being set (#575)
Browse files Browse the repository at this point in the history
* Fixed issue where the work folder was not properly being set

* Fixed misleading thread coming down to the agent

* Addressing bad initializer function

* Fixed unit test failing due to bad agent call

* Fixed bad access for workflow handle

* Seperating out base directory goals depending on utility

* Fixed bad error handling in workflow utils

* Responded to PR feedback

* Adding limits.h to aducpal to cover windows scenarios

* addressed pr comments

* Responded to PR feedback

* Removed unnecessary / optimistic logging operations

* Moved PathUtils to using string format with buffer safety checks
  • Loading branch information
nihemstr committed Dec 14, 2023
1 parent b0fabac commit 9b088ad
Show file tree
Hide file tree
Showing 26 changed files with 1,058 additions and 426 deletions.
2 changes: 1 addition & 1 deletion CMakeLists.txt
Expand Up @@ -216,7 +216,7 @@ endif ()

set (
ADUC_ROOTKEY_STORE_PATH
"${ADUC_DATA_FOLDER}/rootkeystore/"
"${ADUC_DATA_FOLDER}/rootkeystore"
CACHE STRING "Path to the folder containing the information for the local store of root keys")

set (
Expand Down
7 changes: 4 additions & 3 deletions src/agent/adu_core_interface/src/adu_core_interface.c
Expand Up @@ -446,16 +446,17 @@ void OrchestratorUpdateCallback(
Log_Debug("Processing deployment %s ...", workflowId);

ADUC_Result inProgressResult = { .ResultCode = ADUC_GeneralResult_Success, .ExtendedResultCode = 0 };
if (!ReportPreDeploymentProcessingState(propertyValue, ADUCITF_State_DeploymentInProgress, workflowData, inProgressResult))
if (!ReportPreDeploymentProcessingState(
propertyValue, ADUCITF_State_DeploymentInProgress, workflowData, inProgressResult))
{
Log_Warn("Reporting InProgress failed. Continuing processing deployment %s", workflowId);
}

// Ensure update to latest rootkey pkg, which is required for validating the update metadata.
workFolder = workflow_get_workfolder(workflowData->WorkflowHandle);
workFolder = workflow_get_root_sandbox_dir(workflowData->WorkflowHandle);
if (workFolder == NULL)
{
Log_Error("workflow_get_workfolder failed.");
Log_Error("workflow_get_root_sandbox_dir failed");
goto done;
}

Expand Down
Expand Up @@ -16,4 +16,3 @@ sudo chmod 555 -R "$target_devices_dir"

printf "#\n# Reset installed-criteria data..."
sudo rm -f -r /var/lib/adu/installedcriteria

Expand Up @@ -9,7 +9,7 @@

#include "aduc/source_update_cache_utils.h"
#include <aduc/parser_utils.h> // ADUC_FileEntity_Uninit
#include <aduc/path_utils.h> // SanitizePathSegment
#include <aduc/path_utils.h> // PathUtils_SanitizePathSegment
#include <aduc/string_c_utils.h> // IsNullOrEmpty
#include <aduc/system_utils.h> // ADUC_SystemUtils_*
#include <aduc/types/update_content.h> // ADUC_FileEntity
Expand Down Expand Up @@ -109,13 +109,13 @@ STRING_HANDLE ADUC_SourceUpdateCacheUtils_CreateSourceUpdateCachePath(
// file path format:
// {base_dir}/{provider}/{hashAlg}-{hash}

sanitizedProvider = SanitizePathSegment(provider);
sanitizedProvider = PathUtils_SanitizePathSegment(provider);
if (sanitizedProvider == NULL)
{
goto done;
}

sanitizedHashAlgorithm = SanitizePathSegment(alg);
sanitizedHashAlgorithm = PathUtils_SanitizePathSegment(alg);
if (sanitizedHashAlgorithm == NULL)
{
goto done;
Expand Down
4 changes: 2 additions & 2 deletions src/extensions/extension_manager/src/extension_manager.cpp
Expand Up @@ -23,7 +23,7 @@
#include <aduc/hash_utils.h> // for SHAversion
#include <aduc/logging.h>
#include <aduc/parser_utils.h>
#include <aduc/path_utils.h> // SanitizePathSegment
#include <aduc/path_utils.h> // PathUtils_SanitizePathSegment
#include <aduc/plugin_exception.hpp>
#include <aduc/result.h>
#include <aduc/string_c_utils.h>
Expand Down Expand Up @@ -215,7 +215,7 @@ ExtensionManager::LoadUpdateContentHandlerExtension(const std::string& updateTyp
return result;
}

ADUC::StringUtils::STRING_HANDLE_wrapper folderName{ SanitizePathSegment(updateType.c_str()) };
ADUC::StringUtils::STRING_HANDLE_wrapper folderName{ PathUtils_SanitizePathSegment(updateType.c_str()) };
if (folderName.is_null())
{
result.ExtendedResultCode = ADUC_ERC_NOMEM;
Expand Down
863 changes: 570 additions & 293 deletions src/inc/aduc/result.h

Large diffs are not rendered by default.

23 changes: 23 additions & 0 deletions src/libaducpal/inc/aducpal/limits.h
@@ -0,0 +1,23 @@
#ifndef ADUC_PAL_LIMITS_H
#define ADUC_PAL_LIMITS_H

#ifdef ADUCPAL_USE_PAL
# ifndef MAX_PATH
# define MAX_PATH 260
# endif

# include <limits.h>

# ifndef PATH_MAX
# define PATH_MAX MAX_PATH
# endif
#else
# include <limits.h>

# ifndef MAX_PATH

# define MAX_PATH PATH_MAX

# endif
#endif
#endif // ADUCPAL_GRP_H
8 changes: 4 additions & 4 deletions src/rootkey_workflow/src/rootkey_workflow.c
Expand Up @@ -100,19 +100,19 @@ ADUC_Result RootKeyWorkflow_UpdateRootKeys(const char* workflowId, const char* w

#ifndef ADUC_ENABLE_SRVC_E2E_TESTING

#ifdef ADUC_E2E_TESTING_ENABLED
# ifdef ADUC_E2E_TESTING_ENABLED
if (!rootKeyPackage.protectedProperties.isTest)
{
result.ExtendedResultCode = ADUC_ERC_ROOTKEY_PROD_PKG_ON_TEST_AGENT;
goto done;
}
#else
# else
if (rootKeyPackage.protectedProperties.isTest)
{
result.ExtendedResultCode = ADUC_ERC_ROOTKEY_TEST_PKG_ON_PROD_AGENT;
goto done;
}
#endif
# endif

#endif
if (!ADUC_SystemUtils_Exists(ADUC_ROOTKEY_STORE_PATH))
Expand All @@ -132,7 +132,7 @@ ADUC_Result RootKeyWorkflow_UpdateRootKeys(const char* workflowId, const char* w
goto done;
}

if (!ADUC_RootKeyUtility_IsUpdateStoreNeeded(fileDest, rootKeyPackageJsonString))
if (!ADUC_RootKeyUtility_IsUpdateStoreNeeded(fileDest, &rootKeyPackage))
{
// This is a success, but skips writing to local store and includes informational ERC.
result.ResultCode = ADUC_Result_RootKey_Continue;
Expand Down
5 changes: 5 additions & 0 deletions src/utils/c_utils/inc/aduc/string_c_utils.h
Expand Up @@ -16,6 +16,11 @@

EXTERN_C_BEGIN

/**
* @brief Maximum length for the output string of ADUC_StringFormat()
*/
#define ADUC_STRING_FORMAT_MAX_LENGTH 512

char* ADUC_StringUtils_Trim(char* str);
char* ADUC_StringUtils_Map(const char* src, int (*mapFn)(int));

Expand Down
9 changes: 2 additions & 7 deletions src/utils/c_utils/src/string_c_utils.c
Expand Up @@ -22,11 +22,6 @@
// keep this last to avoid interfering with system headers
#include "aduc/aduc_banned.h"

/**
* @brief Maximum length for the output string of ADUC_StringFormat()
*/
#define ADUC_STRING_FORMAT_MAX_LENGTH 512

/**
* @brief Function that sets @p strBuffers to the contents of the file at @p filePath if the contents are smaller in size than the buffer
* @param filePath path to the file who's contents will be read
Expand Down Expand Up @@ -441,7 +436,7 @@ char* ADUC_StringUtils_Map(const char* src, int (*mapFn)(int))
free(tgt);
return NULL;
}
tgt[i] = (char) ( ret & 0xFF);
tgt[i] = (char)(ret & 0xFF);
}

return tgt;
Expand All @@ -466,7 +461,7 @@ size_t ADUC_Safe_StrCopyN(char* dest, const char* src, size_t destByteLen, size_
return 0;
}

if(numSrcCharsToCopy >= destByteLen)
if (numSrcCharsToCopy >= destByteLen)
{
numSrcCharsToCopy = destByteLen - 1;
}
Expand Down
6 changes: 3 additions & 3 deletions src/utils/extension_utils/src/extension_utils.c
Expand Up @@ -10,7 +10,7 @@
#include "aduc/hash_utils.h"
#include "aduc/logging.h"
#include "aduc/parser_utils.h"
#include "aduc/path_utils.h" // SanitizePathSegment
#include "aduc/path_utils.h" // PathUtils_SanitizePathSegment
#include "aduc/string_c_utils.h"
#include "aduc/system_utils.h"

Expand Down Expand Up @@ -122,7 +122,7 @@ static bool GetHandlerExtensionFileEntity(

memset(fileEntity, 0, sizeof(*fileEntity));

STRING_HANDLE folderName = SanitizePathSegment(handlerId);
STRING_HANDLE folderName = PathUtils_SanitizePathSegment(handlerId);

STRING_HANDLE path = STRING_construct_sprintf("%s/%s/%s", extensionDir, STRING_c_str(folderName), regFileName);

Expand Down Expand Up @@ -214,7 +214,7 @@ static bool RegisterHandlerExtension(
goto done;
}

folderName = SanitizePathSegment(handlerId);
folderName = PathUtils_SanitizePathSegment(handlerId);
if (folderName == NULL)
{
Log_Error("Cannot generate a folder name from an Update Type.");
Expand Down
5 changes: 4 additions & 1 deletion src/utils/path_utils/CMakeLists.txt
Expand Up @@ -12,7 +12,10 @@ target_include_directories (${target_name} PUBLIC inc ${ADUC_EXPORT_INCLUDES})
#
set_property (TARGET ${target_name} PROPERTY POSITION_INDEPENDENT_CODE ON)

target_link_libraries (${target_name} PUBLIC aduc::c_utils)
target_link_libraries (
${target_name}
PUBLIC aduc::c_utils
PRIVATE libaducpal)

target_link_aziotsharedutil (${target_name} PRIVATE)

Expand Down
4 changes: 3 additions & 1 deletion src/utils/path_utils/inc/aduc/path_utils.h
Expand Up @@ -13,7 +13,9 @@

EXTERN_C_BEGIN

STRING_HANDLE SanitizePathSegment(const char* unsanitized);
STRING_HANDLE PathUtils_SanitizePathSegment(const char* unsanitized);

char* PathUtils_ConcatenateDirAndFolderPaths(const char* dirPath, const char* folderName);

EXTERN_C_END

Expand Down
62 changes: 61 additions & 1 deletion src/utils/path_utils/src/path_utils.c
Expand Up @@ -11,13 +11,16 @@
#include <azure_c_shared_utility/crt_abstractions.h> // for mallocAndStrcpy_s
#include <ctype.h> // for isalnum

#include <aducpal/limits.h> // for PATH_MAX

#define STR_PATH_MAX PATH_MAX - 1
/**
* @brief Replaces non-alphunumeric chars with _ (underscore) char for use in path segments of a file path.
* @param unsanitized The string to be sanitized
* @return STRING_HANDLE The file path string, or NULL on error.
* @details Caller owns it and must call STRING_delete() when done with it.
*/
STRING_HANDLE SanitizePathSegment(const char* unsanitized)
STRING_HANDLE PathUtils_SanitizePathSegment(const char* unsanitized)
{
char* sanitized = NULL;
STRING_HANDLE sanitizedHandle = NULL;
Expand Down Expand Up @@ -59,3 +62,60 @@ STRING_HANDLE SanitizePathSegment(const char* unsanitized)

return sanitizedHandle;
}

/**
* @brief Concatenates a directory and folder to form a path.
* @param dirPath The directory path (eg /var/lib/adu)
* @param folderName The folder name (eg 12345678-1234-1234-1234-123456789012)
* @return char* The concatenated path, or NULL on error.
*/
char* PathUtils_ConcatenateDirAndFolderPaths(const char* dirPath, const char* folderName)
{
char* ret = NULL;
char* tempRet = NULL;
const char* file_delimeter = "/";
size_t file_delimeter_len = 1; /* can use strlen since this is a known good string */

if (IsNullOrEmpty(dirPath) || IsNullOrEmpty(folderName))
{
goto done;
}

size_t dirPathLen = ADUC_StrNLen(dirPath, STR_PATH_MAX);

if (dirPathLen == 0 || dirPathLen == STR_PATH_MAX)
{
goto done;
}

if (dirPath[dirPathLen - 1] == '/')
{
file_delimeter_len = 0;
}

const size_t folderPathLen = ADUC_StrNLen(folderName, STR_PATH_MAX);

if (folderPathLen == 0 || folderPathLen == STR_PATH_MAX)
{
goto done;
}

if (dirPathLen + folderPathLen + file_delimeter_len > STR_PATH_MAX)
{
goto done;
}

if (file_delimeter_len)
{
tempRet = ADUC_StringFormat("%s%s%s", dirPath, file_delimeter, folderName);
}
else
{
tempRet = ADUC_StringFormat("%s%s", dirPath, folderName);
}

done:
ret = tempRet;

return ret;
}

0 comments on commit 9b088ad

Please sign in to comment.