We should just run through this and make sure instances can't adjust path settings for this stuff, and that it's all forced to some reasonable, namespaced path.
Description
Revisions and Commits
Restricted Diffusion Commit | |||
rP Phabricator | |||
D11764 | rPe5b402d13f76 Lock all reply-handler options in the upstream, plus cookie prefix | ||
D11763 | rPebebeb8f7cb7 Upgrade "masked" config to "hidden" |
Event Timeline
It would probably be good if we both did a pass through this and compared notes. Basically:
- Look through all the config options.
- For options which are not "Locked", can any be set to something which would cause problems with instances? An example is if two instances can set a log file to the same path.
- We set some options in PhacilitySiteSource in rSERVICES, but currently do not add any locks. Most of the options we set should have locks added if they aren't already locked.
- Broadly, for each option, can you come up with a way that users could break their installs or cause trouble by adjusting it?
Once we have a list of things that should be locked, we can trivially lock it in PhacilitySiteSource.
This table are issues where I worried about different instances having the same value...
key | concern |
asana.project-ids | I think okay but worth raising |
asana.workspace-id | I think okay but worth raising |
metamta.default-address | could break per instance reply to email functionality |
metamta.differential.reply-handler-domain | could break per instance reply to email functionality |
metamta.diffusion.reply-handler-domain | could break per instance reply to email functionality |
metamta.macro.reply-handler-domain | could break per instance reply to email functionality |
metamta.maniphest.reply-handler-domain | could break per instance reply to email functionality |
metamta.pholio.reply-handler-domain | could break per instance reply to email functionality |
metamta.reply-handler-domain | could break per instance reply to email functionality |
notification.log | log collision |
notification.pidfile | pid collision |
phd.log-directory | log collision |
phd.pid-directory | pid collision |
sms.default-sender | could break per instance reply to SMS functionality |
storage.s3.bucket | should be unique per instance probably |
This table is config that the users can basically break their instance for fun in some way right now?
key | concern |
metamta.differential.reply-handler | can't really add alternate handler |
metamta.diffusion.reply-handler | can't really add alternate handler |
metamta.maniphest.reply-handler | can't really add alternate handler |
metamta.package.reply-handler | can't really add alternate handler |
Here's my list. We should probably lock these:
asana.project-ids
asana.workspace-id
celerity.minify
celerity.resource-hash
metamta.mail-adapter
phd.variant-config
phd.trace
phd.verbose
search.engine-selector
sms.default-adapter
sms.default-sender
storage.mysql-engine.max-size
syntax-highlighter.engine
...and probably adjust and lock these:
diffusion.ssh-port
diffusion.ssh-user
files.enable-imagemagick
log.access.path
log.ssh.path
metamta.default-address
metamta.domain
metamta.single-reply-handler-prefix
notification.client-uri
notification.log
notification.pidfile
notification.server-uri
notification.ssl-cert
notification.ssl-key
phabricator.production-uri
phd.log-directory
phd.pid-directory
phd.start-taskmasters
storage.engine-selector
storage.s3.bucket
storage.upload-size-limit
...and adjust (but not lock) this:
notification.enabled
...and maybe lock these some day although they seem fine for now:
debug.profile-rate
debug.stop-on-redirect
debug.time-limit
...and probably lock these in the upstream:
*.reply-handler (on the way out, not very useful anyway)
*.reply-handler-domain (could be used by a compromised administrator account to trick users to replying to mail and sending it to an attacker)
phabricator.cookie-prefix (can kind-of-break installs)
Lists look pretty similar, I was just a little more aggressive. A couple things are blocked on S3/Aphlict. I agree on Asana being not-dangerous but think it's worth locking to avoid confusion.