Skip to content
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.

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

Make tar2ext4 deterministic with files without parent dir in tar #2270

Merged
merged 4 commits into from
Sep 25, 2024

Conversation

takuro-sato
Copy link
Contributor

@takuro-sato takuro-sato commented Sep 24, 2024

Problem to fix

./dmverity-vhd roothash is not deterministic for some file docker images.

IMAGE=gcr.io/distroless/cc-debian11@sha256:d5a2169bc2282598f0cf886a3d301269d0ee5bf7f7392184198dd41d36b70548

docker pull $IMAGE > /dev/null

./dmverity-vhd -v -d roothash -i $IMAGE > bad-debian-1.txt
./dmverity-vhd -v -d roothash -i $IMAGE > bad-debian-2.txt

diff  bad-debian-1.txt bad-debian-2.txt

The above command shows a diff which suggest it's not deterministic.
This is causing a problem for az confcom tool that the generated security policy is not consistent for each run (./dmverity-vhd roothash is called here).
Container deployment is blocked as well for C-ACI.

Cause of the problem

In the example image, there is a problematic tar file only with a file etc/nsswitch.conf. For most of tar files there would be also etc/ directory as the parent of nsswitch.conf, but for this specific tar there isn't.
For the such files without parent, MakeParents() makes the parent directories for ext4, but it uses time.Now() and it's causing the non-deterministic behavior.

Fix

Change time.Now() to the child's value.

Test

  • diff bad-debian-1.txt bad-debian-2.txt doesn't show any diff after the fix
  • go test ./ext4/tar2ext4/.... It includes new a unit test case.

Note

go test ./ext4/internal/compactext4/... is broken, but it seems to be the case even for main branch. Does anyone now the problem?

--- FAIL: TestBasic (0.01s)                                                              
    compact_test.go:221: e2fsck 1.45.5 (07-Jan-2020)                                     
        Pass 1: Checking inodes, blocks, and sizes                                       
        Pass 2: Checking directory structure                                             
        Pass 3: Checking directory connectivity                                          
        Pass 4: Checking reference counts                                                
        Pass 5: Checking group summary information                                       
        Padding at end of inode bitmap is not set. Fix? no                               
                                                                                         
                                                                                         
        testfs.img: ********** WARNING: Filesystem still has errors **********           
                                                                                         
                                                                                         
                  26 inodes used (81.25%, out of 32)                                     
                   0 non-contiguous files (0.0%)                                         
                   0 non-contiguous directories (0.0%)                                   
                     # of inodes with ind/dind/tind blocks: 0/0/0                        
                     Extent depth histogram: 9                                           
                  16 blocks used (100.00%, out of 16)                                    
                   0 bad blocks                                                          
                   0 large files                                                         
                                                                                         
                   5 regular files                                                       
                   3 directories                                                         
                   1 character device file                                               
                   1 block device file                                                   
                   1 fifo                   
                   1 link                   
                   5 symbolic links (2 fast symbolic links)                              
                   1 socket                 
        ------------                        
                  18 files                  
    compact_test.go:221: exit status 4 

@takuro-sato takuro-sato force-pushed the handle-file-without-parent-in-tar branch from 6d01bd3 to d43c1e8 Compare September 24, 2024 09:27
Signed-off-by: Takuro Sato <[email protected]>
@takuro-sato takuro-sato marked this pull request as ready for review September 24, 2024 10:05
@takuro-sato takuro-sato requested a review from a team as a code owner September 24, 2024 10:05
Signed-off-by: Takuro Sato <[email protected]>
@takuro-sato
Copy link
Contributor Author

@anmaxvl You might have already checked it, but as I mentioned in the PR description the tests under ./ext4/internal/compactext4/... seems to be broken both in this branch and main. Do you know anything about it?


opts := []Option{ConvertWhiteout}

tmpVhdPath := filepath.Join(os.TempDir(), "test-vhd.ext4")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nit: its not a VHD, since there is no VHD footer option

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 6dee921

Signed-off-by: Takuro Sato <[email protected]>
@anmaxvl anmaxvl merged commit 16dc8eb into microsoft:main Sep 25, 2024
19 checks passed
hgarvison pushed a commit to hgarvison/hcsshim that referenced this pull request Sep 27, 2024
…rosoft#2270)

Make tar2ext4 deterministic with files without parent dir in tar

Signed-off-by: Takuro Sato <[email protected]>
Signed-off-by: Heather Garvison <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants