Skip to content

Commit afaef98

Browse files
rlillbackrlillback
committed
Fixed logic error in validating management job configuration
1 parent 70dc5be commit afaef98

1 file changed

Lines changed: 35 additions & 57 deletions

File tree

‎management.c‎

Lines changed: 35 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -188,24 +188,18 @@ static int add_cert_to_store(const char *storePath, const char *certASCII,
188188
log_trace("%s::%s(%d) : Creating a new PemInventoryItem for certificate",
189189
LOG_INF);
190190
if (!ssl_PemInventoryItem_create(&certToAdd, certASCII)) {
191-
log_error("%s::%s(%d) : Error creating cert thumbprint or invalid cert",
192-
LOG_INF);
193-
append_linef(pMessage, "%s::%s(%d) : Error creating cert thumbprint"
194-
" or invalid cert", LOG_INF);
191+
log_error("%s::%s(%d) : Error creating cert thumbprint or invalid cert", LOG_INF);
192+
append_linef(pMessage, "%s::%s(%d) : Error creating cert thumbprint or invalid cert", LOG_INF);
195193
*pStatus = STAT_ERR;
196194
return -1;
197195
}
198-
log_trace("%s::%s(%d) : New certificate has a thumbprint of %s",
199-
LOG_INF, certToAdd->thumbprint_string);
196+
log_trace("%s::%s(%d) : New certificate has a thumbprint of %s", LOG_INF, certToAdd->thumbprint_string);
200197

201198
/* 2.) Read all the certs in the cert store */
202-
log_trace("%s::%s(%d) : Reading cert store %s's inventory",
203-
LOG_INF, storePath);
199+
log_trace("%s::%s(%d) : Reading cert store %s's inventory", LOG_INF, storePath);
204200
if (0 != ssl_read_store_inventory(storePath, NULL, &pemList)) {
205-
log_error("%s::%s(%d) : Error reading PEM store at %s",
206-
LOG_INF, storePath);
207-
append_linef(pMessage, "%s::%s(%d) : Error reading PEM store at %s",
208-
LOG_INF, storePath);
201+
log_error("%s::%s(%d) : Error reading PEM store at %s", LOG_INF, storePath);
202+
append_linef(pMessage, "%s::%s(%d) : Error reading PEM store at %s", LOG_INF, storePath);
209203
if (certToAdd) {
210204
PemInventoryItem_free(certToAdd);
211205
}
@@ -215,18 +209,15 @@ static int add_cert_to_store(const char *storePath, const char *certASCII,
215209
*pStatus = STAT_ERR;
216210
return -1;
217211
}
218-
log_trace("%s::%s(%d) : Found %d certs in store",
219-
LOG_INF, pemList->item_count);
212+
log_trace("%s::%s(%d) : Found %d certs in store", LOG_INF, pemList->item_count);
220213
for (i = 0; pemList->item_count > i; i++) {
221-
log_trace("%s::%s(%d) : Thumbprint #%d found: %s",
222-
LOG_INF, i, pemList->items[i]->thumbprint_string);
214+
log_trace("%s::%s(%d) : Thumbprint #%d found: %s", LOG_INF, i, pemList->items[i]->thumbprint_string);
223215
}
224216

225217
/* 3.) Does the cert exist in the store already? */
226218
foundCert = false;
227219
i = 0;
228-
log_trace("%s::%s(%d) : Checking if cert to add is already in the store",
229-
LOG_INF);
220+
log_trace("%s::%s(%d) : Checking if cert to add is already in the store", LOG_INF);
230221
while ((pemList->item_count > i) &&
231222
(false == foundCert) &&
232223
(certToAdd)) {
@@ -240,31 +231,26 @@ static int add_cert_to_store(const char *storePath, const char *certASCII,
240231
i++;
241232
}
242233

243-
log_verbose("%s::%s(%d) : Found cert: %s",
244-
LOG_INF, (foundCert ? "yes" : "no"));
234+
log_verbose("%s::%s(%d) : Found cert: %s", LOG_INF, (foundCert ? "yes" : "no"));
245235

246236
/* 4.) If the cert doesn't exist, add it too the store */
247237
if (false == foundCert) {
248-
log_trace("%s::%s(%d) : Adding cert with thumbprint %s to store %s",
249-
LOG_INF, certToAdd->thumbprint_string, storePath);
238+
log_trace("%s::%s(%d) : Adding cert with thumbprint %s to store %s", LOG_INF, certToAdd->thumbprint_string,
239+
storePath);
250240
if (!ssl_Store_Cert_add(storePath, certASCII)) {
251241
log_error("%s::%s(%d) Error writing cert to store", LOG_INF);
252-
append_linef(pMessage, "%s::%s(%d) Error writing cert to store",
253-
LOG_INF);
242+
append_linef(pMessage, "%s::%s(%d) Error writing cert to store", LOG_INF);
254243
*pStatus = STAT_ERR;
255244
ret = -1;
256245
} else {
257-
log_verbose("%s::%s(%d) Certificate successfully written to store",
258-
LOG_INF);
246+
log_verbose("%s::%s(%d) Certificate successfully written to store", LOG_INF);
259247
*pStatus = STAT_SUCCESS;
260248
ret = 0;
261249
}
262250
} else {
263-
log_warn("%s::%s(%d) : WARNING: Certificate with thumbprint %s "
264-
"was already present in store %s", LOG_INF,
251+
log_warn("%s::%s(%d) : WARNING: Certificate with thumbprint %s was already present in store %s", LOG_INF,
265252
certToAdd->thumbprint_string, storePath);
266-
append_linef(pMessage, "%s::%s(%d) : WARNING: Certificate with "
267-
"thumbprint %s was already present in store %s",
253+
append_linef(pMessage, "%s::%s(%d) : WARNING: Certificate with thumbprint %s was already present in store %s",
268254
LOG_INF, certToAdd->thumbprint_string, storePath);
269255
*pStatus = STAT_WARN;
270256
ret = 0;
@@ -302,10 +288,8 @@ static int remove_cert_from_store(const char *storePath,
302288
int ret = 0;
303289

304290
if (!ssl_remove_cert_from_store(storePath, searchThumb, keyPath, password)) {
305-
log_error("%s::%s(%d) : Unable to remove cert from store at %s",
306-
LOG_INF, storePath);
307-
append_linef(pMessage, "Unable to remove cert from store at %s",
308-
storePath);
291+
log_error("%s::%s(%d) : Unable to remove cert from store at %s", LOG_INF, storePath);
292+
append_linef(pMessage, "Unable to remove cert from store at %s", storePath);
309293
*pStatus = STAT_ERR;
310294
}
311295
return ret;
@@ -324,38 +308,37 @@ static int remove_cert_from_store(const char *storePath,
324308
* @param[out] statusMessage String array to append validation error messages
325309
* @return false if validation failed, false if validation passed
326310
*/
327-
static bool validate_management_store_configuration(ManagementConfigResp_t* manConf, char** statusMessage)
311+
static bool management_store_config_valid(ManagementConfigResp_t* manConf, char** statusMessage)
328312
{
329-
bool failed = false;
313+
bool valid = true;
330314
if (manConf->Job.StorePath) {
331315
/* Is the target store a directory and not a file? */
332316
if (is_directory(manConf->Job.StorePath)) {
333-
log_error("%s::%s(%d) : The store path must be a file and "
334-
"not a directory.", LOG_INF);
317+
log_error("%s::%s(%d) : The store path must be a file and not a directory.", LOG_INF);
335318
append_linef(statusMessage, "The store path must be a file and not a directory.");
336-
failed = true;
319+
valid = false;
337320
}
338321
if (ConfigData->UseAgentCert && ConfigData->AgentCert) { /* If we have an agent cert, we can't manage it through this job */
339322
/* Is the target store the Agent store? */
340323
if (0 == strcasecmp(ConfigData->AgentCert, manConf->Job.StorePath)) {
341324
log_warn("%s::%s(%d) : Attempting a Management job on the agent cert store is not allowed.", LOG_INF);
342325
append_linef(statusMessage, "Attempting a Management job the agent cert store is not allowed.");
343-
failed = true;
326+
valid = false;
344327
}
345328
}
346329
/* Verify the target store exists */
347330
if (!file_exists(manConf->Job.StorePath)) {
348331
log_warn("%s::%s(%d) : Attempting to manage a certificate store that does not exist yet.", LOG_INF);
349332
append_linef(statusMessage, "Attempting to manage a certificate store that does not exist yet.");
350-
failed = true;
333+
valid = false;
351334
}
352335
} else {
353336
log_error("%s::%s(%d) : Job doesn't contain a target store to manage.", LOG_INF);
354337
append_linef(statusMessage, "Job doesn't contain a target store to manage.");
355-
failed = true;
338+
valid = false;
356339
}
357-
return failed;
358-
} /* validate_management_store_configuration */
340+
return valid;
341+
} /* management_store_config_valid */
359342

360343
/******************************************************************************/
361344
/*********************** GLOBAL FUNCTION DEFINITIONS **************************/
@@ -384,8 +367,7 @@ int cms_job_manage(SessionJob_t * jobInfo, char *sessionToken,
384367
char *statusMessage = strdup("");
385368
enum AgentApiResultStatus status = STAT_UNK;
386369
int returnable = 0;
387-
log_info("%s::%s(%d) : Starting management job %s", LOG_INF,
388-
jobInfo->JobId);
370+
log_info("%s::%s(%d) : Starting management job %s", LOG_INF, jobInfo->JobId);
389371

390372
res = get_management_config(sessionToken, jobInfo->JobId,
391373
jobInfo->ConfigurationEndpoint, &manConf);
@@ -398,7 +380,7 @@ int cms_job_manage(SessionJob_t * jobInfo, char *sessionToken,
398380
/* Validate data */
399381
if (manConf) {
400382
/* if any test failed, then let the platform know about it. */
401-
if (false == validate_management_store_configuration(manConf, &statusMessage)) {
383+
if (false == management_store_config_valid(manConf, &statusMessage)) {
402384
ManagementCompleteResp_t *manComp = NULL;
403385
send_management_job_complete(sessionToken, jobInfo->JobId,
404386
jobInfo->CompletionEndpoint, STAT_ERR, manConf->AuditId,
@@ -445,11 +427,10 @@ int cms_job_manage(SessionJob_t * jobInfo, char *sessionToken,
445427
}
446428
break;
447429
case OP_REM:
448-
log_verbose("%s::%s(%d) : Remove certificate operation",
449-
LOG_INF);
430+
log_verbose("%s::%s(%d) : Remove certificate operation", LOG_INF);
450431
res = remove_cert_from_store(manConf->Job.StorePath,
451432
manConf->Job.Alias, manConf->Job.PrivateKeyPath,
452-
manConf->Job.StorePassword, &statusMessage, &status);
433+
manConf->Job.StorePassword, &statusMessage, &status);
453434
if (res != 0) {
454435
log_error("%s::%s(%d) : Failed to remove certificate from the store", LOG_INF);
455436
returnable = 999;
@@ -470,7 +451,7 @@ int cms_job_manage(SessionJob_t * jobInfo, char *sessionToken,
470451
ManagementCompleteResp_t *manComp = NULL;
471452
res = send_management_job_complete(sessionToken,
472453
jobInfo->JobId, jobInfo->CompletionEndpoint, status + 1,
473-
auditId, statusMessage, &manComp);
454+
auditId, statusMessage, &manComp);
474455
/* NOTE: Removed chain job due to inventory job running twice */
475456

476457
if (res != 0) {
@@ -481,15 +462,12 @@ int cms_job_manage(SessionJob_t * jobInfo, char *sessionToken,
481462
}
482463

483464
if (status >= STAT_ERR) {
484-
log_error("%s::%s(%d) : Management job %s failed with "
485-
"error: %s", LOG_INF, jobInfo->JobId, statusMessage);
465+
log_error("%s::%s(%d) : Management job %s failed with error: %s", LOG_INF, jobInfo->JobId, statusMessage);
486466
returnable = 999;
487467
} else if (status == STAT_WARN) {
488-
log_warn("%s::%s(%d) : Management job %s completed"
489-
" with warning: %s", LOG_INF, jobInfo->JobId, statusMessage);
468+
log_warn("%s::%s(%d) : Management job %s completed with warning: %s", LOG_INF, jobInfo->JobId, statusMessage);
490469
} else {
491-
log_info("%s::%s(%d) : Management job %s completed"
492-
" successfully", LOG_INF, jobInfo->JobId);
470+
log_info("%s::%s(%d) : Management job %s completed successfully", LOG_INF, jobInfo->JobId);
493471
}
494472

495473
ManagementCompleteResp_free(manComp);

0 commit comments

Comments
 (0)