From d023cd6c95fcbc6b5056ba1f425d2ac3df4abc5a Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Fri, 10 Sep 2021 12:14:19 +0200 Subject: [PATCH] cli: drop -forcedecode flag The rewritten openssl backend does not support this flag anymore, and it was inherently dangerour. Drop it (ignored for compatibility) --- Documentation/MANPAGE.md | 15 +---------- cli_args.go | 33 +++--------------------- internal/configfile/config_file.go | 4 +-- internal/contentenc/content.go | 18 +++---------- internal/contentenc/content_test.go | 12 ++++----- internal/contentenc/offsets_test.go | 4 +-- internal/cryptocore/cryptocore.go | 8 +++--- internal/cryptocore/cryptocore_test.go | 8 +++--- internal/fusefrontend/args.go | 2 -- internal/fusefrontend/file.go | 14 +++------- internal/fusefrontend/xattr_unit_test.go | 4 +-- internal/speed/speed.go | 2 +- internal/speed/speed_test.go | 2 +- internal/stupidgcm/gcm.go | 2 +- internal/stupidgcm/gcm_test.go | 2 +- internal/stupidgcm/without_openssl.go | 2 +- mount.go | 9 ++----- 17 files changed, 37 insertions(+), 104 deletions(-) diff --git a/Documentation/MANPAGE.md b/Documentation/MANPAGE.md index 7371fc2..e4e8f2e 100644 --- a/Documentation/MANPAGE.md +++ b/Documentation/MANPAGE.md @@ -256,21 +256,8 @@ of a case where this may be useful is a situation where content is stored on a filesystem that doesn't properly support UNIX ownership and permissions. #### -forcedecode -Force decode of encrypted files even if the integrity check fails, instead of -failing with an IO error. Warning messages are still printed to syslog if corrupted -files are encountered. -It can be useful to recover files from disks with bad sectors or other corrupted -media. It shall not be used if the origin of corruption is unknown, specially -if you want to run executable files. -For corrupted media, note that you probably want to use dd_rescue(1) -instead, which will recover all but the corrupted 4kB block. - -This option makes no sense in reverse mode. It requires gocryptfs to be compiled with openssl -support and implies -openssl true. Because of this, it is not compatible with -aessiv, -that uses built-in Go crypto. - -Setting this option forces the filesystem to read-only and noexec. +Obsolete and ignored on gocryptfs v2.2 and later. #### -fsname string Override the filesystem name (first column in df -T). Can also be diff --git a/cli_args.go b/cli_args.go index d666b47..b415b21 100644 --- a/cli_args.go +++ b/cli_args.go @@ -29,7 +29,7 @@ type argContainer struct { debug, init, zerokey, fusedebug, openssl, passwd, fg, version, plaintextnames, quiet, nosyslog, wpanic, longnames, allow_other, reverse, aessiv, nonempty, raw64, - noprealloc, speed, hkdf, serialize_reads, forcedecode, hh, info, + noprealloc, speed, hkdf, serialize_reads, hh, info, sharedstorage, fsck, one_file_system, deterministic_names, xchacha bool // Mount options with opposites @@ -172,8 +172,6 @@ func parseCliOpts(osArgs []string) (args argContainer) { flagSet.BoolVar(&args.speed, "speed", false, "Run crypto speed test") flagSet.BoolVar(&args.hkdf, "hkdf", true, "Use HKDF as an additional key derivation step") flagSet.BoolVar(&args.serialize_reads, "serialize_reads", false, "Try to serialize read operations") - flagSet.BoolVar(&args.forcedecode, "forcedecode", false, "Force decode of files even if integrity check fails."+ - " Requires gocryptfs to be compiled with openssl support and implies -openssl true") flagSet.BoolVar(&args.hh, "hh", false, "Show this long help text") flagSet.BoolVar(&args.info, "info", false, "Display information about CIPHERDIR") flagSet.BoolVar(&args.sharedstorage, "sharedstorage", false, "Make concurrent access to a shared CIPHERDIR safer") @@ -234,7 +232,8 @@ func parseCliOpts(osArgs []string) (args argContainer) { { var tmp bool flagSet.BoolVar(&tmp, "nofail", false, "Ignored for /etc/fstab compatibility") - flagSet.BoolVar(&tmp, "devrandom", false, "Deprecated (ignored for compatibility)") + flagSet.BoolVar(&tmp, "devrandom", false, "Obsolete, ignored for compatibility") + flagSet.BoolVar(&tmp, "forcedecode", false, "Obsolete, ignored for compatibility") } // Actual parsing @@ -265,32 +264,6 @@ func parseCliOpts(osArgs []string) (args argContainer) { os.Exit(exitcodes.Usage) } } - // "-forcedecode" only works with openssl. Check compilation and command line parameters - if args.forcedecode { - if stupidgcm.BuiltWithoutOpenssl { - tlog.Fatal.Printf("The -forcedecode flag requires openssl support, but gocryptfs was compiled without it!") - os.Exit(exitcodes.Usage) - } - if args.aessiv { - tlog.Fatal.Printf("The -forcedecode and -aessiv flags are incompatible because they use different crypto libs (openssl vs native Go)") - os.Exit(exitcodes.Usage) - } - if args.reverse { - tlog.Fatal.Printf("The reverse mode and the -forcedecode option are not compatible") - os.Exit(exitcodes.Usage) - } - // Has the user explicitly disabled openssl using "-openssl=false/0"? - if !args.openssl && opensslAuto != "auto" { - tlog.Fatal.Printf("-forcedecode requires openssl, but is disabled via command-line option") - os.Exit(exitcodes.Usage) - } - args.openssl = true - - // Try to make it harder for the user to shoot himself in the foot. - args.ro = true - args.allow_other = false - args.ko = "noexec" - } if len(args.extpass) > 0 && len(args.passfile) != 0 { tlog.Fatal.Printf("The options -extpass and -passfile cannot be used at the same time") os.Exit(exitcodes.Usage) diff --git a/internal/configfile/config_file.go b/internal/configfile/config_file.go index 06b665b..828f034 100644 --- a/internal/configfile/config_file.go +++ b/internal/configfile/config_file.go @@ -298,8 +298,8 @@ func getKeyEncrypter(scryptHash []byte, useHKDF bool) *contentenc.ContentEnc { if useHKDF { IVLen = contentenc.DefaultIVBits } - cc := cryptocore.New(scryptHash, cryptocore.BackendGoGCM, IVLen, useHKDF, false) - ce := contentenc.New(cc, 4096, false) + cc := cryptocore.New(scryptHash, cryptocore.BackendGoGCM, IVLen, useHKDF) + ce := contentenc.New(cc, 4096) return ce } diff --git a/internal/contentenc/content.go b/internal/contentenc/content.go index 13e0ce0..3005bf5 100644 --- a/internal/contentenc/content.go +++ b/internal/contentenc/content.go @@ -13,7 +13,6 @@ import ( "github.com/hanwen/go-fuse/v2/fuse" "github.com/rfjakob/gocryptfs/v2/internal/cryptocore" - "github.com/rfjakob/gocryptfs/v2/internal/stupidgcm" "github.com/rfjakob/gocryptfs/v2/internal/tlog" ) @@ -41,8 +40,6 @@ type ContentEnc struct { allZeroBlock []byte // All-zero block of size IVBitLen/8, for fast compares allZeroNonce []byte - // Force decode even if integrity check fails (openSSL only) - forceDecode bool // Ciphertext block "sync.Pool" pool. Always returns cipherBS-sized byte // slices (usually 4128 bytes). @@ -60,9 +57,8 @@ type ContentEnc struct { } // New returns an initialized ContentEnc instance. -func New(cc *cryptocore.CryptoCore, plainBS uint64, forceDecode bool) *ContentEnc { - tlog.Debug.Printf("contentenc.New: plainBS=%d, forceDecode=%v", - plainBS, forceDecode) +func New(cc *cryptocore.CryptoCore, plainBS uint64) *ContentEnc { + tlog.Debug.Printf("contentenc.New: plainBS=%d", plainBS) if fuse.MAX_KERNEL_WRITE%plainBS != 0 { log.Panicf("unaligned MAX_KERNEL_WRITE=%d", fuse.MAX_KERNEL_WRITE) @@ -81,7 +77,6 @@ func New(cc *cryptocore.CryptoCore, plainBS uint64, forceDecode bool) *ContentEn cipherBS: cipherBS, allZeroBlock: make([]byte, cipherBS), allZeroNonce: make([]byte, cc.IVLen), - forceDecode: forceDecode, cBlockPool: newBPool(int(cipherBS)), CReqPool: newBPool(cReqSize), pBlockPool: newBPool(int(plainBS)), @@ -111,11 +106,7 @@ func (be *ContentEnc) DecryptBlocks(ciphertext []byte, firstBlockNo uint64, file var pBlock []byte pBlock, err = be.DecryptBlock(cBlock, blockNo, fileID) if err != nil { - if be.forceDecode && err == stupidgcm.ErrAuth { - tlog.Warn.Printf("DecryptBlocks: authentication failure in block #%d, overridden by forcedecode", firstBlockNo) - } else { - break - } + break } pBuf.Write(pBlock) be.pBlockPool.Put(pBlock) @@ -183,9 +174,6 @@ func (be *ContentEnc) DecryptBlock(ciphertext []byte, blockNo uint64, fileID []b if err != nil { tlog.Debug.Printf("DecryptBlock: %s, len=%d", err.Error(), len(ciphertextOrig)) tlog.Debug.Println(hex.Dump(ciphertextOrig)) - if be.forceDecode && err == stupidgcm.ErrAuth { - return plaintext, err - } return nil, err } diff --git a/internal/contentenc/content_test.go b/internal/contentenc/content_test.go index 9cc8753..4a4b2de 100644 --- a/internal/contentenc/content_test.go +++ b/internal/contentenc/content_test.go @@ -23,8 +23,8 @@ func TestSplitRange(t *testing.T) { testRange{6654, 8945}) key := make([]byte, cryptocore.KeyLen) - cc := cryptocore.New(key, cryptocore.BackendGoGCM, DefaultIVBits, true, false) - f := New(cc, DefaultBS, false) + cc := cryptocore.New(key, cryptocore.BackendGoGCM, DefaultIVBits, true) + f := New(cc, DefaultBS) for _, r := range ranges { parts := f.ExplodePlainRange(r.offset, r.length) @@ -51,8 +51,8 @@ func TestCiphertextRange(t *testing.T) { testRange{6654, 8945}) key := make([]byte, cryptocore.KeyLen) - cc := cryptocore.New(key, cryptocore.BackendGoGCM, DefaultIVBits, true, false) - f := New(cc, DefaultBS, false) + cc := cryptocore.New(key, cryptocore.BackendGoGCM, DefaultIVBits, true) + f := New(cc, DefaultBS) for _, r := range ranges { @@ -74,8 +74,8 @@ func TestCiphertextRange(t *testing.T) { func TestBlockNo(t *testing.T) { key := make([]byte, cryptocore.KeyLen) - cc := cryptocore.New(key, cryptocore.BackendGoGCM, DefaultIVBits, true, false) - f := New(cc, DefaultBS, false) + cc := cryptocore.New(key, cryptocore.BackendGoGCM, DefaultIVBits, true) + f := New(cc, DefaultBS) b := f.CipherOffToBlockNo(788) if b != 0 { diff --git a/internal/contentenc/offsets_test.go b/internal/contentenc/offsets_test.go index 768393c..b35964a 100644 --- a/internal/contentenc/offsets_test.go +++ b/internal/contentenc/offsets_test.go @@ -10,8 +10,8 @@ import ( // TestSizeToSize tests CipherSizeToPlainSize and PlainSizeToCipherSize func TestSizeToSize(t *testing.T) { key := make([]byte, cryptocore.KeyLen) - cc := cryptocore.New(key, cryptocore.BackendGoGCM, DefaultIVBits, true, false) - ce := New(cc, DefaultBS, false) + cc := cryptocore.New(key, cryptocore.BackendGoGCM, DefaultIVBits, true) + ce := New(cc, DefaultBS) const rangeMax = 10000 diff --git a/internal/cryptocore/cryptocore.go b/internal/cryptocore/cryptocore.go index dd7c98b..48386f8 100644 --- a/internal/cryptocore/cryptocore.go +++ b/internal/cryptocore/cryptocore.go @@ -73,9 +73,9 @@ type CryptoCore struct { // // Note: "key" is either the scrypt hash of the password (when decrypting // a config file) or the masterkey (when finally mounting the filesystem). -func New(key []byte, aeadType AEADTypeEnum, IVBitLen int, useHKDF bool, forceDecode bool) *CryptoCore { - tlog.Debug.Printf("cryptocore.New: key=%d bytes, aeadType=%v, IVBitLen=%d, useHKDF=%v, forceDecode=%v", - len(key), aeadType, IVBitLen, useHKDF, forceDecode) +func New(key []byte, aeadType AEADTypeEnum, IVBitLen int, useHKDF bool) *CryptoCore { + tlog.Debug.Printf("cryptocore.New: key=%d bytes, aeadType=%v, IVBitLen=%d, useHKDF=%v", + len(key), aeadType, IVBitLen, useHKDF) if len(key) != KeyLen { log.Panicf("Unsupported key length of %d bytes", len(key)) @@ -120,7 +120,7 @@ func New(key []byte, aeadType AEADTypeEnum, IVBitLen int, useHKDF bool, forceDec if IVBitLen != 128 { log.Panicf("stupidgcm only supports 128-bit IVs, you wanted %d", IVBitLen) } - aeadCipher = stupidgcm.NewAES256GCM(gcmKey, forceDecode) + aeadCipher = stupidgcm.NewAES256GCM(gcmKey) case BackendGoGCM: goGcmBlockCipher, err := aes.NewCipher(gcmKey) if err != nil { diff --git a/internal/cryptocore/cryptocore_test.go b/internal/cryptocore/cryptocore_test.go index 319a900..d37e941 100644 --- a/internal/cryptocore/cryptocore_test.go +++ b/internal/cryptocore/cryptocore_test.go @@ -10,18 +10,18 @@ import ( func TestCryptoCoreNew(t *testing.T) { key := make([]byte, 32) for _, useHKDF := range []bool{true, false} { - c := New(key, BackendGoGCM, 96, useHKDF, false) + c := New(key, BackendGoGCM, 96, useHKDF) if c.IVLen != 12 { t.Fail() } - c = New(key, BackendGoGCM, 128, useHKDF, false) + c = New(key, BackendGoGCM, 128, useHKDF) if c.IVLen != 16 { t.Fail() } if stupidgcm.BuiltWithoutOpenssl { continue } - c = New(key, BackendOpenSSL, 128, useHKDF, false) + c = New(key, BackendOpenSSL, 128, useHKDF) if c.IVLen != 16 { t.Fail() } @@ -37,5 +37,5 @@ func TestNewPanic(t *testing.T) { }() key := make([]byte, 16) - New(key, BackendOpenSSL, 128, true, false) + New(key, BackendOpenSSL, 128, true) } diff --git a/internal/fusefrontend/args.go b/internal/fusefrontend/args.go index 4aedf2e..64a5923 100644 --- a/internal/fusefrontend/args.go +++ b/internal/fusefrontend/args.go @@ -26,8 +26,6 @@ type Args struct { ConfigCustom bool // NoPrealloc disables automatic preallocation before writing NoPrealloc bool - // Force decode even if integrity check fails (openSSL only) - ForceDecode bool // Exclude is a list of paths to make inaccessible, starting match at // the filesystem root Exclude []string diff --git a/internal/fusefrontend/file.go b/internal/fusefrontend/file.go index 661c2b8..3ce1b1e 100644 --- a/internal/fusefrontend/file.go +++ b/internal/fusefrontend/file.go @@ -20,7 +20,6 @@ import ( "github.com/rfjakob/gocryptfs/v2/internal/contentenc" "github.com/rfjakob/gocryptfs/v2/internal/inomap" "github.com/rfjakob/gocryptfs/v2/internal/openfiletable" - "github.com/rfjakob/gocryptfs/v2/internal/stupidgcm" "github.com/rfjakob/gocryptfs/v2/internal/syscallcompat" "github.com/rfjakob/gocryptfs/v2/internal/tlog" ) @@ -208,16 +207,9 @@ func (f *File) doRead(dst []byte, off uint64, length uint64) ([]byte, syscall.Er plaintext, err := f.contentEnc.DecryptBlocks(ciphertext, firstBlockNo, fileID) f.rootNode.contentEnc.CReqPool.Put(ciphertext) if err != nil { - if f.rootNode.args.ForceDecode && err == stupidgcm.ErrAuth { - // We do not have the information which block was corrupt here anymore, - // but DecryptBlocks() has already logged it anyway. - tlog.Warn.Printf("doRead %d: off=%d len=%d: returning corrupt data due to forcedecode", - f.qIno.Ino, off, length) - } else { - curruptBlockNo := firstBlockNo + f.contentEnc.PlainOffToBlockNo(uint64(len(plaintext))) - tlog.Warn.Printf("doRead %d: corrupt block #%d: %v", f.qIno.Ino, curruptBlockNo, err) - return nil, syscall.EIO - } + curruptBlockNo := firstBlockNo + f.contentEnc.PlainOffToBlockNo(uint64(len(plaintext))) + tlog.Warn.Printf("doRead %d: corrupt block #%d: %v", f.qIno.Ino, curruptBlockNo, err) + return nil, syscall.EIO } // Crop down to the relevant part diff --git a/internal/fusefrontend/xattr_unit_test.go b/internal/fusefrontend/xattr_unit_test.go index 7449d24..5bffd5e 100644 --- a/internal/fusefrontend/xattr_unit_test.go +++ b/internal/fusefrontend/xattr_unit_test.go @@ -17,8 +17,8 @@ import ( func newTestFS(args Args) *RootNode { // Init crypto backend key := make([]byte, cryptocore.KeyLen) - cCore := cryptocore.New(key, cryptocore.BackendGoGCM, contentenc.DefaultIVBits, true, false) - cEnc := contentenc.New(cCore, contentenc.DefaultBS, false) + cCore := cryptocore.New(key, cryptocore.BackendGoGCM, contentenc.DefaultIVBits, true) + cEnc := contentenc.New(cCore, contentenc.DefaultBS) n := nametransform.New(cCore.EMECipher, true, true, nil, false) rn := NewRootNode(args, cEnc, n) oneSec := time.Second diff --git a/internal/speed/speed.go b/internal/speed/speed.go index f7cb5d3..9958697 100644 --- a/internal/speed/speed.go +++ b/internal/speed/speed.go @@ -114,7 +114,7 @@ func bStupidGCM(b *testing.B) { if stupidgcm.BuiltWithoutOpenssl { b.Skip("openssl has been disabled at compile-time") } - bEncrypt(b, stupidgcm.NewAES256GCM(randBytes(32), false)) + bEncrypt(b, stupidgcm.NewAES256GCM(randBytes(32))) } // bGoGCM benchmarks Go stdlib GCM diff --git a/internal/speed/speed_test.go b/internal/speed/speed_test.go index 11c68d0..5f3001b 100644 --- a/internal/speed/speed_test.go +++ b/internal/speed/speed_test.go @@ -31,7 +31,7 @@ func BenchmarkStupidGCMDecrypt(b *testing.B) { if stupidgcm.BuiltWithoutOpenssl { b.Skip("openssl has been disabled at compile-time") } - bDecrypt(b, stupidgcm.NewAES256GCM(randBytes(32), false)) + bDecrypt(b, stupidgcm.NewAES256GCM(randBytes(32))) } func BenchmarkGoGCM(b *testing.B) { diff --git a/internal/stupidgcm/gcm.go b/internal/stupidgcm/gcm.go index c38dd5f..00819dd 100644 --- a/internal/stupidgcm/gcm.go +++ b/internal/stupidgcm/gcm.go @@ -26,7 +26,7 @@ type stupidGCM struct { // NewAES256GCM returns a new AES-256-GCM cipher that satisfies the cipher.AEAD interface. // // Only 32-bytes keys and 16-byte IVs are supported. -func NewAES256GCM(keyIn []byte, forceDecode bool) cipher.AEAD { +func NewAES256GCM(keyIn []byte) cipher.AEAD { if len(keyIn) != keyLen { log.Panicf("Only %d-byte keys are supported", keyLen) } diff --git a/internal/stupidgcm/gcm_test.go b/internal/stupidgcm/gcm_test.go index b587e58..73668fa 100644 --- a/internal/stupidgcm/gcm_test.go +++ b/internal/stupidgcm/gcm_test.go @@ -13,7 +13,7 @@ import ( func TestStupidGCM(t *testing.T) { key := randBytes(32) - sGCM := NewAES256GCM(key, false) + sGCM := NewAES256GCM(key) gAES, err := aes.NewCipher(key) if err != nil { diff --git a/internal/stupidgcm/without_openssl.go b/internal/stupidgcm/without_openssl.go index 93efcb4..42604de 100644 --- a/internal/stupidgcm/without_openssl.go +++ b/internal/stupidgcm/without_openssl.go @@ -21,7 +21,7 @@ func errExit() { os.Exit(exitcodes.OpenSSL) } -func NewAES256GCM(_ []byte, _ bool) cipher.AEAD { +func NewAES256GCM(_ []byte) cipher.AEAD { errExit() return nil } diff --git a/mount.go b/mount.go index b1c76dd..dfabbc9 100644 --- a/mount.go +++ b/mount.go @@ -277,7 +277,6 @@ func initFuseFrontend(args *argContainer) (rootNode fs.InodeEmbedder, wipeKeys f LongNames: args.longnames, ConfigCustom: args._configCustom, NoPrealloc: args.noprealloc, - ForceDecode: args.forcedecode, ForceOwner: args._forceOwner, Exclude: args.exclude, ExcludeWildcard: args.excludeWildcard, @@ -323,8 +322,8 @@ func initFuseFrontend(args *argContainer) (rootNode fs.InodeEmbedder, wipeKeys f } // Init crypto backend - cCore := cryptocore.New(masterkey, cryptoBackend, IVBits, args.hkdf, args.forcedecode) - cEnc := contentenc.New(cCore, contentenc.DefaultBS, args.forcedecode) + cCore := cryptocore.New(masterkey, cryptoBackend, IVBits, args.hkdf) + cEnc := contentenc.New(cCore, contentenc.DefaultBS) nameTransform := nametransform.New(cCore.EMECipher, frontendArgs.LongNames, args.raw64, []string(args.badname), frontendArgs.DeterministicNames) // After the crypto backend is initialized, @@ -408,10 +407,6 @@ func initGoFuse(rootNode fs.InodeEmbedder, args *argContainer) *fuse.Server { if args.acl { mOpts.EnableAcl = true } - if args.forcedecode { - tlog.Info.Printf(tlog.ColorYellow + "THE OPTION \"-forcedecode\" IS ACTIVE. GOCRYPTFS WILL RETURN CORRUPT DATA!" + - tlog.ColorReset) - } // fusermount from libfuse 3.x removed the "nonempty" option and exits // with an error if it sees it. Only add it to the options on libfuse 2.x. if args.nonempty && haveFusermount2() {