linuxboot / fiano

Go-based tools for modifying UEFI firmware
BSD 3-Clause "New" or "Revised" License
302 stars 49 forks source link

uefi: Garbage logging on `NewFlashImage` #330

Closed xaionaro closed 3 years ago

xaionaro commented 3 years ago

We write a comprehensive tooling for TXT and BootGuard, and we need to avoid any logs print by fiano. But fiano does not allow to do that. And here's a simple experiment:

type panicWriter struct{}

func (panicWriter) Write(b []byte) (int, error) {
    panic("PANIC")
}

    log.SetOutput(panicWriter{})    
...
    flash, err := uefi.NewFlashImage(image)
    if err != nil {
        return 0, 0, fmt.Errorf("count not initialize a flash image: %w", err)
    }

Then run it against coreboot and get:

xaionaro@void:~/go/src/github.com/9elements/converged-security-suite/pkg/tools$ GO111MODULE=off go test ./... -run=TestCalcImageOffsetNoLogGarbage -count=1
--- FAIL: TestCalcImageOffsetNoLogGarbage (2.14s)
panic: PANIC [recovered]
    panic: PANIC

goroutine 6 [running]:
testing.tRunner.func1.1(0x6504c0, 0x6f76b0)
    /home/xaionaro/.gimme/versions/go1.15.linux.amd64/src/testing/testing.go:1057 +0x30d
testing.tRunner.func1(0xc000001b00)
    /home/xaionaro/.gimme/versions/go1.15.linux.amd64/src/testing/testing.go:1060 +0x41a
panic(0x6504c0, 0x6f76b0)
    /home/xaionaro/.gimme/versions/go1.15.linux.amd64/src/runtime/panic.go:969 +0x175
github.com/9elements/converged-security-suite/pkg/tools.panicWriter.Write(...)
    /home/xaionaro/go/src/github.com/9elements/converged-security-suite/pkg/tools/ifd_test.go:15
log.(*Logger).Output(0xc0000720a0, 0x2, 0xc000022120, 0x107, 0x0, 0x0)
    /home/xaionaro/.gimme/versions/go1.15.linux.amd64/src/log/log.go:181 +0x284
log.Printf(0x6b9b5b, 0x2a, 0xc0000a3ca8, 0x1, 0x1)
    /home/xaionaro/.gimme/versions/go1.15.linux.amd64/src/log/log.go:320 +0x85
github.com/linuxboot/fiano/pkg/uefi.NewMERegion(0xc00c901000, 0x2fe7000, 0x7ffee00, 0xc00001e248, 0x1, 0xc0000100e0, 0x0, 0x1, 0x0)
    /home/xaionaro/go/src/github.com/linuxboot/fiano/pkg/uefi/meregion.go:195 +0x26a
github.com/linuxboot/fiano/pkg/uefi.NewFlashImage(0xc00c900000, 0x4000000, 0x7fffe00, 0x14, 0x0, 0x0)
    /home/xaionaro/go/src/github.com/linuxboot/fiano/pkg/uefi/flash.go:276 +0x562
github.com/9elements/converged-security-suite/pkg/tools.GetRegion(0xc00c900000, 0x4000000, 0x7fffe00, 0x0, 0x0, 0x0, 0x6c5b01)
    /home/xaionaro/go/src/github.com/9elements/converged-security-suite/pkg/tools/ifd.go:25 +0xfb
github.com/9elements/converged-security-suite/pkg/tools.CalcImageOffset(0xc00c900000, 0x4000000, 0x7fffe00, 0x1, 0x0, 0x0, 0x0)
    /home/xaionaro/go/src/github.com/9elements/converged-security-suite/pkg/tools/ifd.go:13 +0x4c
github.com/9elements/converged-security-suite/pkg/tools.TestCalcImageOffsetNoLogGarbage(0xc000001b00)
    /home/xaionaro/go/src/github.com/9elements/converged-security-suite/pkg/tools/ifd_test.go:25 +0xde
testing.tRunner(0xc000001b00, 0x6c52e8)
    /home/xaionaro/.gimme/versions/go1.15.linux.amd64/src/testing/testing.go:1108 +0xef
created by testing.(*T).Run
    /home/xaionaro/.gimme/versions/go1.15.linux.amd64/src/testing/testing.go:1159 +0x386
FAIL    github.com/9elements/converged-security-suite/pkg/tools 2.145s
FAIL

What happens is:

And we need to suppress this log.Printf. The only thing we can do is log.SetOutput(ioutil.Discard), but we would like to avoid modifying a global logger.

So as a quick solution I propose to add package log to fiano and define a logger there, which will be used everywhere else within fiano. Any opinions?

xaionaro commented 3 years ago

Related: https://github.com/linuxboot/fiano/issues/272

GanShun commented 3 years ago

I'm perfectly good with adding a logger to fiano!

xaionaro commented 3 years ago

Closed through: https://github.com/linuxboot/fiano/pull/331