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

Add Getter Setters for Extended Attributes and Reparse points. #229

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
47 changes: 47 additions & 0 deletions ea.go
@@ -1,9 +1,15 @@
//go:build windows
// +build windows

package winio

import (
"bytes"
"encoding/binary"
"errors"
"fmt"

"golang.org/x/sys/windows"
)

type fileFullEaInformation struct {
Expand All @@ -13,6 +19,9 @@ type fileFullEaInformation struct {
ValueLength uint16
}

//sys getFileEA(handle windows.Handle, iosb *ioStatusBlock, buf *uint8, bufLen uint32, returnSingleEntry bool, eaList uintptr, eaListLen uint32, eaIndex *uint32, restartScan bool) (status ntstatus) = ntdll.NtQueryEaFile
//sys setFileEA(handle windows.Handle, iosb *ioStatusBlock, buf *uint8, bufLen uint32) (status ntstatus) = ntdll.NtSetEaFile

var (
fileFullEaInformationSize = binary.Size(&fileFullEaInformation{})

Expand All @@ -28,6 +37,44 @@ type ExtendedAttribute struct {
Flags uint8
}

// GetFileEA retrieves the extended attributes for the file represented by `handle`. The
// `handle` must have been opened with file access flag FILE_READ_EA (0x8).
func GetFileEA(handle windows.Handle) ([]ExtendedAttribute, error) {
// default buffer size to start with
bufLen := 1024
buf := make([]byte, bufLen)
var iosb ioStatusBlock
// keep increasing the buffer size until it is large enough
for {
status := getFileEA(handle, &iosb, &buf[0], uint32(bufLen), false, 0, 0, nil, true)
ambarve marked this conversation as resolved.
Show resolved Hide resolved
if status.Err() != nil {
Copy link

@aneesh-n aneesh-n Aug 30, 2022

Choose a reason for hiding this comment

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

Thank you for this contribution @ambarve and I'm thankful to all the contributors of this project, it's been very helpful.

I think one aspect that could be improved in this function is that if you pass a handle to a file which does not have any extended attributes, then it returns an error, "get file EA failed with: The file or directory is corrupted and unreadable."
However, this should ideally be a normal situation that should be handled gracefully.
I noticed that the system call returns -1073741742 status code for both files and directories in this case.

Suggested change -

status := getFileEA(handle, &iosb, &buf[0], uint32(bufLen), false, 0, 0, nil, true)

		if status == -1073741742 {
			//If status is -1073741742, no extended attributes were found
			return nil, nil
		} else {
			err := status.Err()
			if err != nil {
				// convert ntstatus code to windows error
				if err == windows.ERROR_INSUFFICIENT_BUFFER || err == windows.ERROR_MORE_DATA {
					bufLen *= 2
					buf = make([]byte, bufLen)
					continue
				}
				return nil, fmt.Errorf("get file EA failed with: %w", err)
			}
		}
		break

That said, I do not know if this is the best way to handle this scenario or if that status code will always be consistent.
Anyone with a more comprehensive understanding of the system internals have any better suggestions?

// convert ntstatus code to windows error
if status.Err() == windows.ERROR_INSUFFICIENT_BUFFER || status.Err() == windows.ERROR_MORE_DATA {
bufLen *= 2
buf = make([]byte, bufLen)
} else {
msscotb marked this conversation as resolved.
Show resolved Hide resolved
return nil, fmt.Errorf("get file EA failed with: %w", status.Err())
}
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't need the else

Choose a reason for hiding this comment

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

We need the else with break otherwise the for loop doesn't end and the code gets stuck in a loop.
Existing code is correct.

Copy link
Contributor

@dcantah dcantah Aug 29, 2022

Choose a reason for hiding this comment

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

I should've been more clear. Keep the break just don't need the else. But we'd need something to retry the loop so we'd need a continue after buf = make([]byte, bufLen), e.g. the below. This seems mostly preference, but in my jumbled brain it flows a bit better

for {
    status := getFileEA(handle, &iosb, &buf[0], uint32(bufLen), false, 0, 0, nil, true)
    if status.Err() != nil {
	// convert ntstatus code to windows error
	if status.Err() == windows.ERROR_INSUFFICIENT_BUFFER || status.Err() == windows.ERROR_MORE_DATA {
		bufLen *= 2
		buf = make([]byte, bufLen)
                continue
	} 
        return nil, fmt.Errorf("get file EA failed with: %w", status.Err())
   }
   break
}

Choose a reason for hiding this comment

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

I see what you mean, that does seem a bit more concise.

break
}
}
return DecodeExtendedAttributes(buf)
}

// SetFileEA sets the extended attributes for the file represented by `handle`. The
// handle must have been opened with the file access flag FILE_WRITE_EA(0x10).
func SetFileEA(handle windows.Handle, attrs []ExtendedAttribute) error {
encodedEA, err := EncodeExtendedAttributes(attrs)
if err != nil {
return fmt.Errorf("failed to encoded extended attributes: %w", err)
}

var iosb ioStatusBlock

return setFileEA(handle, &iosb, &encodedEA[0], uint32(len(encodedEA))).Err()
}

func parseEa(b []byte) (ea ExtendedAttribute, nb []byte, err error) {
var info fileFullEaInformation
err = binary.Read(bytes.NewReader(b), binary.LittleEndian, &info)
Expand Down
56 changes: 56 additions & 0 deletions ea_test.go
Expand Up @@ -4,12 +4,19 @@
package winio

import (
"fmt"
"io/ioutil"
"math"
"math/rand"
"os"
"path/filepath"
"reflect"
"syscall"
"testing"
"time"
"unsafe"

"golang.org/x/sys/windows"
)

var (
Expand All @@ -23,6 +30,10 @@ var (
testEasTruncated = testEasEncoded[0:20]
)

func init() {
rand.Seed(time.Now().Unix())
}

func Test_RoundTripEas(t *testing.T) {
b, err := EncodeExtendedAttributes(testEas)
if err != nil {
Expand Down Expand Up @@ -90,3 +101,48 @@ func Test_SetFileEa(t *testing.T) {
t.Fatalf("NtSetEaFile failed with %08x", r)
}
}

func Test_SetGetFileEA(t *testing.T) {
tempDir := t.TempDir()
ambarve marked this conversation as resolved.
Show resolved Hide resolved
testfilePath := filepath.Join(tempDir, "testfile.txt")
// create temp file
testfile, err := os.Create(testfilePath)
if err != nil {
t.Fatalf("failed to create temporary file: %s", err)
}
testfile.Close()

nAttrs := 3
testEAs := make([]ExtendedAttribute, 3)
// generate random extended attributes for test
for i := 0; i < nAttrs; i++ {
// EA name is automatically converted to upper case before storing, so
// when reading it back it returns the upper case name. To avoid test
// failures because of that keep the name upper cased.
testEAs[i].Name = fmt.Sprintf("TESTEA%d", i+1)
testEAs[i].Value = make([]byte, rand.Int31n(math.MaxUint8))
rand.Read(testEAs[i].Value)
}

utf16Path := windows.StringToUTF16Ptr(testfilePath)
fileAccessRightReadWriteEA := (0x8 | 0x10)
fileHandle, err := windows.CreateFile(utf16Path, uint32(fileAccessRightReadWriteEA), 0, nil, windows.OPEN_EXISTING, windows.FILE_ATTRIBUTE_NORMAL, 0)
if err != nil {
t.Fatalf("open file failed with: %s", err)
}
defer windows.Close(fileHandle)

if err := SetFileEA(fileHandle, testEAs); err != nil {
t.Fatalf("set EA for file failed: %s", err)
}

var readEAs []ExtendedAttribute
if readEAs, err = GetFileEA(fileHandle); err != nil {
t.Fatalf("get EA for file failed: %s", err)
}

if !reflect.DeepEqual(readEAs, testEAs) {
t.Logf("expected: %+v, found: %+v\n", testEAs, readEAs)
t.Fatalf("EAs read from testfile don't match")
}
}
Copy link

@aneesh-n aneesh-n Aug 30, 2022

Choose a reason for hiding this comment

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

Another useful test would be for setting and getting extended attributes for folders.
The main change required is the passing of an extra attribute - windows.FILE_FLAG_BACKUP_SEMANTICS for getting the file handle.


func Test_SetGetFolderEA(t *testing.T) {
	tempDir := t.TempDir()
	testfolderPath := filepath.Join(tempDir, "testfolder")
	// create temp folder
	err := os.Mkdir(testfolderPath, os.ModeDir)
	if err != nil {
		t.Fatalf("failed to create temporary file: %s", err)
	}

	nAttrs := 3
	testEAs := make([]ExtendedAttribute, 3)
	// generate random extended attributes for test
	for i := 0; i < nAttrs; i++ {
		// EA name is automatically converted to upper case before storing, so
		// when reading it back it returns the upper case name. To avoid test
		// failures because of that keep the name upper cased.
		testEAs[i].Name = fmt.Sprintf("TESTEA%d", i+1)
		testEAs[i].Value = make([]byte, rand.Int31n(math.MaxUint8))
		rand.Read(testEAs[i].Value)
	}

	utf16Path := windows.StringToUTF16Ptr(testfolderPath)
	fileAccessRightReadWriteEA := (0x8 | 0x10)
	fileHandle, err := windows.CreateFile(utf16Path, uint32(fileAccessRightReadWriteEA), 0, nil, windows.OPEN_EXISTING, windows.FILE_ATTRIBUTE_NORMAL|windows.FILE_FLAG_BACKUP_SEMANTICS, 0)

	if err != nil {
		t.Fatalf("open folder failed with: %s", err)
	}
	defer windows.Close(fileHandle)

	if err := SetFileEA(fileHandle, testEAs); err != nil {
		t.Fatalf("set EA for folder failed: %s", err)
	}

	var readEAs []ExtendedAttribute
	if readEAs, err = GetFileEA(fileHandle); err != nil {
		t.Fatalf("get EA for folder failed: %s", err)
	}

	if !reflect.DeepEqual(readEAs, testEAs) {
		t.Logf("expected: %+v, found: %+v\n", testEAs, readEAs)
		t.Fatalf("EAs read from test folder don't match")
	}
}

17 changes: 13 additions & 4 deletions fileinfo.go
Expand Up @@ -22,7 +22,7 @@ type FileBasicInfo struct {
func GetFileBasicInfo(f *os.File) (*FileBasicInfo, error) {
bi := &FileBasicInfo{}
if err := windows.GetFileInformationByHandleEx(windows.Handle(f.Fd()), windows.FileBasicInfo, (*byte)(unsafe.Pointer(bi)), uint32(unsafe.Sizeof(*bi))); err != nil {
return nil, &os.PathError{Op: "GetFileInformationByHandleEx", Path: f.Name(), Err: err}
return nil, &os.SyscallError{Syscall: "GetFileInformationByHandleEx", Err: err}
}
runtime.KeepAlive(f)
return bi, nil
Expand All @@ -31,12 +31,21 @@ func GetFileBasicInfo(f *os.File) (*FileBasicInfo, error) {
// SetFileBasicInfo sets times and attributes for a file.
func SetFileBasicInfo(f *os.File, bi *FileBasicInfo) error {
if err := windows.SetFileInformationByHandle(windows.Handle(f.Fd()), windows.FileBasicInfo, (*byte)(unsafe.Pointer(bi)), uint32(unsafe.Sizeof(*bi))); err != nil {
return &os.PathError{Op: "SetFileInformationByHandle", Path: f.Name(), Err: err}
return &os.SyscallError{Syscall: "SetFileInformationByHandle", Err: err}
}
runtime.KeepAlive(f)
return nil
}

// SetFileBasicInfoForHandle sets times and attributes for a file represented by
// the given handle
func SetFileBasicInfoForHandle(f windows.Handle, bi *FileBasicInfo) error {
if err := windows.SetFileInformationByHandle(f, windows.FileBasicInfo, (*byte)(unsafe.Pointer(bi)), uint32(unsafe.Sizeof(*bi))); err != nil {
return &os.SyscallError{Syscall: "SetFileInformationByHandle", Err: err}
}
return nil
}

// FileStandardInfo contains extended information for the file.
// FILE_STANDARD_INFO in WinBase.h
// https://docs.microsoft.com/en-us/windows/win32/api/winbase/ns-winbase-file_standard_info
Expand All @@ -50,7 +59,7 @@ type FileStandardInfo struct {
func GetFileStandardInfo(f *os.File) (*FileStandardInfo, error) {
si := &FileStandardInfo{}
if err := windows.GetFileInformationByHandleEx(windows.Handle(f.Fd()), windows.FileStandardInfo, (*byte)(unsafe.Pointer(si)), uint32(unsafe.Sizeof(*si))); err != nil {
return nil, &os.PathError{Op: "GetFileInformationByHandleEx", Path: f.Name(), Err: err}
return nil, &os.SyscallError{Syscall: "GetFileInformationByHandleEx", Err: err}
}
runtime.KeepAlive(f)
return si, nil
Expand All @@ -67,7 +76,7 @@ type FileIDInfo struct {
func GetFileID(f *os.File) (*FileIDInfo, error) {
fileID := &FileIDInfo{}
if err := windows.GetFileInformationByHandleEx(windows.Handle(f.Fd()), windows.FileIdInfo, (*byte)(unsafe.Pointer(fileID)), uint32(unsafe.Sizeof(*fileID))); err != nil {
return nil, &os.PathError{Op: "GetFileInformationByHandleEx", Path: f.Name(), Err: err}
return nil, &os.SyscallError{Syscall: "GetFileInformationByHandleEx", Err: err}
}
runtime.KeepAlive(f)
return fileID, nil
Expand Down
1 change: 1 addition & 0 deletions pipe.go
Expand Up @@ -27,6 +27,7 @@ import (
//sys rtlDosPathNameToNtPathName(name *uint16, ntName *unicodeString, filePart uintptr, reserved uintptr) (status ntstatus) = ntdll.RtlDosPathNameToNtPathName_U
//sys rtlDefaultNpAcl(dacl *uintptr) (status ntstatus) = ntdll.RtlDefaultNpAcl

// ioStatusBlock represents the IO_STATUS_BLOCK struct defined here: https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/ns-wdm-_io_status_block
type ioStatusBlock struct {
Status, Information uintptr
}
Expand Down
80 changes: 80 additions & 0 deletions reparse.go
Expand Up @@ -10,13 +10,21 @@ import (
"strings"
"unicode/utf16"
"unsafe"

"golang.org/x/sys/windows"
)

const (
reparseTagMountPoint = 0xA0000003
reparseTagSymlink = 0xA000000C

_FSCTL_SET_REPARSE_POINT uint32 = ((0x9 << 16) | (41 << 2))
_FSCTL_GET_REPARSE_POINT uint32 = ((0x9 << 16) | (42 << 2))
_MAXIMUM_REPARSE_DATA_BUFFER_SIZE = 16 * 1024
)

// reparseDataBuffer is only kept for compatibility purposes. When adding new
// code prefer ReparseDataBuffer instead.
type reparseDataBuffer struct {
ReparseTag uint32
ReparseDataLength uint16
Expand All @@ -28,6 +36,9 @@ type reparseDataBuffer struct {
}

// ReparsePoint describes a Win32 symlink or mount point.
// However, there can be other types of reparse points that are specific to
// third party applications. To work with such reparse points use
// ReparseDataBuffer instead.
type ReparsePoint struct {
Target string
IsMountPoint bool
Expand Down Expand Up @@ -129,3 +140,72 @@ func EncodeReparsePoint(rp *ReparsePoint) []byte {
binary.Write(&b, binary.LittleEndian, target16)
return b.Bytes()
}

// ReparseDataBuffer corresponds to REPARSE_DATA_BUFFER struct defined here:
// https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/ntifs/ns-ntifs-_reparse_data_buffer.
// ReparesPoint struct (and corresponding Encode/Decode functions) provided above only
// handle symlink or mountpoint type of reparse points. To work with other reparse points
// use this struct. When calling SetReparsePoint, if it is a symlink or mountpoint
// reparse point pass in the buffer generated by EncodeReparsePointData. If it is some
// other kind of reparse point pass in the buffer generated by
// ReparseDataBuffer.Encode. (same goes for GetReparsePoints)
type ReparseDataBuffer struct {
ReparseTag uint32
ReparseDataLength uint16
Reserved uint16
// a generic data buffer
DataBuffer []byte
}

// Encode encodes the current ReparseDataBuffer struct that can be passed
// to the SetReparsePoint.
func (r *ReparseDataBuffer) Encode() []byte {
var buf bytes.Buffer
binary.Write(&buf, binary.LittleEndian, r.ReparseTag)
binary.Write(&buf, binary.LittleEndian, r.ReparseDataLength)
binary.Write(&buf, binary.LittleEndian, r.Reserved)
buf.Write(r.DataBuffer)
return buf.Bytes()
}

// Decode decodes the byte array (returned by GetReparsePoint) into
// the current ReparseDataBuffer struct.
func (r *ReparseDataBuffer) Decode(data []byte) error {
rdr := bytes.NewReader(data)
err := binary.Read(rdr, binary.LittleEndian, r)
if err != nil {
fmt.Errorf("failed to decode reparse data buffer: %w", err)
}
return nil
}

// GetReparsePoint returns a byte array that represents a ReparseDataBuffer if
// the file at `path` is a valid reparse point.
func GetReparsePoint(path string) ([]byte, error) {
utf16DestPath, err := windows.UTF16FromString(path)
if err != nil {
return nil, err
}
// We need to open the file with windows specific permissions so just using
// os.Open won't work.
fileHandle, err := windows.CreateFile(&utf16DestPath[0], windows.GENERIC_READ, 0, nil, windows.OPEN_EXISTING, (windows.FILE_ATTRIBUTE_NORMAL | windows.FILE_FLAG_OPEN_REPARSE_POINT | windows.FILE_FLAG_BACKUP_SEMANTICS), 0)
if err != nil {
return nil, fmt.Errorf("open file failed with: %w", err)
}
defer windows.Close(fileHandle)

outBuf := make([]byte, _MAXIMUM_REPARSE_DATA_BUFFER_SIZE)
var outBufLen uint32
err = windows.DeviceIoControl(fileHandle, _FSCTL_GET_REPARSE_POINT, nil, 0, &outBuf[0], _MAXIMUM_REPARSE_DATA_BUFFER_SIZE, &outBufLen, nil)
if err != nil {
return nil, fmt.Errorf("failed to get reparse point for file %s: %w", path, err)
}

return outBuf[:outBufLen], nil
}

// SetReparsePoint sets a reparse point with `data` on the file/directory represented by
// `handle` . `data` should be an encoded ReparseDataBuffer struct.
func SetReparsePoint(handle windows.Handle, data []byte) error {
Copy link
Contributor

@dcantah dcantah Jun 16, 2022

Choose a reason for hiding this comment

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

Would it make sense to make these "friendly" APIs and just take in ReparseDataBuffers directly and we encode inside?

e.g.

func SetReparsePoint(handle windows.Handle, data *ReparseDataBuffer) error {
    dataBuf := data.Encode() 
    // rest of the function
}

and if folks want to call encode/decode and pass around byte slices themselves we leave those methods exported?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IIRC the problem here is that we currently have two structs representing the reparse data buffer in this code. One is the ReparsePoint struct and other is the newly added ReparseDataBuffer struct. The ReparsePoint struct only supports symlinks and mount points while ReparseDataBuffer supports all kinds of reparse points. We have to keep the ReparsePoint struct for backwards compatibility. We want to use the SetReparsePoint call for a buffer that could be an encoded buffer of ReparseDataBuffer or ReparsePoint, hence the []byte array. Let me know what you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Well the bit that confused me is the comment, it states that it should be an encoded ReparseDataBuffer explicitly, so I thought this didn't care about the old definition

Comment on lines +184 to +209
Copy link
Contributor

@dcantah dcantah Aug 12, 2022

Choose a reason for hiding this comment

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

I still think the split here in API between Get taking in a path and Set using a handle is strange. Maybe we expose a method to call CreateFile with the minimum set of flags get/set need for the reparse operations to work and make them tweakable. I'd have to look more on what's needed..

return windows.DeviceIoControl(handle, _FSCTL_SET_REPARSE_POINT, &data[0], uint32(len(data)), nil, 0, nil, nil)
}