Opened 10 years ago

Closed 10 years ago

#139 closed enhancement (fixed)

review: copy environment variables from bind10 to children

Reported by: jinmei Owned by: jinmei
Priority: low Milestone:
Component: Unclassified Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity:
Sub-Project: Feature Depending on Ticket:
Estimated Difficulty: Add Hours to Ticket:
Total Hours: Internal?:

Description

Please review r1689. I was not sure if we limit environment variables for children spawn from the bind10 process, and actually found it inconvement when I wanted to specify LD_LIBRARY_PATH as a global environment variable.

This patch fixes this problem. Unless there's a reason for not doing so I suggest we adopt this change.

Giving it to Shane.

Subtickets

Change History (2)

comment:1 follow-up: Changed 10 years ago by shane

  • Owner changed from shane to jinmei
  • Status changed from new to assigned

The intention was to run children with only environment variables that we set. This is in keeping with practice of things like cron. My thinking was that various types of attack are perhaps possible by running with a bogus environment - like setting strange LD_LIBRARY_PATH. ;)

However, I think we need to look at the BIND 10 security model in greater depth. Probably something to discuss in person and make an actual deliverable. So, rather than making the software harder for us to use for a poorly-thought-out "security" feature, go ahead and merge this change.

comment:2 in reply to: ↑ 1 Changed 10 years ago by jinmei

  • Resolution set to fixed
  • Status changed from assigned to closed

Replying to shane:

However, I think we need to look at the BIND 10 security model in greater depth. Probably something to discuss in person and make an actual deliverable. So, rather than making the software harder for us to use for a poorly-thought-out "security" feature, go ahead and merge this change.

Okay. Then I'll simply close this ticket. (It is already *merged* in that it's been committed to the trunk. I have a question about the relationship among "patch", "review", "merge", "trunk", "release", etc, as I quickly raised this morning/afternoon/night, but that's a different and more generic issue)

Now, commenting on the original rationale:

The intention was to run children with only environment variables that we set. This is in keeping with practice of things like cron. My thinking was that various types of attack are perhaps possible by running with a bogus environment - like setting strange LD_LIBRARY_PATH. ;)

I don't think this trick buys much, if any, wrt security. With this logic we couldn't even let shells delegate environment varialbes to child processes, could we? After all, if a malicious guy tries to do some evil thing by setting LD_LIBRARY_PATH or whatever to a strange value and passing it to b10-xxx processes via the boss process, that person already has sufficient privilege to do that even with the previous model of env handling. Besides, bind10 is "hackable" by default (right?) so the evil person could even rewrite or develop their own evil version of the boss program.

I didn't know cron strictly limits inheritable env variables or whether it's for "security" purposes, but that might be because the cron daemon is supposed to invoke arbitrary program, often changing the owner of the process from a super user to an arbitrary normal user. The relationship of the BIND10 boss process and other b10-xxx is pretty much different from this: invokable programs are generally fixed and hardcoded, and there would only be a few known variations for the owner of the child processes (a super user + user "bind").

If we refer to other applications for an analogy, I think postfix is more appropriate than cron. And, from a quick study of its source code the master process of postfix doesn't filter any environment variables for its children.

Note: See TracTickets for help on using tickets.