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

Add "objectstore.umask" configuration option for file/directory creation #566

Merged
merged 1 commit into from
Jan 19, 2021

Conversation

nomis
Copy link
Contributor

@nomis nomis commented Aug 10, 2020

This defaults to 0077 to preserve the existing behaviour. It can be set to
0027 to allow users in the same group to backup the token files.

It does not attempt to modify the process umask (because this is a library)
so it can only apply in addition to whatever umask the process has.

Fixes #560 after #40 restricted all new files to the current user.

This defaults to 0077 to preserve the existing behaviour. It can be set to
0027 to allow users in the same group to backup the token files.

It does not attempt to modify the process umask (because this is a library)
so it can only apply in addition to whatever umask the process has.

Fixes opendnssec#560 after opendnssec#40 restricted all new files to the current user.
@rijswijk
Copy link
Contributor

Thanks for the PR! I performed a first review, looks good at first glance, requesting a review from @halderen and asking him to merge if he also thinks the code is OK.

@rijswijk rijswijk requested a review from halderen August 11, 2020 06:30
@nomis
Copy link
Contributor Author

nomis commented Aug 11, 2020

Watch out for the File constructor when reviewing because C++ allows the umask argument to be passed a bool.

@halderen
Copy link
Member

Nice contribution. Although most times the shell umask is used, I could see the this be overridden.
A generic comment I have is that all options in the configuration file are more-or-less mandatory at the moment, causing warnings if they're not explicitly set. I need (in a separately pull request) to change this.

@halderen halderen merged commit 8f5e4e4 into opendnssec:develop Jan 19, 2021
nomis added a commit to nomis/SoftHSMv2 that referenced this pull request Aug 16, 2022
The "true" in the call to Generation::create() in OSToken::OSToken() is
used as the umask when it's supposed to be the isToken value (opendnssec#566).

Remove the default value from isToken because it's dangerous and there are
only two callers. Explicitly pass "true" and "false" for isToken.

Failing to consider this a token generation file means that the value is
never refreshed for read-only operations. All objects are reloaded from
disk every time one of them is refreshed. List operations take a long time
because all of the objects are re-read for each object.

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

Successfully merging this pull request may close these issues.

Allow file modes to be configured
3 participants