Windows: Back up files mutated during layer import

The Windows BCD store for the utility VM is mutated during layer import,
which causes failures in docker save. Back up the BCD store and related
log files so that save has access to their original contents.

Fixes #25893.

Signed-off-by: John Starks <jostarks@microsoft.com>

daemon/graphdriver/windows/windows.go

@@ -31,7 +31,6 @@ import (
 	"github.com/docker/docker/pkg/longpath"
 	"github.com/docker/docker/pkg/reexec"
 	"github.com/docker/go-units"
-	"github.com/vbatts/tar-split/tar/storage"
 )
 
 // filterDriver is an HCSShim driver type for the Windows Filter driver.
@@ -41,6 +40,15 @@ var (
 	vmcomputedll            = syscall.NewLazyDLL("vmcompute.dll")
 	hcsExpandSandboxSize    = vmcomputedll.NewProc("ExpandSandboxSize")
 	hcsSandboxSizeSupported = hcsExpandSandboxSize.Find() == nil
+
+	// mutatedFiles is a list of files that are mutated by the import process
+	// and must be backed up and restored.
+	mutatedFiles = map[string]string{
+		"UtilityVM/Files/EFI/Microsoft/Boot/BCD":      "bcd.bak",
+		"UtilityVM/Files/EFI/Microsoft/Boot/BCD.LOG":  "bcd.log.bak",
+		"UtilityVM/Files/EFI/Microsoft/Boot/BCD.LOG1": "bcd.log1.bak",
+		"UtilityVM/Files/EFI/Microsoft/Boot/BCD.LOG2": "bcd.log2.bak",
+	}
 )
 
 // init registers the windows graph drivers to the register.
@@ -532,7 +540,48 @@ func (d *Driver) exportLayer(id string, parentLayerPaths []string) (archive.Arch
 	return archive, nil
 }
 
-func writeLayerFromTar(r archive.Reader, w hcsshim.LayerWriter) (int64, error) {
+// writeBackupStreamFromTarAndSaveMutatedFiles reads data from a tar stream and
+// writes it to a backup stream, and also saves any files that will be mutated
+// by the import layer process to a backup location.
+func writeBackupStreamFromTarAndSaveMutatedFiles(buf *bufio.Writer, w io.Writer, t *tar.Reader, hdr *tar.Header, root string) (nextHdr *tar.Header, err error) {
+	var bcdBackup *os.File
+	var bcdBackupWriter *winio.BackupFileWriter
+	if backupPath, ok := mutatedFiles[hdr.Name]; ok {
+		bcdBackup, err = os.Create(filepath.Join(root, backupPath))
+		if err != nil {
+			return nil, err
+		}
+		defer func() {
+			cerr := bcdBackup.Close()
+			if err == nil {
+				err = cerr
+			}
+		}()
+
+		bcdBackupWriter = winio.NewBackupFileWriter(bcdBackup, false)
+		defer func() {
+			cerr := bcdBackupWriter.Close()
+			if err == nil {
+				err = cerr
+			}
+		}()
+
+		buf.Reset(io.MultiWriter(w, bcdBackupWriter))
+	} else {
+		buf.Reset(w)
+	}
+
+	defer func() {
+		ferr := buf.Flush()
+		if err == nil {
+			err = ferr
+		}
+	}()
+
+	return backuptar.WriteBackupStreamFromTarFile(buf, t, hdr)
+}
+
+func writeLayerFromTar(r archive.Reader, w hcsshim.LayerWriter, root string) (int64, error) {
 	t := tar.NewReader(r)
 	hdr, err := t.Next()
 	totalSize := int64(0)
@@ -566,13 +615,7 @@ func writeLayerFromTar(r archive.Reader, w hcsshim.LayerWriter) (int64, error) {
 			if err != nil {
 				return 0, err
 			}
-			buf.Reset(w)
-
-			hdr, err = backuptar.WriteBackupStreamFromTarFile(buf, t, hdr)
-			ferr := buf.Flush()
-			if ferr != nil {
-				err = ferr
-			}
+			hdr, err = writeBackupStreamFromTarAndSaveMutatedFiles(buf, w, t, hdr, root)
 			totalSize += size
 		}
 	}
@@ -582,46 +625,6 @@ func writeLayerFromTar(r archive.Reader, w hcsshim.LayerWriter) (int64, error) {
 	return totalSize, nil
 }
 
-func addAceToSddlDacl(sddl, ace string) (string, bool) {
-	daclStart := strings.Index(sddl, "D:")
-	if daclStart < 0 {
-		return sddl, false
-	}
-
-	dacl := sddl[daclStart:]
-	daclEnd := strings.Index(dacl, "S:")
-	if daclEnd < 0 {
-		daclEnd = len(dacl)
-	}
-	dacl = dacl[:daclEnd]
-
-	if strings.Contains(dacl, ace) {
-		return sddl, true
-	}
-
-	i := 2
-	for i+1 < len(dacl) {
-		if dacl[i] != '(' {
-			return sddl, false
-		}
-
-		if dacl[i+1] == 'A' {
-			break
-		}
-
-		i += 2
-		for p := 1; i < len(dacl) && p > 0; i++ {
-			if dacl[i] == '(' {
-				p++
-			} else if dacl[i] == ')' {
-				p--
-			}
-		}
-	}
-
-	return sddl[:daclStart+i] + ace + sddl[daclStart+i:], true
-}
-
 // importLayer adds a new layer to the tag and graph store based on the given data.
 func (d *Driver) importLayer(id string, layerData archive.Reader, parentLayerPaths []string) (size int64, err error) {
 	cmd := reexec.Command(append([]string{"docker-windows-write-layer", d.info.HomeDir, id}, parentLayerPaths...)...)
@@ -663,7 +666,7 @@ func writeLayer() {
 			return err
 		}
 
-		size, err := writeLayerFromTar(os.Stdin, w)
+		size, err := writeLayerFromTar(os.Stdin, w, filepath.Join(home, id))
 		if err != nil {
 			return err
 		}
@@ -743,6 +746,10 @@ type fileGetCloserWithBackupPrivileges struct {
 }
 
 func (fg *fileGetCloserWithBackupPrivileges) Get(filename string) (io.ReadCloser, error) {
+	if backupPath, ok := mutatedFiles[filename]; ok {
+		return os.Open(filepath.Join(fg.path, backupPath))
+	}
+
 	var f *os.File
 	// Open the file while holding the Windows backup privilege. This ensures that the
 	// file can be opened even if the caller does not actually have access to it according
@@ -767,16 +774,6 @@ func (fg *fileGetCloserWithBackupPrivileges) Close() error {
 	return nil
 }
 
-type fileGetDestroyCloser struct {
-	storage.FileGetter
-	path string
-}
-
-func (f *fileGetDestroyCloser) Close() error {
-	// TODO: activate layers and release here?
-	return os.RemoveAll(f.path)
-}
-
 // DiffGetter returns a FileGetCloser that can read files from the directory that
 // contains files for the layer differences. Used for direct access for tar-split.
 func (d *Driver) DiffGetter(id string) (graphdriver.FileGetCloser, error) {

daemon/graphdriver/windows/windows_windows_test.go

@@ -1,18 +0,0 @@
-package windows
-
-import "testing"
-
-func TestAddAceToSddlDacl(t *testing.T) {
-	cases := [][3]string{
-		{"D:", "(A;;;)", "D:(A;;;)"},
-		{"D:(A;;;)", "(A;;;)", "D:(A;;;)"},
-		{"O:D:(A;;;stuff)", "(A;;;new)", "O:D:(A;;;new)(A;;;stuff)"},
-		{"O:D:(D;;;no)(A;;;stuff)", "(A;;;new)", "O:D:(D;;;no)(A;;;new)(A;;;stuff)"},
-	}
-
-	for _, c := range cases {
-		if newSddl, worked := addAceToSddlDacl(c[0], c[1]); !worked || newSddl != c[2] {
-			t.Errorf("%s + %s == %s, expected %s (%v)", c[0], c[1], newSddl, c[2], worked)
-		}
-	}
-}