From f6d4982664a952f8fee031959b93d1d2b7cba3f9 Mon Sep 17 00:00:00 2001 From: Pablo Fraile Alonso Date: Wed, 20 Nov 2024 15:22:05 +0100 Subject: [PATCH 1/4] fix: don't use keyboxd for gpg detection Now the new detection heuristic is to check if we open the private key files. Since we're detecting only the keys that need a yubikey touch, we filter the ones that have the shadowed-private-key attribute on the S-expression inside the key file ( more information about it here: https://github.com/gpg/gnupg/blob/6737e07a9b04064947ae37abd28b845a09abee22/doc/keyformat.txt#shadowed-private-key-format ) We can have false positives if the user has a yubikey + other gpg smartcard device (since will detect it will try to open the private key file too!). This closes #54 --- detector/gpg.go | 20 +++++++-------- main.go | 65 ++++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 66 insertions(+), 19 deletions(-) diff --git a/detector/gpg.go b/detector/gpg.go index 7dac6df..c07c0d9 100644 --- a/detector/gpg.go +++ b/detector/gpg.go @@ -12,17 +12,17 @@ import ( ) // WatchGPG watches for hints that YubiKey is maybe waiting for a touch on a GPG request -func WatchGPG(gpgPubringPath string, requestGPGCheck chan bool) { - // No need for a buffered channel, - // we are interested only in the first event, it's ok to skip all subsequent ones - events := make(chan notify.EventInfo) +func WatchGPG(filesToWatch []string, requestGPGCheck chan bool) { + events := make(chan notify.EventInfo, len(filesToWatch)) initWatcher := func() { - if err := notify.Watch(gpgPubringPath, events, notify.InOpen, notify.InDeleteSelf, notify.InMoveSelf); err != nil { - log.Errorf("Cannot establish a watch on gpg's pubring.kbx file '%v': %v", gpgPubringPath, err) - return - } - log.Debug("GPG watcher is successfully established") + for _, file := range filesToWatch { + if err := notify.Watch(file, events, notify.InOpen); err != nil { + log.Errorf("Failed to watch file '%s': %v\n", file, err) + return; + } + log.Debug("GPG watcher is watching '%s'...\n", file) + } } initWatcher() @@ -36,7 +36,7 @@ func WatchGPG(gpgPubringPath string, requestGPGCheck chan bool) { default: } default: - log.Debugf("pubring.kbx received file event '%+v', recreating the watcher.", event.Event()) + log.Debugf("GPG received file event '%+v', recreating the watcher.", event.Event()) notify.Stop(events) time.Sleep(5 * time.Second) initWatcher() diff --git a/main.go b/main.go index 3b2d0c4..229e7c4 100644 --- a/main.go +++ b/main.go @@ -1,11 +1,13 @@ package main import ( + "bufio" "flag" "fmt" "os" "os/signal" "path" + "path/filepath" "strings" "sync" "syscall" @@ -78,21 +80,66 @@ func main() { } else if ctx.SetProtocol(gpgme.ProtocolAssuan) != nil { log.Debugf("Cannot initialize Assuan IPC: %v. Disabling GPG and SSH watchers.", err) } else { - gpgPubringPath := path.Join(gpgme.GetDirInfo("homedir"), "pubring.kbx") - if _, err := os.Stat(gpgPubringPath); err == nil { - - requestGPGCheck := make(chan bool) - go detector.CheckGPGOnRequest(requestGPGCheck, notifiers, ctx) - go detector.WatchGPG(gpgPubringPath, requestGPGCheck) - go detector.WatchSSH(requestGPGCheck, exits) - } else { - log.Debugf("'%v' could not be found. Disabling GPG and SSH watchers.", gpgPubringPath) + var gpgPubringPath = path.Join(gpgme.GetDirInfo("homedir"), "private-keys-v1.d") + if _, err := os.Stat(gpgPubringPath); os.IsNotExist(err) { + fmt.Printf("Directory '%s' does not exist (you have no private keys).\n", gpgPubringPath) + return } + var searchTerm = "shadowed-private-key" + var filesToWatch []string + filesToWatch, err := findMatchingFiles(gpgPubringPath, searchTerm) + if err != nil { + fmt.Printf("Error finding files: %v\n", err) + return + } + if len(filesToWatch) == 0 { + fmt.Printf("No files matching the term '%s' found.\n", searchTerm) + return + } + requestGPGCheck := make(chan bool) + go detector.CheckGPGOnRequest(requestGPGCheck, notifiers, ctx) + go detector.WatchGPG(filesToWatch, requestGPGCheck) + go detector.WatchSSH(requestGPGCheck, exits) } wait := make(chan bool) <-wait } +func findMatchingFiles(folderPath, term string) ([]string, error) { + var result []string + err := filepath.Walk(folderPath, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if info.IsDir() { + return nil + } + file, err := os.Open(path) + if err != nil { + return err + } + defer file.Close() + + scanner := bufio.NewScanner(file) + for scanner.Scan() { + line := scanner.Text() + if strings.Contains(line, term) { + result = append(result, path) + break // No need to scan further lines if we already found the string + } + } + + if err := scanner.Err(); err != nil { + return err + } + return nil + }) + if err != nil { + return nil, err + } + return result, nil +} + func setupExitSignalWatch(exits *sync.Map) { exitSignal := make(chan os.Signal, 1) signal.Notify(exitSignal, os.Interrupt, syscall.SIGTERM) From 9e241294bc78c600b282ded51f6d08981c964863 Mon Sep 17 00:00:00 2001 From: Pablo Fraile Alonso Date: Thu, 21 Nov 2024 16:57:57 +0100 Subject: [PATCH 2/4] refactor: implement changes from #58 Co-authored-by: Maxim Baz --- detector/gpg.go | 16 ++++++++-------- main.go | 35 ++++++++++------------------------- 2 files changed, 18 insertions(+), 33 deletions(-) diff --git a/detector/gpg.go b/detector/gpg.go index c07c0d9..e0a30e5 100644 --- a/detector/gpg.go +++ b/detector/gpg.go @@ -13,16 +13,16 @@ import ( // WatchGPG watches for hints that YubiKey is maybe waiting for a touch on a GPG request func WatchGPG(filesToWatch []string, requestGPGCheck chan bool) { - events := make(chan notify.EventInfo, len(filesToWatch)) + events := make(chan notify.EventInfo) initWatcher := func() { - for _, file := range filesToWatch { - if err := notify.Watch(file, events, notify.InOpen); err != nil { - log.Errorf("Failed to watch file '%s': %v\n", file, err) - return; - } - log.Debug("GPG watcher is watching '%s'...\n", file) - } + for _, file := range filesToWatch { + if err := notify.Watch(file, events, notify.InOpen, notify.InDeleteSelf, notify.InMoveSelf); err != nil { + log.Errorf("Failed to watch file '%s': %v\n", file, err) + return + } + log.Debugf("GPG watcher is watching '%s'...\n", file) + } } initWatcher() diff --git a/main.go b/main.go index 229e7c4..9273a30 100644 --- a/main.go +++ b/main.go @@ -1,7 +1,6 @@ package main import ( - "bufio" "flag" "fmt" "os" @@ -80,14 +79,14 @@ func main() { } else if ctx.SetProtocol(gpgme.ProtocolAssuan) != nil { log.Debugf("Cannot initialize Assuan IPC: %v. Disabling GPG and SSH watchers.", err) } else { - var gpgPubringPath = path.Join(gpgme.GetDirInfo("homedir"), "private-keys-v1.d") - if _, err := os.Stat(gpgPubringPath); os.IsNotExist(err) { - fmt.Printf("Directory '%s' does not exist (you have no private keys).\n", gpgPubringPath) + var gpgPrivateKeysDirPath = path.Join(gpgme.GetDirInfo("homedir"), "private-keys-v1.d") + if _, err := os.Stat(gpgPrivateKeysDirPath); os.IsNotExist(err) { + log.Debugf("Directory '%s' does not exist (you have no private keys).\n", gpgPrivateKeysDirPath) return } - var searchTerm = "shadowed-private-key" + searchTerm := "shadowed-private-key" var filesToWatch []string - filesToWatch, err := findMatchingFiles(gpgPubringPath, searchTerm) + filesToWatch, err := findShadowedPrivateKeys(gpgPrivateKeysDirPath, searchTerm) if err != nil { fmt.Printf("Error finding files: %v\n", err) return @@ -105,32 +104,18 @@ func main() { <-wait } -func findMatchingFiles(folderPath, term string) ([]string, error) { +func findShadowedPrivateKeys(folderPath, term string) ([]string, error) { var result []string err := filepath.Walk(folderPath, func(path string, info os.FileInfo, err error) error { - if err != nil { + if err != nil || info.IsDir() { return err } - if info.IsDir() { - return nil - } - file, err := os.Open(path) + data, err := os.ReadFile(path) if err != nil { return err } - defer file.Close() - - scanner := bufio.NewScanner(file) - for scanner.Scan() { - line := scanner.Text() - if strings.Contains(line, term) { - result = append(result, path) - break // No need to scan further lines if we already found the string - } - } - - if err := scanner.Err(); err != nil { - return err + if strings.Contains(string(data), term) { + result = append(result, path) } return nil }) From d8b3f9dbbbc8370ab4690d54ec43f892121a6aea Mon Sep 17 00:00:00 2001 From: Maxim Baz Date: Thu, 21 Nov 2024 20:08:16 +0100 Subject: [PATCH 3/4] Minor code review improvements --- detector/gpg.go | 4 +++- main.go | 14 ++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/detector/gpg.go b/detector/gpg.go index e0a30e5..995bbc7 100644 --- a/detector/gpg.go +++ b/detector/gpg.go @@ -13,12 +13,14 @@ import ( // WatchGPG watches for hints that YubiKey is maybe waiting for a touch on a GPG request func WatchGPG(filesToWatch []string, requestGPGCheck chan bool) { + // No need for a buffered channel, + // we are interested only in the first event, it's ok to skip all subsequent ones events := make(chan notify.EventInfo) initWatcher := func() { for _, file := range filesToWatch { if err := notify.Watch(file, events, notify.InOpen, notify.InDeleteSelf, notify.InMoveSelf); err != nil { - log.Errorf("Failed to watch file '%s': %v\n", file, err) + log.Errorf("Failed to establish a watch on GPG file '%s': %v\n", file, err) return } log.Debugf("GPG watcher is watching '%s'...\n", file) diff --git a/main.go b/main.go index 9273a30..cd1408e 100644 --- a/main.go +++ b/main.go @@ -84,15 +84,13 @@ func main() { log.Debugf("Directory '%s' does not exist (you have no private keys).\n", gpgPrivateKeysDirPath) return } - searchTerm := "shadowed-private-key" - var filesToWatch []string - filesToWatch, err := findShadowedPrivateKeys(gpgPrivateKeysDirPath, searchTerm) + filesToWatch, err := findShadowedPrivateKeys(gpgPrivateKeysDirPath) if err != nil { - fmt.Printf("Error finding files: %v\n", err) + log.Debugf("Error finding shadowed private keys: %v\n", err) return } if len(filesToWatch) == 0 { - fmt.Printf("No files matching the term '%s' found.\n", searchTerm) + log.Debugf("No shadowed private keys found.\n") return } requestGPGCheck := make(chan bool) @@ -104,9 +102,9 @@ func main() { <-wait } -func findShadowedPrivateKeys(folderPath, term string) ([]string, error) { +func findShadowedPrivateKeys(folderPath string) ([]string, error) { var result []string - err := filepath.Walk(folderPath, func(path string, info os.FileInfo, err error) error { + err := filepath.WalkDir(folderPath, func(path string, info os.DirEntry, err error) error { if err != nil || info.IsDir() { return err } @@ -114,7 +112,7 @@ func findShadowedPrivateKeys(folderPath, term string) ([]string, error) { if err != nil { return err } - if strings.Contains(string(data), term) { + if strings.Contains(string(data), "shadowed-private-key") { result = append(result, path) } return nil From 664792394760a540f95a035a5a42c68567917ce3 Mon Sep 17 00:00:00 2001 From: Maxim Baz Date: Thu, 21 Nov 2024 20:11:11 +0100 Subject: [PATCH 4/4] Treat any Stat error as folder not existing --- main.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/main.go b/main.go index cd1408e..bb596cb 100644 --- a/main.go +++ b/main.go @@ -80,8 +80,8 @@ func main() { log.Debugf("Cannot initialize Assuan IPC: %v. Disabling GPG and SSH watchers.", err) } else { var gpgPrivateKeysDirPath = path.Join(gpgme.GetDirInfo("homedir"), "private-keys-v1.d") - if _, err := os.Stat(gpgPrivateKeysDirPath); os.IsNotExist(err) { - log.Debugf("Directory '%s' does not exist (you have no private keys).\n", gpgPrivateKeysDirPath) + if _, err := os.Stat(gpgPrivateKeysDirPath); err != nil { + log.Debugf("Directory '%s' does not exist or cannot stat it\n", gpgPrivateKeysDirPath) return } filesToWatch, err := findShadowedPrivateKeys(gpgPrivateKeysDirPath)