From cf103a6528cddfc1d0bc3e2976f408c741000b08 Mon Sep 17 00:00:00 2001 From: alexmullins Date: Thu, 5 Nov 2015 23:12:14 -0600 Subject: [PATCH] defer authentication/buffering to Read not Open --- README.txt | 13 +++--- crypto.go | 109 ++++++++++++++++++++++++++++++++++++++++--------- reader.go | 25 +++++++++--- reader_test.go | 94 ------------------------------------------ 4 files changed, 113 insertions(+), 128 deletions(-) diff --git a/README.txt b/README.txt index 44971bb..33436bd 100644 --- a/README.txt +++ b/README.txt @@ -6,14 +6,12 @@ This package DOES NOT intend to implement the encryption methods mentioned in the original PKWARE spec (sections 6.0 and 7.0): https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT - The process -============================================================================== +============ hello.txt -> compressed -> encrypted -> .zip -> decrypted -> decompressed -> hello.txt - Roadmap -============================================================================== +======== Reading - Done. TODO: 1. Change to streaming authentication and decryption. (Maybe not such a good @@ -22,9 +20,8 @@ Reading - Done. Writing - Not started. Testing - Needs more. - WinZip AES specifies -============================================================================== +===================== 1. Encryption-Decryption w/ AES-CTR (128, 192, or 256 bits) 2. Key generation with PBKDF2-HMAC-SHA1 (1000 iteration count) that generates a master key broken into the following: @@ -71,8 +68,8 @@ used that was replaced by the encryption process mentioned in #8. 15. AE-1 keeps the CRC and should be verified after decompression. AE-2 removes the CRC and shouldn't be verified after decompression. Refer to http://www.winzip.com/aes_info.htm#winzip11 for the reasoning. -16. Storage Format (file data payload) totals CompressedSize64 bytes: +16. Storage Format (file data payload totals CompressedSize64 bytes): a. Salt - 8, 12, or 16 bytes depending on keysize b. Password Verification Value - 2 bytes - c. Encrypted Data - compressed size - salt - pwv - auth lengths + c. Encrypted Data - compressed size - salt - pwv - auth code lengths d. Authentication code - 10 bytes diff --git a/crypto.go b/crypto.go index 09206f1..e5f00e1 100644 --- a/crypto.go +++ b/crypto.go @@ -10,13 +10,20 @@ import ( "crypto/cipher" "crypto/hmac" "crypto/sha1" + "crypto/subtle" + "errors" "io" + "io/ioutil" "golang.org/x/crypto/pbkdf2" ) -// Counter (CTR) mode. +// Decryption Errors +var ( + ErrDecryption = errors.New("zip: decryption error") +) +// Counter (CTR) mode. // CTR converts a block cipher into a stream cipher by // repeatedly encrypting an incrementing counter and // xoring the resulting stream of data with the input. @@ -111,6 +118,61 @@ func xorBytes(dst, a, b []byte) int { return n } +type authReader struct { + data io.Reader // data to be authenticated + adata io.Reader // the authentication code to read + akey []byte // authentication key + buf *bytes.Buffer // buffer to store data to authenticate + err error + auth bool +} + +func newAuthReader(akey []byte, data, adata io.Reader) io.Reader { + return &authReader{ + data: data, + adata: adata, + akey: akey, + buf: new(bytes.Buffer), + err: nil, + auth: false, + } +} + +// Read will fully buffer the file data payload to authenticate first. +// If authentication fails, returns ErrDecryption immediately. +// Else, sends data along for decryption. +func (a *authReader) Read(b []byte) (int, error) { + // check for sticky error + if a.err != nil { + return 0, a.err + } + // make sure we have auth'ed before we send any data + if !a.auth { + nn, err := io.Copy(a.buf, a.data) + if err != nil { + a.err = ErrDecryption + return 0, a.err + } + ab := new(bytes.Buffer) + nn, err = io.Copy(ab, a.adata) + if err != nil || nn != 10 { + a.err = ErrDecryption + return 0, a.err + } + a.auth = checkAuthentication(a.buf.Bytes(), ab.Bytes(), a.akey) + if !a.auth { + a.err = ErrDecryption + return 0, a.err + } + } + // so we've authenticated the data, now just pass it on. + n, err := a.buf.Read(b) + if err != nil { + a.err = err + } + return n, a.err +} + func checkAuthentication(message, authcode, key []byte) bool { mac := hmac.New(sha1.New, key) mac.Write(message) @@ -118,7 +180,13 @@ func checkAuthentication(message, authcode, key []byte) bool { // Truncate at the first 10 bytes expectedAuthCode = expectedAuthCode[:10] // Change to use crypto/subtle for constant time comparison - return bytes.Equal(expectedAuthCode, authcode) + b := subtle.ConstantTimeCompare(expectedAuthCode, authcode) > 0 + return b +} + +func checkPasswordVerification(pwvv, pwv []byte) bool { + b := subtle.ConstantTimeCompare(pwvv, pwv) > 0 + return b } func generateKeys(password, salt []byte, keySize int) (encKey, authKey, pwv []byte) { @@ -130,7 +198,7 @@ func generateKeys(password, salt []byte, keySize int) (encKey, authKey, pwv []by return } -func newDecryptionReader(r io.Reader, f *File) (io.Reader, error) { +func newDecryptionReader(r *io.SectionReader, f *File) (io.ReadCloser, error) { keyLen := aesKeyLen(f.aesStrength) saltLen := keyLen / 2 // salt is half of key len if saltLen == 0 { @@ -141,38 +209,39 @@ func newDecryptionReader(r io.Reader, f *File) (io.Reader, error) { // See: // https://www.imperialviolet.org/2014/06/27/streamingencryption.html // https://www.imperialviolet.org/2015/05/16/aeads.html - content := make([]byte, f.CompressedSize64) - if _, err := io.ReadFull(r, content); err != nil { + // grab the salt, pwvv, data, and authcode + saltpwvv := make([]byte, saltLen+2) + if _, err := r.Read(saltpwvv); err != nil { return nil, ErrDecryption } - // grab the salt, pwvv, data, and authcode - salt := content[:saltLen] - pwvv := content[saltLen : saltLen+2] - content = content[saltLen+2:] - size := f.CompressedSize64 - uint64(saltLen) - 2 - 10 - data := content[:size] - authcode := content[size:] + salt := saltpwvv[:saltLen] + pwvv := saltpwvv[saltLen : saltLen+2] + dataOff := int64(saltLen + 2) + dataLen := int64(f.CompressedSize64 - uint64(saltLen) - 2 - 10) + data := io.NewSectionReader(r, dataOff, dataLen) + authOff := dataOff + dataLen + authcode := io.NewSectionReader(r, authOff, 10) // generate keys decKey, authKey, pwv := generateKeys(f.password, salt, keyLen) // check password verifier (pwv) // Change to use crypto/subtle for constant time comparison - if !bytes.Equal(pwv, pwvv) { + if !checkPasswordVerification(pwv, pwvv) { return nil, ErrDecryption } - // check authentication - if !checkAuthentication(data, authcode, authKey) { - return nil, ErrDecryption - } - return decryptStream(data, decKey), nil + // setup auth reader + ar := newAuthReader(authKey, data, authcode) + // return decryption reader + dr := decryptStream(decKey, ar) + return ioutil.NopCloser(dr), nil } -func decryptStream(ciphertext, key []byte) io.Reader { +func decryptStream(key []byte, ciphertext io.Reader) io.Reader { block, err := aes.NewCipher(key) if err != nil { return nil } stream := newWinZipCTR(block) - reader := cipher.StreamReader{S: stream, R: bytes.NewReader(ciphertext)} + reader := &cipher.StreamReader{S: stream, R: ciphertext} return reader } diff --git a/reader.go b/reader.go index 269c714..1e6dee5 100644 --- a/reader.go +++ b/reader.go @@ -16,10 +16,9 @@ import ( ) var ( - ErrFormat = errors.New("zip: not a valid zip file") - ErrAlgorithm = errors.New("zip: unsupported compression algorithm") - ErrChecksum = errors.New("zip: checksum error") - ErrDecryption = errors.New("zip: decryption error") + ErrFormat = errors.New("zip: not a valid zip file") + ErrAlgorithm = errors.New("zip: unsupported compression algorithm") + ErrChecksum = errors.New("zip: checksum error") ) type Reader struct { @@ -53,6 +52,10 @@ func (f *File) IsEncrypted() bool { return f.Flags&0x1 == 1 } +func (f *File) isAE2() bool { + return f.ae == 2 +} + func (f *File) hasDataDescriptor() bool { return f.Flags&0x8 != 0 } @@ -156,12 +159,14 @@ func (f *File) Open() (rc io.ReadCloser, err error) { // and auth code lengths size := int64(f.CompressedSize64) var r io.Reader - r = io.NewSectionReader(f.zipr, f.headerOffset+bodyOffset, size) + rr := io.NewSectionReader(f.zipr, f.headerOffset+bodyOffset, size) // check for encryption if f.IsEncrypted() { - if r, err = newDecryptionReader(r, f); err != nil { + if r, err = newDecryptionReader(rr, f); err != nil { return } + } else { + r = rr } dcomp := decompressor(f.Method) if dcomp == nil { @@ -174,6 +179,14 @@ func (f *File) Open() (rc io.ReadCloser, err error) { if f.hasDataDescriptor() { desr = io.NewSectionReader(f.zipr, f.headerOffset+bodyOffset+size, dataDescriptorLen) } + // if !f.isAE2() { + // rc = &checksumReader{ + // rc: rc, + // hash: crc32.NewIEEE(), + // f: f, + // desr: desr, + // } + // } rc = &checksumReader{ rc: rc, hash: crc32.NewIEEE(), diff --git a/reader_test.go b/reader_test.go index 2f4ee76..547dd39 100644 --- a/reader_test.go +++ b/reader_test.go @@ -605,97 +605,3 @@ func TestIssue11146(t *testing.T) { } r.Close() } - -func TestSimplePassword(t *testing.T) { - file := "hello-aes.zip" - var buf bytes.Buffer - r, err := OpenReader(filepath.Join("testdata", file)) - if err != nil { - t.Errorf("Expected %s to open: %v.", file, err) - } - defer r.Close() - if len(r.File) != 1 { - t.Errorf("Expected %s to contain one file.", file) - } - f := r.File[0] - if f.FileInfo().Name() != "hello.txt" { - t.Errorf("Expected %s to have a file named hello.txt", file) - } - if f.Method != 0 { - t.Errorf("Expected %s to have its Method set to 0.", file) - } - f.SetPassword([]byte("golang")) - rc, err := f.Open() - if err != nil { - t.Errorf("Expected to open the readcloser: %v.", err) - } - _, err = io.Copy(&buf, rc) - if err != nil { - t.Errorf("Expected to copy bytes: %v.", err) - } - if !bytes.Contains(buf.Bytes(), []byte("Hello World\r\n")) { - t.Errorf("Expected contents were not found.") - } -} - -func TestHelloWorldAes(t *testing.T) { - file := "world-aes.zip" - expecting := "helloworld" - r, err := OpenReader(filepath.Join("testdata", file)) - if err != nil { - t.Errorf("Expected %s to open: %v", file, err) - } - defer r.Close() - if len(r.File) != 2 { - t.Errorf("Expected %s to contain two files.", file) - } - var b bytes.Buffer - for _, f := range r.File { - if !f.IsEncrypted() { - t.Errorf("Expected %s to be encrypted.", f.FileInfo().Name) - } - f.SetPassword([]byte("golang")) - rc, err := f.Open() - if err != nil { - t.Errorf("Expected to open readcloser: %v", err) - } - defer rc.Close() - if _, err := io.Copy(&b, rc); err != nil { - t.Errorf("Expected to copy bytes to buffer: %v", err) - } - } - if !bytes.Equal([]byte(expecting), b.Bytes()) { - t.Errorf("Expected ending content to be %s instead of %s", expecting, b.Bytes()) - } -} - -func TestMacbethAct1(t *testing.T) { - file := "macbeth-act1.zip" - expecting := "Exeunt" - var b bytes.Buffer - r, err := OpenReader(filepath.Join("testdata", file)) - if err != nil { - t.Errorf("Expected %s to open: %v", file, err) - } - defer r.Close() - for _, f := range r.File { - if !f.IsEncrypted() { - t.Errorf("Expected %s to be encrypted.", f.Name) - } - f.SetPassword([]byte("golang")) - rc, err := f.Open() - if err != nil { - t.Errorf("Expected to open readcloser: %v", err) - } - defer rc.Close() - if _, err := io.Copy(&b, rc); err != nil { - t.Errorf("Expected to copy bytes to buffer: %v", err) - } - } - if !bytes.Contains(b.Bytes(), []byte(expecting)) { - t.Errorf("Expected to find %s in the buffer %v", expecting, b.Bytes()) - } -} - -// Test for AE-1 vs AE-2 -// Test for tampered data payload, use messWith