8000 Add perms test to Qpsmtpd::DB::File::DBM::dir() by jaredj · Pull Request #234 · smtpd/qpsmtpd · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add perms test to Qpsmtpd::DB::File::DBM::dir() #234

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

Jump to bottom
Merged
merged 4 commits into from
M 8000 ar 10, 2015
Merged

Add perms test to Qpsmtpd::DB::File::DBM::dir() #234

merged 4 commits into from
Mar 10, 2015

Conversation

jaredj
Copy link
Contributor
@jaredj jaredj commented Feb 24, 2015

No description provided.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 49.11% when pulling 2740f06 on jaredj:writeable-dir into 573f7c5 on smtpd:master.

@jaredj
Copy link
Contributor Author
jaredj commented Feb 25, 2015

This is ready except for the question of what to do when one of the default 'candidate directory' locations exists but is not writeable. We currently skip such a directory with no warning. It has been suggested to warn on this condition, however because of the (perhaps convoluted) way that we search for valid directories, this could lead to problems:

  1. static directories /dir1/ and /dir2/ are being searched
  2. /dir1/ has been unwriteable for months, so /dir2/greylist.dbm is being used
  3. the admin notices a warning about /dir1/ being unwriteable, and "fixes" it with a chmod
  4. the greylist plugin begins ignoring /dir2/greylist.dbm, creating a fresh new database in /dir1/, and suddenly greylisting a bunch of new systems and potentially causing mail flow interruption

@msimerson
Copy link
Member

It has been suggested to warn on this condition, however because of the (perhaps convoluted) way that we search for valid directories, this could lead to problems:

Then warn of that possibility. Not warning about existing (possibly used) dirs and letting the sysadmin do something more intelligent than you can automate is likely the type of silent error that's could lead to the scenario you describe.

@jaredj
Copy link
Contributor Author
jaredj commented Mar 9, 2015

I think I can come up with something.

@jaredj
Copy link
Contributor Author
jaredj commented Mar 10, 2015

Something seems to be broken with Coveralls preventing Travis from functioning correctly. This does pass manual travis, though

msimerson added a commit that referenced this pull request Mar 10, 2015
@msimerson
Merge pull request #234 from jaredj/writeable-dir
11a9154 
Add perms test to Qpsmtpd::DB::File::DBM::dir()
@msimerson msimerson merged commit 11a9154 into smtpd:master Mar 10, 2015
@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 49.23% when pulling 88c55ed on jaredj:writeable-dir into 1965006 on smtpd:master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jaredj @coveralls @msimerson
0