Always use the Darwin sandbox
Even with "build-use-sandbox = false", we now use sandboxing with a permissive profile that allows everything except the creation of setuid/setgid binaries.
This commit is contained in:
		
							parent
							
								
									d3f780996c
								
							
						
					
					
						commit
						85e93d7b87
					
				
					 5 changed files with 100 additions and 87 deletions
				
			
		
							
								
								
									
										4
									
								
								.gitignore
									
										
									
									
										vendored
									
									
								
							
							
						
						
									
										4
									
								
								.gitignore
									
										
									
									
										vendored
									
									
								
							|  | @ -48,9 +48,7 @@ perl/Makefile.config | ||||||
| /src/libexpr/nix.tbl | /src/libexpr/nix.tbl | ||||||
| 
 | 
 | ||||||
| # /src/libstore/ | # /src/libstore/ | ||||||
| /src/libstore/schema.sql.gen.hh | /src/libstore/*.gen.hh | ||||||
| /src/libstore/sandbox-defaults.sb.gen.hh |  | ||||||
| /src/libstore/sandbox-network.sb.gen.hh |  | ||||||
| 
 | 
 | ||||||
| /src/nix/nix | /src/nix/nix | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -2611,104 +2611,102 @@ void DerivationGoal::runChild() | ||||||
| 
 | 
 | ||||||
|         const char *builder = "invalid"; |         const char *builder = "invalid"; | ||||||
| 
 | 
 | ||||||
|         string sandboxProfile; |  | ||||||
|         if (drv->isBuiltin()) { |         if (drv->isBuiltin()) { | ||||||
|             ; |             ; | ||||||
|         } |         } | ||||||
| #if __APPLE__ | #if __APPLE__ | ||||||
|         else if (useChroot) { |         else { | ||||||
|             /* Lots and lots and lots of file functions freak out if they can't stat their full ancestry */ |             /* This has to appear before import statements. */ | ||||||
|             PathSet ancestry; |             std::string sandboxProfile = "(version 1)\n"; | ||||||
| 
 | 
 | ||||||
|             /* We build the ancestry before adding all inputPaths to the store because we know they'll
 |             if (useChroot) { | ||||||
|                all have the same parents (the store), and there might be lots of inputs. This isn't | 
 | ||||||
|                particularly efficient... I doubt it'll be a bottleneck in practice */ |                 /* Lots and lots and lots of file functions freak out if they can't stat their full ancestry */ | ||||||
|             for (auto & i : dirsInChroot) { |                 PathSet ancestry; | ||||||
|                 Path cur = i.first; | 
 | ||||||
|                 while (cur.compare("/") != 0) { |                 /* We build the ancestry before adding all inputPaths to the store because we know they'll
 | ||||||
|                     cur = dirOf(cur); |                    all have the same parents (the store), and there might be lots of inputs. This isn't | ||||||
|                     ancestry.insert(cur); |                    particularly efficient... I doubt it'll be a bottleneck in practice */ | ||||||
|  |                 for (auto & i : dirsInChroot) { | ||||||
|  |                     Path cur = i.first; | ||||||
|  |                     while (cur.compare("/") != 0) { | ||||||
|  |                         cur = dirOf(cur); | ||||||
|  |                         ancestry.insert(cur); | ||||||
|  |                     } | ||||||
|                 } |                 } | ||||||
|             } |  | ||||||
| 
 | 
 | ||||||
|             /* And we want the store in there regardless of how empty dirsInChroot. We include the innermost
 |                 /* And we want the store in there regardless of how empty dirsInChroot. We include the innermost
 | ||||||
|                path component this time, since it's typically /nix/store and we care about that. */ |                    path component this time, since it's typically /nix/store and we care about that. */ | ||||||
|             Path cur = worker.store.storeDir; |                 Path cur = worker.store.storeDir; | ||||||
|             while (cur.compare("/") != 0) { |                 while (cur.compare("/") != 0) { | ||||||
|                 ancestry.insert(cur); |                     ancestry.insert(cur); | ||||||
|                 cur = dirOf(cur); |                     cur = dirOf(cur); | ||||||
|             } |                 } | ||||||
| 
 | 
 | ||||||
|             /* Add all our input paths to the chroot */ |                 /* Add all our input paths to the chroot */ | ||||||
|             for (auto & i : inputPaths) |                 for (auto & i : inputPaths) | ||||||
|                 dirsInChroot[i] = i; |                     dirsInChroot[i] = i; | ||||||
| 
 | 
 | ||||||
|             /* This has to appear before import statements */ |                 /* Violations will go to the syslog if you set this. Unfortunately the destination does not appear to be configurable */ | ||||||
|             sandboxProfile += "(version 1)\n"; |                 if (settings.darwinLogSandboxViolations) { | ||||||
|  |                     sandboxProfile += "(deny default)\n"; | ||||||
|  |                 } else { | ||||||
|  |                     sandboxProfile += "(deny default (with no-log))\n"; | ||||||
|  |                 } | ||||||
| 
 | 
 | ||||||
|             /* Violations will go to the syslog if you set this. Unfortunately the destination does not appear to be configurable */ |  | ||||||
|             if (settings.darwinLogSandboxViolations) { |  | ||||||
|                 sandboxProfile += "(deny default)\n"; |  | ||||||
|             } else { |  | ||||||
|                 sandboxProfile += "(deny default (with no-log))\n"; |  | ||||||
|             } |  | ||||||
| 
 |  | ||||||
|             sandboxProfile += |  | ||||||
|                 #include "sandbox-defaults.sb.gen.hh" |  | ||||||
|                 ; |  | ||||||
| 
 |  | ||||||
|             if (fixedOutput) |  | ||||||
|                 sandboxProfile += |                 sandboxProfile += | ||||||
|                     #include "sandbox-network.sb.gen.hh" |                     #include "sandbox-defaults.sb.gen.hh" | ||||||
|                     ; |                     ; | ||||||
| 
 | 
 | ||||||
|             /* The tmpDir in scope points at the temporary build directory for our derivation. Some packages try different mechanisms
 |                 if (fixedOutput) | ||||||
|                to find temporary directories, so we want to open up a broader place for them to dump their files, if needed. */ |                     sandboxProfile += | ||||||
|             Path globalTmpDir = canonPath(getEnv("TMPDIR", "/tmp"), true); |                         #include "sandbox-network.sb.gen.hh" | ||||||
|  |                         ; | ||||||
| 
 | 
 | ||||||
|             /* They don't like trailing slashes on subpath directives */ |                 /* Our rwx outputs */ | ||||||
|             if (globalTmpDir.back() == '/') globalTmpDir.pop_back(); |                 sandboxProfile += "(allow file-read* file-write* process-exec\n"; | ||||||
| 
 |                 for (auto & i : missingPaths) { | ||||||
|             /* Our rwx outputs */ |                     sandboxProfile += (format("\t(subpath \"%1%\")\n") % i.c_str()).str(); | ||||||
|             sandboxProfile += "(allow file-read* file-write* process-exec\n"; |  | ||||||
|             for (auto & i : missingPaths) { |  | ||||||
|                 sandboxProfile += (format("\t(subpath \"%1%\")\n") % i.c_str()).str(); |  | ||||||
|             } |  | ||||||
|             sandboxProfile += ")\n"; |  | ||||||
| 
 |  | ||||||
|             /* Our inputs (transitive dependencies and any impurities computed above)
 |  | ||||||
| 
 |  | ||||||
|                without file-write* allowed, access() incorrectly returns EPERM |  | ||||||
|              */ |  | ||||||
|             sandboxProfile += "(allow file-read* file-write* process-exec\n"; |  | ||||||
|             for (auto & i : dirsInChroot) { |  | ||||||
|                 if (i.first != i.second.source) |  | ||||||
|                     throw Error(format( |  | ||||||
|                         "can't map '%1%' to '%2%': mismatched impure paths not supported on Darwin") |  | ||||||
|                         % i.first % i.second.source); |  | ||||||
| 
 |  | ||||||
|                 string path = i.first; |  | ||||||
|                 struct stat st; |  | ||||||
|                 if (lstat(path.c_str(), &st)) { |  | ||||||
|                     if (i.second.optional && errno == ENOENT) |  | ||||||
|                         continue; |  | ||||||
|                     throw SysError(format("getting attributes of path ‘%1%’") % path); |  | ||||||
|                 } |                 } | ||||||
|                 if (S_ISDIR(st.st_mode)) |                 sandboxProfile += ")\n"; | ||||||
|                     sandboxProfile += (format("\t(subpath \"%1%\")\n") % path).str(); |  | ||||||
|                 else |  | ||||||
|                     sandboxProfile += (format("\t(literal \"%1%\")\n") % path).str(); |  | ||||||
|             } |  | ||||||
|             sandboxProfile += ")\n"; |  | ||||||
| 
 | 
 | ||||||
|             /* Allow file-read* on full directory hierarchy to self. Allows realpath() */ |                 /* Our inputs (transitive dependencies and any impurities computed above)
 | ||||||
|             sandboxProfile += "(allow file-read*\n"; |  | ||||||
|             for (auto & i : ancestry) { |  | ||||||
|                 sandboxProfile += (format("\t(literal \"%1%\")\n") % i.c_str()).str(); |  | ||||||
|             } |  | ||||||
|             sandboxProfile += ")\n"; |  | ||||||
| 
 | 
 | ||||||
|             sandboxProfile += additionalSandboxProfile; |                    without file-write* allowed, access() incorrectly returns EPERM | ||||||
|  |                  */ | ||||||
|  |                 sandboxProfile += "(allow file-read* file-write* process-exec\n"; | ||||||
|  |                 for (auto & i : dirsInChroot) { | ||||||
|  |                     if (i.first != i.second.source) | ||||||
|  |                         throw Error(format( | ||||||
|  |                             "can't map '%1%' to '%2%': mismatched impure paths not supported on Darwin") | ||||||
|  |                             % i.first % i.second.source); | ||||||
|  | 
 | ||||||
|  |                     string path = i.first; | ||||||
|  |                     struct stat st; | ||||||
|  |                     if (lstat(path.c_str(), &st)) { | ||||||
|  |                         if (i.second.optional && errno == ENOENT) | ||||||
|  |                             continue; | ||||||
|  |                         throw SysError(format("getting attributes of path ‘%1%’") % path); | ||||||
|  |                     } | ||||||
|  |                     if (S_ISDIR(st.st_mode)) | ||||||
|  |                         sandboxProfile += (format("\t(subpath \"%1%\")\n") % path).str(); | ||||||
|  |                     else | ||||||
|  |                         sandboxProfile += (format("\t(literal \"%1%\")\n") % path).str(); | ||||||
|  |                 } | ||||||
|  |                 sandboxProfile += ")\n"; | ||||||
|  | 
 | ||||||
|  |                 /* Allow file-read* on full directory hierarchy to self. Allows realpath() */ | ||||||
|  |                 sandboxProfile += "(allow file-read*\n"; | ||||||
|  |                 for (auto & i : ancestry) { | ||||||
|  |                     sandboxProfile += (format("\t(literal \"%1%\")\n") % i.c_str()).str(); | ||||||
|  |                 } | ||||||
|  |                 sandboxProfile += ")\n"; | ||||||
|  | 
 | ||||||
|  |                 sandboxProfile += additionalSandboxProfile; | ||||||
|  |             } else | ||||||
|  |                 sandboxProfile += | ||||||
|  |                     #include "sandbox-minimal.sb.gen.hh" | ||||||
|  |                     ; | ||||||
| 
 | 
 | ||||||
|             debug("Generated sandbox profile:"); |             debug("Generated sandbox profile:"); | ||||||
|             debug(sandboxProfile); |             debug(sandboxProfile); | ||||||
|  | @ -2717,6 +2715,13 @@ void DerivationGoal::runChild() | ||||||
| 
 | 
 | ||||||
|             writeFile(sandboxFile, sandboxProfile); |             writeFile(sandboxFile, sandboxProfile); | ||||||
| 
 | 
 | ||||||
|  |             /* The tmpDir in scope points at the temporary build directory for our derivation. Some packages try different mechanisms
 | ||||||
|  |                to find temporary directories, so we want to open up a broader place for them to dump their files, if needed. */ | ||||||
|  |             Path globalTmpDir = canonPath(getEnv("TMPDIR", "/tmp"), true); | ||||||
|  | 
 | ||||||
|  |             /* They don't like trailing slashes on subpath directives */ | ||||||
|  |             if (globalTmpDir.back() == '/') globalTmpDir.pop_back(); | ||||||
|  | 
 | ||||||
|             builder = "/usr/bin/sandbox-exec"; |             builder = "/usr/bin/sandbox-exec"; | ||||||
|             args.push_back("sandbox-exec"); |             args.push_back("sandbox-exec"); | ||||||
|             args.push_back("-f"); |             args.push_back("-f"); | ||||||
|  | @ -2725,12 +2730,13 @@ void DerivationGoal::runChild() | ||||||
|             args.push_back("_GLOBAL_TMP_DIR=" + globalTmpDir); |             args.push_back("_GLOBAL_TMP_DIR=" + globalTmpDir); | ||||||
|             args.push_back(drv->builder); |             args.push_back(drv->builder); | ||||||
|         } |         } | ||||||
| #endif | #else | ||||||
|         else { |         else { | ||||||
|             builder = drv->builder.c_str(); |             builder = drv->builder.c_str(); | ||||||
|             string builderBasename = baseNameOf(drv->builder); |             string builderBasename = baseNameOf(drv->builder); | ||||||
|             args.push_back(builderBasename); |             args.push_back(builderBasename); | ||||||
|         } |         } | ||||||
|  | #endif | ||||||
| 
 | 
 | ||||||
|         for (auto & i : drv->args) |         for (auto & i : drv->args) | ||||||
|             args.push_back(rewriteStrings(i, inputRewrites)); |             args.push_back(rewriteStrings(i, inputRewrites)); | ||||||
|  |  | ||||||
|  | @ -36,7 +36,9 @@ libstore_CXXFLAGS = \ | ||||||
| 
 | 
 | ||||||
| $(d)/local-store.cc: $(d)/schema.sql.gen.hh | $(d)/local-store.cc: $(d)/schema.sql.gen.hh | ||||||
| 
 | 
 | ||||||
| $(d)/build.cc: $(d)/sandbox-defaults.sb.gen.hh $(d)/sandbox-network.sb.gen.hh | sandbox-headers = $(d)/sandbox-defaults.sb.gen.hh $(d)/sandbox-network.sb.gen.hh $(d)/sandbox-minimal.sb.gen.hh | ||||||
|  | 
 | ||||||
|  | $(d)/build.cc: $(sandbox-headers) | ||||||
| 
 | 
 | ||||||
| %.gen.hh: % | %.gen.hh: % | ||||||
| 	@echo 'R"foo(' >> $@.tmp | 	@echo 'R"foo(' >> $@.tmp | ||||||
|  | @ -44,6 +46,6 @@ $(d)/build.cc: $(d)/sandbox-defaults.sb.gen.hh $(d)/sandbox-network.sb.gen.hh | ||||||
| 	@echo ')foo"' >> $@.tmp | 	@echo ')foo"' >> $@.tmp | ||||||
| 	@mv $@.tmp $@ | 	@mv $@.tmp $@ | ||||||
| 
 | 
 | ||||||
| clean-files += $(d)/schema.sql.gen.hh $(d)/sandbox-defaults.sb.gen.hh $(d)/sandbox-network.sb.gen.hh | clean-files += $(d)/schema.sql.gen.hh $(sandbox-headers) | ||||||
| 
 | 
 | ||||||
| $(eval $(call install-file-in, $(d)/nix-store.pc, $(prefix)/lib/pkgconfig, 0644)) | $(eval $(call install-file-in, $(d)/nix-store.pc, $(prefix)/lib/pkgconfig, 0644)) | ||||||
|  |  | ||||||
|  | @ -1,5 +1,7 @@ | ||||||
| (define TMPDIR (param "_GLOBAL_TMP_DIR")) | (define TMPDIR (param "_GLOBAL_TMP_DIR")) | ||||||
| 
 | 
 | ||||||
|  | (deny default) | ||||||
|  | 
 | ||||||
| ; Disallow creating setuid/setgid binaries, since that | ; Disallow creating setuid/setgid binaries, since that | ||||||
| ; would allow breaking build user isolation. | ; would allow breaking build user isolation. | ||||||
| (deny file-write-setugid) | (deny file-write-setugid) | ||||||
|  |  | ||||||
							
								
								
									
										5
									
								
								src/libstore/sandbox-minimal.sb
									
										
									
									
									
										Normal file
									
								
							
							
						
						
									
										5
									
								
								src/libstore/sandbox-minimal.sb
									
										
									
									
									
										Normal file
									
								
							|  | @ -0,0 +1,5 @@ | ||||||
|  | (allow default) | ||||||
|  | 
 | ||||||
|  | ; Disallow creating setuid/setgid binaries, since that | ||||||
|  | ; would allow breaking build user isolation. | ||||||
|  | (deny file-write-setugid) | ||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue