From c57414b83b103d71ab56ffe51b69d56b28a6578a Mon Sep 17 00:00:00 2001
From: "M. J. Fromberger" <michael.j.fromberger@gmail.com>
Date: Mon, 26 Oct 2020 18:42:15 -0700
Subject: [PATCH] Fix padding removal when decrypting a config file. (#36)
OpenSSL uses PKCS#5 padding, and the decryption code was not removing it
correctly. In some cases, this causes the last line of the decrypted config to
be mangled and produces invalid results.
To support this:
- Move config loading to gauth.LoadConfigFile.
- Inject a hook to read the user's password.
- Add unit tests that decryption doesn't corrupt the result.
- Update module dependencies.
- Update Go versions in CI, and fix some config-check warnings.
---
.travis.yml | 8 ++---
gauth.go | 66 +++++++++++-------------------------
gauth/gauth.go | 54 +++++++++++++++++++++++++++++
gauth/gauth_test.go | 42 +++++++++++++++++++++++
gauth/testdata/encrypted.csv | 1 +
gauth/testdata/plaintext.csv | 3 ++
go.mod | 6 ++--
go.sum | 12 +++----
8 files changed, 132 insertions(+), 60 deletions(-)
create mode 100644 gauth/testdata/encrypted.csv
create mode 100644 gauth/testdata/plaintext.csv
diff --git a/.travis.yml b/.travis.yml
index 887b4e4..09d5ef3 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,9 +1,7 @@
-sudo: false
language: go
go:
- - 1.7
- - 1.8
- - 1.9
- - "1.10"
- 1.11
- 1.12
+ - 1.13
+ - 1.14
+ - 1.15
diff --git a/gauth.go b/gauth.go
index 59df65f..8cbd13e 100644
--- a/gauth.go
+++ b/gauth.go
@@ -2,12 +2,8 @@ package main
import (
"bytes"
- "crypto/aes"
- "crypto/cipher"
- "crypto/sha256"
"encoding/csv"
"fmt"
- "io/ioutil"
"log"
"os"
"os/user"
@@ -21,55 +17,27 @@ import (
)
func main() {
- user, e := user.Current()
- if e != nil {
- log.Fatal(e)
- }
- cfgPath := path.Join(user.HomeDir, ".config/gauth.csv")
-
- cfgContent, e := ioutil.ReadFile(cfgPath)
- if e != nil {
- log.Fatal(e)
+ cfgPath := os.Getenv("GAUTH_CONFIG")
+ if cfgPath == "" {
+ user, err := user.Current()
+ if err != nil {
+ log.Fatal(err)
+ }
+ cfgPath = path.Join(user.HomeDir, ".config/gauth.csv")
}
- // Support for 'openssl enc -aes-128-cbc -md sha256 -pass pass:'
- if bytes.HasPrefix(cfgContent, []byte("Salted__")) {
- fmt.Printf("Encryption password: ")
- passwd, e := terminal.ReadPassword(int(syscall.Stdin))
- fmt.Printf("\n")
- if e != nil {
- log.Fatal(e)
- }
- salt := cfgContent[8:16]
- rest := cfgContent[16:]
- salting := sha256.New()
- salting.Write([]byte(passwd))
- salting.Write(salt)
- sum := salting.Sum(nil)
- key := sum[:16]
- iv := sum[16:]
- block, e := aes.NewCipher(key)
- if e != nil {
- log.Fatal(e)
- }
-
- mode := cipher.NewCBCDecrypter(block, iv)
- mode.CryptBlocks(rest, rest)
- // Remove padding
- i := len(rest) - 1
- for rest[i] < 16 {
- i--
- }
- cfgContent = rest[:i]
+ cfgContent, err := gauth.LoadConfigFile(cfgPath, getPassword)
+ if err != nil {
+ log.Fatalf("Loading config: %v", err)
}
cfgReader := csv.NewReader(bytes.NewReader(cfgContent))
// Unix-style tabular
cfgReader.Comma = ':'
- cfg, e := cfgReader.ReadAll()
- if e != nil {
- log.Fatal(e)
+ cfg, err := cfgReader.ReadAll()
+ if err != nil {
+ log.Fatalf("Decoding CSV: %v", err)
}
currentTS, progress := gauth.IndexNow()
@@ -80,10 +48,16 @@ func main() {
name, secret := record[0], record[1]
prev, curr, next, err := gauth.Codes(secret, currentTS)
if err != nil {
- log.Fatalf("Code: %v", err)
+ log.Fatalf("Generating codes: %v", err)
}
fmt.Fprintf(tw, "%s\t%s\t%s\t%s\n", name, prev, curr, next)
}
tw.Flush()
fmt.Printf("[%-29s]\n", strings.Repeat("=", progress))
}
+
+func getPassword() ([]byte, error) {
+ fmt.Printf("Encryption password: ")
+ defer fmt.Println()
+ return terminal.ReadPassword(int(syscall.Stdin))
+}
diff --git a/gauth/gauth.go b/gauth/gauth.go
index 330790e..612df31 100644
--- a/gauth/gauth.go
+++ b/gauth/gauth.go
@@ -3,6 +3,13 @@
package gauth
import (
+ "bytes"
+ "crypto/aes"
+ "crypto/cipher"
+ "crypto/sha256"
+ "errors"
+ "fmt"
+ "io/ioutil"
"time"
"github.com/creachadair/otp"
@@ -28,3 +35,50 @@ func Codes(sec string, ts int64) (prev, curr, next string, _ error) {
next = cfg.HOTP(uint64(ts + 1))
return
}
+
+// LoadConfigFile reads and decrypts, if necessary, the CSV config at path.
+// The getPass function is called to obtain a password if needed.
+func LoadConfigFile(path string, getPass func() ([]byte, error)) ([]byte, error) {
+ data, err := ioutil.ReadFile(path)
+ if err != nil {
+ return nil, err
+ }
+
+ if !bytes.HasPrefix(data, []byte("Salted__")) {
+ return data, nil // not encrypted
+ }
+
+ // Support for 'openssl enc -aes-128-cbc -md sha256 -pass pass:'
+ passwd, err := getPass()
+ if err != nil {
+ return nil, fmt.Errorf("reading passphrase: %v", err)
+ }
+
+ salt := data[8:16]
+ rest := data[16:]
+ salting := sha256.New()
+ salting.Write([]byte(passwd))
+ salting.Write(salt)
+ sum := salting.Sum(nil)
+ key := sum[:16]
+ iv := sum[16:]
+ block, err := aes.NewCipher(key)
+ if err != nil {
+ return nil, fmt.Errorf("creating cipher: %v", err)
+ }
+
+ mode := cipher.NewCBCDecrypter(block, iv)
+ mode.CryptBlocks(rest, rest)
+
+ // Remove CBC padding and verify that the key was valid.
+ pad := int(rest[len(rest)-1])
+ if pad == 0 || pad > len(rest) {
+ return nil, errors.New("invalid decryption key")
+ }
+ for i := len(rest) - pad; i < len(rest); i++ {
+ if int(rest[i]) != pad {
+ return nil, errors.New("invalid block padding")
+ }
+ }
+ return rest[:len(rest)-int(pad)], nil
+}
diff --git a/gauth/gauth_test.go b/gauth/gauth_test.go
index ffe343e..de93664 100644
--- a/gauth/gauth_test.go
+++ b/gauth/gauth_test.go
@@ -1,6 +1,7 @@
package gauth_test
import (
+ "bytes"
"testing"
"github.com/pcarrier/gauth/gauth"
@@ -28,3 +29,44 @@ func TestCodes(t *testing.T) {
}
}
}
+
+//go:generate openssl enc -aes-128-cbc -md sha256 -pass pass:x -in testdata/plaintext.csv -out testdata/encrypted.csv
+
+func TestLoadConfig(t *testing.T) {
+
+ // To update test data, edit testdata/plaintext.csv as desired,
+ // then run go generate ./...
+ // If you change the passphrase, update getPass below.
+ //
+ // For this test, the contents don't actually matter.
+
+ var calledGetPass bool
+
+ getPass := func() ([]byte, error) {
+ calledGetPass = true
+ return []byte("x"), nil
+ }
+
+ // Load the plaintext configuration file, and verify that we did not try to
+ // decrypt its content.
+ plain, err := gauth.LoadConfigFile("testdata/plaintext.csv", getPass)
+ if err != nil {
+ t.Fatalf("Loading plaintext config: %v", err)
+ } else if calledGetPass {
+ t.Error("Loading plaintext unexpectedly called getPass")
+ calledGetPass = false
+ }
+
+ // Load the encrypted configuration file, and verify that we were able to
+ // decrypt it successfully.
+ enc, err := gauth.LoadConfigFile("testdata/encrypted.csv", getPass)
+ if err != nil {
+ t.Fatalf("Loading encrypted config: %v", err)
+ } else if !calledGetPass {
+ t.Error("Loading encrypted did not call getPass")
+ }
+
+ if !bytes.Equal(plain, enc) {
+ t.Errorf("Decrypted not equal to plaintext:\ngot %+v\nwant %+v", enc, plain)
+ }
+}
diff --git a/gauth/testdata/encrypted.csv b/gauth/testdata/encrypted.csv
new file mode 100644
index 0000000..0185d1b
--- /dev/null
+++ b/gauth/testdata/encrypted.csv
@@ -0,0 +1 @@
+Salted__$1�:�'�� �m����E�2:���^� �%ՋY��L*��S:��=��K�۞
\ No newline at end of file
diff --git a/gauth/testdata/plaintext.csv b/gauth/testdata/plaintext.csv
new file mode 100644
index 0000000..ec462d0
--- /dev/null
+++ b/gauth/testdata/plaintext.csv
@@ -0,0 +1,3 @@
+test2:AEBAGBAFAYDQQCIK
+test1:AAAQEAYEAUDAOCAJ
+
diff --git a/go.mod b/go.mod
index d714e1f..16136bc 100644
--- a/go.mod
+++ b/go.mod
@@ -3,7 +3,7 @@ module github.com/pcarrier/gauth
go 1.12
require (
- github.com/creachadair/otp v0.1.0
- golang.org/x/crypto v0.0.0-20190530122614-20be4c3c3ed5
- golang.org/x/sys v0.0.0-20190531175056-4c3a928424d2 // indirect
+ github.com/creachadair/otp v0.1.1
+ golang.org/x/crypto v0.0.0-20201016220609-9e8e0b390897
+ golang.org/x/sys v0.0.0-20201020230747-6e5568b54d1a // indirect
)
diff --git a/go.sum b/go.sum
index 9a68a85..17e2488 100644
--- a/go.sum
+++ b/go.sum
@@ -1,12 +1,12 @@
-github.com/creachadair/otp v0.1.0 h1:JgsbBS3KYbZ7/HE4t5ylRIFRjtDrhzpUHwmpZc03INY=
-github.com/creachadair/otp v0.1.0/go.mod h1:vPuEqgSogZ1vtpF8OeUg28ke/PK2FIo85GZHJz74d0M=
+github.com/creachadair/otp v0.1.1 h1:SMeGZefF9eP+QjDGCRbW5a5mptIaP+HkMvzV+OhsukA=
+github.com/creachadair/otp v0.1.1/go.mod h1:vPuEqgSogZ1vtpF8OeUg28ke/PK2FIo85GZHJz74d0M=
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
-golang.org/x/crypto v0.0.0-20190530122614-20be4c3c3ed5 h1:8dUaAV7K4uHsF56JQWkprecIQKdPHtR9jCHF5nB8uzc=
-golang.org/x/crypto v0.0.0-20190530122614-20be4c3c3ed5/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
+golang.org/x/crypto v0.0.0-20201016220609-9e8e0b390897 h1:pLI5jrR7OSLijeIDcmRxNmw2api+jEfxLoykJVice/E=
+golang.org/x/crypto v0.0.0-20201016220609-9e8e0b390897/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a h1:1BGLXjeY4akVXGgbC9HugT3Jv3hCI0z56oJR5vAMgBU=
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
-golang.org/x/sys v0.0.0-20190531175056-4c3a928424d2 h1:T5DasATyLQfmbTpfEXx/IOL9vfjzW6up+ZDkmHvIf2s=
-golang.org/x/sys v0.0.0-20190531175056-4c3a928424d2/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.0.0-20201020230747-6e5568b54d1a h1:e3IU37lwO4aq3uoRKINC7JikojFmE5gO7xhfxs8VC34=
+golang.org/x/sys v0.0.0-20201020230747-6e5568b54d1a/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=