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.

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

krb5: use hidden file when creating config snippets #5825

Closed
wants to merge 1 commit into from

Conversation

sumit-bose
Copy link
Contributor

When creating config snippets fir libkrb5 SSSD first creates a temporary
file with a random suffix and renames this file after all content is
written. If this temporary file is not properly removed or renamed dur
to an error it might confuse libkrb5.

To avoid this confusion with this patch the temporary files are created
as hidden files, the name will start with a '.', which are ignored by
libkrb5.

Resolves: #5824

@alexey-tikhonov
Copy link
Member

There is ding-libs/path_utils that has relevant functions but I think it doesn't make sense to pull it in...

@@ -631,6 +631,28 @@ errno_t get_dom_names(TALLOC_CTX *mem_ctx,
return ret;
}

char *get_hidden_tmp_path(TALLOC_CTX *mem_ctx, const char *path)
{
char *s;
Copy link
Member

Choose a reason for hiding this comment

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

[This is a "bug" in C that strrchr() returns pointer to non const char. Nonetheless,] this var can and should be declared as const char *, imo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I changed it. But I think it is not a bug because it depends on the string you put into the first argument of strrchr(). If it is just a char * having char * as output is ok, since the string can be modified. I think the typical example is to split a string into 2 on a given character by replacing it with '\0'. If the strings is a const char *, as in the case here, using const char * is a good idea since you do not want to try and modify the original string.

Copy link
Member

@alexey-tikhonov alexey-tikhonov Oct 13, 2021

Choose a reason for hiding this comment

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

But if const arg is provided then function discards this const and this is a "bug".

This is a reason C++ has 2 versions:

const char* strrchr( const char* str, int ch );
char* strrchr(       char* str, int ch );

But IIUC in C this function was introduced before const was available hence it wasn't possible to "fix" due to backward compatibility issues...

When creating config snippets fir libkrb5 SSSD first creates a temporary
file with a random suffix and renames this file after all content is
written. If this temporary file is not properly removed or renamed dur
to an error it might confuse libkrb5.

To avoid this confusion with this patch the temporary files are created
as hidden files, the name will start with a '.', which are ignored by
libkrb5.

Resolves: SSSD#5824
@sumit-bose
Copy link
Contributor Author

There is ding-libs/path_utils that has relevant functions but I think it doesn't make sense to pull it in...

Hi,

yes, there are the POSIX basename() and dirname calls as well, but those and the ding-libs versions are not aware of talloc, so I thought implementing it directly would be more efficient.

bye,
Sumit

@alexey-tikhonov
Copy link
Member

Thank you, ACK.

@pbrezina
Copy link
Member

Pushed PR: #5825

  • master
    • 7941271 - krb5: use hidden file when creating config snippets

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SSSD should use "hidden" temporary file in its krb locator
3 participants