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

Make sure tools never create config.ldb #7551

Closed
wants to merge 4 commits into from
Closed

Make sure tools never create config.ldb #7551

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ce0b070
TOOLS: skip confdb_init if no context ptr provided
alexey-tikhonov Aug 30, 2024
c78f328
TOOLS: get rid of code duplication
alexey-tikhonov Aug 30, 2024
12d38cc
TOOLS: use `sss_tool_confdb_init()` everywhere
alexey-tikhonov Aug 30, 2024
0c3d9a0
CONFDB: move sanity check
alexey-tikhonov Sep 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 6 additions & 35 deletions src/tools/common/sss_tools.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,17 +87,15 @@ static void sss_tool_common_opts(struct sss_tool_ctx *tool_ctx,
poptFreeContext(pc);
}

static errno_t sss_tool_confdb_init(TALLOC_CTX *mem_ctx,
struct confdb_ctx **_confdb)
errno_t sss_tool_confdb_init(TALLOC_CTX *mem_ctx, struct confdb_ctx **_confdb)
{
struct confdb_ctx *confdb;
char *path;
static const char *path = DB_PATH"/"CONFDB_FILE;
errno_t ret;
struct stat statbuf;

path = talloc_asprintf(mem_ctx, "%s/%s", DB_PATH, CONFDB_FILE);
if (path == NULL) {
return ENOMEM;
if (_confdb == NULL) {
DEBUG(SSSDBG_FATAL_FAILURE, "Bad argument\n");
return EFAULT;
}

ret = stat(path, &statbuf);
Expand All @@ -108,18 +106,13 @@ static errno_t sss_tool_confdb_init(TALLOC_CTX *mem_ctx,
return ret;
}

ret = confdb_init(mem_ctx, &confdb, path);
talloc_zfree(path);
ret = confdb_init(mem_ctx, _confdb, path);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not directly related to you commit.

confdb_init() is assuming its cdb_ctx parameter is not NULL. It clearly is not here, but could be in other calls. Shouldn't we also check inside this function that it is not NULL? Or maybe just move the check to that function?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we also check inside this function that it is not NULL?

this == sss_tool_confdb_init()?

But here is the check in place at the beginning (line 97)
Or do I misunderstand?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, sorry. this == confdb_init().

In other words, sss_tool_confdb_init() is checking _confdb != NULL, but other functions are also calling confdb_init(), and those functions may or may not check that condition. It might be better to move the check inside confdb_init().

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, added one more patch.

if (ret != EOK) {
DEBUG(SSSDBG_FATAL_FAILURE, "Unable to connect to config DB [%d]: %s\n",
ret, sss_strerror(ret));
return ret;
}

if (_confdb != NULL) {
*_confdb = confdb;
}

return EOK;
}

Expand Down Expand Up @@ -599,25 +592,3 @@ errno_t sss_tool_parse_name(TALLOC_CTX *mem_ctx,

return ret;
}

errno_t sss_tool_connect_to_confdb(TALLOC_CTX *ctx, struct confdb_ctx **cdb_ctx)
{
int ret;
char *confdb_path = NULL;

confdb_path = talloc_asprintf(ctx, "%s/%s", DB_PATH, CONFDB_FILE);
if (confdb_path == NULL) {
DEBUG(SSSDBG_CRIT_FAILURE,
"Could not allocate memory for confdb path\n");
return ENOMEM;
}

ret = confdb_init(ctx, cdb_ctx, confdb_path);
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE,
"Could not initialize connection to the confdb\n");
}

talloc_free(confdb_path);
return ret;
}
2 changes: 1 addition & 1 deletion src/tools/common/sss_tools.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,6 @@ errno_t sss_tool_parse_name(TALLOC_CTX *mem_ctx,
struct sss_domain_info **_domain);


errno_t sss_tool_connect_to_confdb(TALLOC_CTX *ctx, struct confdb_ctx **cdb_ctx);
errno_t sss_tool_confdb_init(TALLOC_CTX *mem_ctx, struct confdb_ctx **_confdb);

#endif /* SRC_TOOLS_COMMON_SSS_TOOLS_H_ */
16 changes: 4 additions & 12 deletions src/tools/sss_cache.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

#include "util/util.h"
#include "tools/tools_util.h"
#include "tools/common/sss_tools.h"
#include "db/sysdb.h"
#include "db/sysdb_services.h"
#include "db/sysdb_autofs.h"
Expand Down Expand Up @@ -661,22 +662,13 @@ static errno_t invalidate_entry(TALLOC_CTX *ctx,
static errno_t init_domains(struct cache_tool_ctx *ctx,
const char *domain)
{
char *confdb_path;
int ret;
struct sss_domain_info *dinfo;

confdb_path = talloc_asprintf(ctx, "%s/%s", DB_PATH, CONFDB_FILE);
if (confdb_path == NULL) {
return ENOMEM;
}

/* Connect to the conf db */
ret = confdb_init(ctx, &ctx->confdb, confdb_path);
talloc_free(confdb_path);
ret = sss_tool_confdb_init(ctx, &ctx->confdb);
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE,
"Could not initialize connection to the confdb\n");
return ret;
ERROR("Can't find configuration db, was SSSD configured and run?\n");
return ERR_NO_DOMAIN_ENABLED;
}

if (domain) {
Expand Down
11 changes: 2 additions & 9 deletions src/tools/sss_seed.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include "util/util.h"
#include "db/sysdb.h"
#include "tools/tools_util.h"
#include "tools/common/sss_tools.h"
#include "confdb/confdb.h"

#ifndef BUFSIZE
Expand Down Expand Up @@ -628,7 +629,6 @@ static int seed_init_db(TALLOC_CTX *mem_ctx,
struct sysdb_ctx **_sysdb)
{
TALLOC_CTX *tmp_ctx = NULL;
char *confdb_path = NULL;
struct confdb_ctx *confdb = NULL;
struct sss_domain_info *domain = NULL;
int ret = EOK;
Expand All @@ -639,14 +639,7 @@ static int seed_init_db(TALLOC_CTX *mem_ctx,
goto done;
}

/* setup confdb */
confdb_path = talloc_asprintf(tmp_ctx, "%s/%s", DB_PATH, CONFDB_FILE);
if (confdb_path == NULL) {
ret = ENOMEM;
goto done;
}

ret = confdb_init(tmp_ctx, &confdb, confdb_path);
ret = sss_tool_confdb_init(tmp_ctx, &confdb);
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE,
"Could not initialize connection to the confdb\n");
Expand Down
2 changes: 1 addition & 1 deletion src/tools/sssctl/sssctl_data.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ static errno_t sssctl_cache_index_action(enum sysdb_index_actions action,

if (domains == NULL) {
/* If the user selected no domain, act on all of them */
ret = sss_tool_connect_to_confdb(tmp_ctx, &confdb);
ret = sss_tool_confdb_init(tmp_ctx, &confdb);
if (ret != EOK) {
DEBUG(SSSDBG_OP_FAILURE,
"Could not connect to configuration database.\n");
Expand Down
2 changes: 1 addition & 1 deletion src/tools/sssctl/sssctl_logs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,7 @@ errno_t sssctl_debug_level(struct sss_cmdline *cmdline,
targets = get_targets(ctx, pc_services, pc_domains);
CHECK(targets == NULL, fini, "Could not allocate memory.");

ret = sss_tool_connect_to_confdb(ctx, &ctx->confdb);
ret = sss_tool_confdb_init(ctx, &ctx->confdb);
CHECK(ret != EOK, fini, "Could not connect to configuration database.");

ret = get_confdb_sections(ctx, ctx->confdb, &ctx->sections);
Expand Down
Loading