From add07bdad9c7162c9cd0898ff615669f95076354 Mon Sep 17 00:00:00 2001 From: Daniel Huigens Date: Mon, 9 Dec 2024 18:08:57 +0100 Subject: [PATCH 1/6] Don't allocate the nonce for each chunk --- openpgp/packet/aead_crypter.go | 10 +++++----- openpgp/packet/aead_encrypted.go | 2 +- openpgp/packet/aead_encrypted_test.go | 2 +- openpgp/packet/symmetrically_encrypted_aead.go | 14 +++++++------- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/openpgp/packet/aead_crypter.go b/openpgp/packet/aead_crypter.go index 2eecd062f..b23a00d78 100644 --- a/openpgp/packet/aead_crypter.go +++ b/openpgp/packet/aead_crypter.go @@ -15,7 +15,7 @@ import ( type aeadCrypter struct { aead cipher.AEAD chunkSize int - initialNonce []byte + nonce []byte associatedData []byte // Chunk-independent associated data chunkIndex []byte // Chunk counter packetTag packetType // SEIP packet (v2) or AEAD Encrypted Data packet @@ -28,12 +28,12 @@ type aeadCrypter struct { // 5.16.1 and 5.16.2). It returns the resulting nonce. func (wo *aeadCrypter) computeNextNonce() (nonce []byte) { if wo.packetTag == packetTypeSymmetricallyEncryptedIntegrityProtected { - return append(wo.initialNonce, wo.chunkIndex...) + return wo.nonce } - nonce = make([]byte, len(wo.initialNonce)) - copy(nonce, wo.initialNonce) - offset := len(wo.initialNonce) - 8 + nonce = make([]byte, len(wo.nonce)) + copy(nonce, wo.nonce) + offset := len(wo.nonce) - 8 for i := 0; i < 8; i++ { nonce[i+offset] ^= wo.chunkIndex[i] } diff --git a/openpgp/packet/aead_encrypted.go b/openpgp/packet/aead_encrypted.go index 98bd876bf..89a611efb 100644 --- a/openpgp/packet/aead_encrypted.go +++ b/openpgp/packet/aead_encrypted.go @@ -76,7 +76,7 @@ func (ae *AEADEncrypted) decrypt(key []byte) (io.ReadCloser, error) { aeadCrypter: aeadCrypter{ aead: aead, chunkSize: chunkSize, - initialNonce: ae.initialNonce, + nonce: ae.initialNonce, associatedData: ae.associatedData(), chunkIndex: make([]byte, 8), packetTag: packetTypeAEADEncrypted, diff --git a/openpgp/packet/aead_encrypted_test.go b/openpgp/packet/aead_encrypted_test.go index 97736071a..38dc9168c 100644 --- a/openpgp/packet/aead_encrypted_test.go +++ b/openpgp/packet/aead_encrypted_test.go @@ -454,7 +454,7 @@ func SerializeAEADEncrypted(w io.Writer, key []byte, config *Config) (io.WriteCl chunkSize: chunkSize, associatedData: prefix, chunkIndex: make([]byte, 8), - initialNonce: nonce, + nonce: nonce, packetTag: packetTypeAEADEncrypted, }, writer: writer, diff --git a/openpgp/packet/symmetrically_encrypted_aead.go b/openpgp/packet/symmetrically_encrypted_aead.go index 3957b2d53..377273dd7 100644 --- a/openpgp/packet/symmetrically_encrypted_aead.go +++ b/openpgp/packet/symmetrically_encrypted_aead.go @@ -81,9 +81,9 @@ func (se *SymmetricallyEncrypted) decryptAead(inputKey []byte) (io.ReadCloser, e aeadCrypter: aeadCrypter{ aead: aead, chunkSize: decodeAEADChunkSize(se.ChunkSizeByte), - initialNonce: nonce, + nonce: nonce, associatedData: se.associatedData(), - chunkIndex: make([]byte, 8), + chunkIndex: nonce[len(nonce)-8:], packetTag: packetTypeSymmetricallyEncryptedIntegrityProtected, }, reader: se.Contents, @@ -135,8 +135,8 @@ func serializeSymmetricallyEncryptedAead(ciphertext io.WriteCloser, cipherSuite aead: aead, chunkSize: decodeAEADChunkSize(chunkSizeByte), associatedData: prefix, - chunkIndex: make([]byte, 8), - initialNonce: nonce, + nonce: nonce, + chunkIndex: nonce[len(nonce)-8:], packetTag: packetTypeSymmetricallyEncryptedIntegrityProtected, }, writer: ciphertext, @@ -149,10 +149,10 @@ func getSymmetricallyEncryptedAeadInstance(c CipherFunction, mode AEADMode, inpu encryptionKey := make([]byte, c.KeySize()) _, _ = readFull(hkdfReader, encryptionKey) - // Last 64 bits of nonce are the counter - nonce = make([]byte, mode.IvLength()-8) + nonce = make([]byte, mode.IvLength()) - _, _ = readFull(hkdfReader, nonce) + // Last 64 bits of nonce are the counter + _, _ = readFull(hkdfReader, nonce[:len(nonce)-8]) blockCipher := c.new(encryptionKey) aead = mode.new(blockCipher) From 6fa7f918eaaa1ff22e1b16dfeed2041ec117fe2a Mon Sep 17 00:00:00 2001 From: Daniel Huigens Date: Mon, 9 Dec 2024 18:09:44 +0100 Subject: [PATCH 2/6] Preallocate the chunk size rather than buffering Since the chunk size is capped at 4MB now, we can safely preallocate it so that we don't have to buffer each chunk. --- openpgp/packet/aead_crypter.go | 54 ++++++------------- openpgp/packet/aead_encrypted.go | 10 ++-- openpgp/packet/aead_encrypted_test.go | 1 + .../packet/symmetrically_encrypted_aead.go | 5 +- 4 files changed, 29 insertions(+), 41 deletions(-) diff --git a/openpgp/packet/aead_crypter.go b/openpgp/packet/aead_crypter.go index b23a00d78..a85a9789e 100644 --- a/openpgp/packet/aead_crypter.go +++ b/openpgp/packet/aead_crypter.go @@ -62,8 +62,8 @@ func (wo *aeadCrypter) incrementIndex() error { type aeadDecrypter struct { aeadCrypter // Embedded ciphertext opener reader io.Reader // 'reader' is a partialLengthReader + chunkBytes []byte peekedBytes []byte // Used to detect last chunk - eof bool } // Read decrypts bytes and reads them into dst. It decrypts when necessary and @@ -75,22 +75,18 @@ func (ar *aeadDecrypter) Read(dst []byte) (n int, err error) { return ar.buffer.Read(dst) } - // Return EOF if we've previously validated the final tag - if ar.eof { - return 0, io.EOF - } - // Read a chunk tagLen := ar.aead.Overhead() - cipherChunkBuf := new(bytes.Buffer) - _, errRead := io.CopyN(cipherChunkBuf, ar.reader, int64(ar.chunkSize+tagLen)) - cipherChunk := cipherChunkBuf.Bytes() - if errRead != nil && errRead != io.EOF { + copy(ar.chunkBytes, ar.peekedBytes) // Copy bytes peeked in previous chunk or in initialization + bytesRead, errRead := io.ReadFull(ar.reader, ar.chunkBytes[tagLen:]) + if errRead != nil && errRead != io.EOF && errRead != io.ErrUnexpectedEOF { return 0, errRead } - if len(cipherChunk) > 0 { - decrypted, errChunk := ar.openChunk(cipherChunk) + if bytesRead > 0 { + ar.peekedBytes = ar.chunkBytes[bytesRead:bytesRead+tagLen] + + decrypted, errChunk := ar.openChunk(ar.chunkBytes[:bytesRead]) if errChunk != nil { return 0, errChunk } @@ -102,28 +98,19 @@ func (ar *aeadDecrypter) Read(dst []byte) (n int, err error) { } else { n = copy(dst, decrypted) } + return } - // Check final authentication tag - if errRead == io.EOF { - errChunk := ar.validateFinalTag(ar.peekedBytes) - if errChunk != nil { - return n, errChunk - } - ar.eof = true // Mark EOF for when we've returned all buffered data - } - return + return 0, io.EOF } -// Close is noOp. The final authentication tag of the stream was already -// checked in the last Read call. In the future, this function could be used to -// wipe the reader and peeked, decrypted bytes, if necessary. +// Close checks the final authentication tag of the stream. +// In the future, this function could also be used to wipe the reader +// and peeked & decrypted bytes, if necessary. func (ar *aeadDecrypter) Close() (err error) { - if !ar.eof { - errChunk := ar.validateFinalTag(ar.peekedBytes) - if errChunk != nil { - return errChunk - } + errChunk := ar.validateFinalTag(ar.peekedBytes) + if errChunk != nil { + return errChunk } return nil } @@ -132,20 +119,13 @@ func (ar *aeadDecrypter) Close() (err error) { // the underlying plaintext and an error. It accesses peeked bytes from next // chunk, to identify the last chunk and decrypt/validate accordingly. func (ar *aeadDecrypter) openChunk(data []byte) ([]byte, error) { - tagLen := ar.aead.Overhead() - // Restore carried bytes from last call - chunkExtra := append(ar.peekedBytes, data...) - // 'chunk' contains encrypted bytes, followed by an authentication tag. - chunk := chunkExtra[:len(chunkExtra)-tagLen] - ar.peekedBytes = chunkExtra[len(chunkExtra)-tagLen:] - adata := ar.associatedData if ar.aeadCrypter.packetTag == packetTypeAEADEncrypted { adata = append(ar.associatedData, ar.chunkIndex...) } nonce := ar.computeNextNonce() - plainChunk, err := ar.aead.Open(nil, nonce, chunk, adata) + plainChunk, err := ar.aead.Open(nil, nonce, data, adata) if err != nil { return nil, errors.ErrAEADTagVerification } diff --git a/openpgp/packet/aead_encrypted.go b/openpgp/packet/aead_encrypted.go index 89a611efb..583765d87 100644 --- a/openpgp/packet/aead_encrypted.go +++ b/openpgp/packet/aead_encrypted.go @@ -65,13 +65,15 @@ func (ae *AEADEncrypted) decrypt(key []byte) (io.ReadCloser, error) { blockCipher := ae.cipher.new(key) aead := ae.mode.new(blockCipher) // Carry the first tagLen bytes + chunkSize := decodeAEADChunkSize(ae.chunkSizeByte) tagLen := ae.mode.TagLength() - peekedBytes := make([]byte, tagLen) + chunkBytes := make([]byte, chunkSize+tagLen*2) + peekedBytes := chunkBytes[chunkSize+tagLen:] n, err := io.ReadFull(ae.Contents, peekedBytes) if n < tagLen || (err != nil && err != io.EOF) { return nil, errors.AEADError("Not enough data to decrypt:" + err.Error()) } - chunkSize := decodeAEADChunkSize(ae.chunkSizeByte) + return &aeadDecrypter{ aeadCrypter: aeadCrypter{ aead: aead, @@ -82,7 +84,9 @@ func (ae *AEADEncrypted) decrypt(key []byte) (io.ReadCloser, error) { packetTag: packetTypeAEADEncrypted, }, reader: ae.Contents, - peekedBytes: peekedBytes}, nil + chunkBytes: chunkBytes, + peekedBytes: peekedBytes, + }, nil } // associatedData for chunks: tag, version, cipher, mode, chunk size byte diff --git a/openpgp/packet/aead_encrypted_test.go b/openpgp/packet/aead_encrypted_test.go index 38dc9168c..d3e569b0f 100644 --- a/openpgp/packet/aead_encrypted_test.go +++ b/openpgp/packet/aead_encrypted_test.go @@ -407,6 +407,7 @@ func readDecryptedStream(rc io.ReadCloser) (got []byte, err error) { } } } + err = rc.Close() return got, err } diff --git a/openpgp/packet/symmetrically_encrypted_aead.go b/openpgp/packet/symmetrically_encrypted_aead.go index 377273dd7..99ec6fc57 100644 --- a/openpgp/packet/symmetrically_encrypted_aead.go +++ b/openpgp/packet/symmetrically_encrypted_aead.go @@ -70,8 +70,10 @@ func (se *SymmetricallyEncrypted) decryptAead(inputKey []byte) (io.ReadCloser, e aead, nonce := getSymmetricallyEncryptedAeadInstance(se.Cipher, se.Mode, inputKey, se.Salt[:], se.associatedData()) // Carry the first tagLen bytes + chunkSize := decodeAEADChunkSize(se.ChunkSizeByte) tagLen := se.Mode.TagLength() - peekedBytes := make([]byte, tagLen) + chunkBytes := make([]byte, chunkSize+tagLen*2) + peekedBytes := chunkBytes[chunkSize+tagLen:] n, err := io.ReadFull(se.Contents, peekedBytes) if n < tagLen || (err != nil && err != io.EOF) { return nil, errors.StructuralError("not enough data to decrypt:" + err.Error()) @@ -87,6 +89,7 @@ func (se *SymmetricallyEncrypted) decryptAead(inputKey []byte) (io.ReadCloser, e packetTag: packetTypeSymmetricallyEncryptedIntegrityProtected, }, reader: se.Contents, + chunkBytes: chunkBytes, peekedBytes: peekedBytes, }, nil } From fee782456821f3ab5f1de998d3e959c7e2cbae98 Mon Sep 17 00:00:00 2001 From: Daniel Huigens Date: Mon, 9 Dec 2024 22:53:52 +0100 Subject: [PATCH 3/6] Reuse ciphertext slice for plaintext when decrypting --- internal/byteutil/byteutil.go | 8 ++++---- ocb/ocb.go | 28 ++++++++++++---------------- openpgp/packet/aead_crypter.go | 4 ++-- 3 files changed, 18 insertions(+), 22 deletions(-) diff --git a/internal/byteutil/byteutil.go b/internal/byteutil/byteutil.go index affb74a76..d558b9bd8 100644 --- a/internal/byteutil/byteutil.go +++ b/internal/byteutil/byteutil.go @@ -49,16 +49,16 @@ func ShiftNBytesLeft(dst, x []byte, n int) { dst = append(dst, make([]byte, n/8)...) } -// XorBytesMut assumes equal input length, replaces X with X XOR Y +// XorBytesMut replaces X with X XOR Y. len(X) must be >= len(Y). func XorBytesMut(X, Y []byte) { - for i := 0; i < len(X); i++ { + for i := 0; i < len(Y); i++ { X[i] ^= Y[i] } } -// XorBytes assumes equal input length, puts X XOR Y into Z +// XorBytes puts X XOR Y into Z. len(Z) and len(X) must be >= len(Y). func XorBytes(Z, X, Y []byte) { - for i := 0; i < len(X); i++ { + for i := 0; i < len(Y); i++ { Z[i] = X[i] ^ Y[i] } } diff --git a/ocb/ocb.go b/ocb/ocb.go index 5022285b4..1ed58f7c9 100644 --- a/ocb/ocb.go +++ b/ocb/ocb.go @@ -110,7 +110,8 @@ func (o *ocb) Seal(dst, nonce, plaintext, adata []byte) []byte { panic("crypto/ocb: Incorrect nonce length given to OCB") } ret, out := byteutil.SliceForAppend(dst, len(plaintext)+o.tagSize) - o.crypt(enc, out, nonce, adata, plaintext) + tag := o.crypt(enc, out, nonce, adata, plaintext) + copy(out[len(plaintext):], tag) return ret } @@ -122,12 +123,10 @@ func (o *ocb) Open(dst, nonce, ciphertext, adata []byte) ([]byte, error) { return nil, ocbError("Ciphertext shorter than tag length") } sep := len(ciphertext) - o.tagSize - ret, out := byteutil.SliceForAppend(dst, len(ciphertext)) + ret, out := byteutil.SliceForAppend(dst, sep) ciphertextData := ciphertext[:sep] - tag := ciphertext[sep:] - o.crypt(dec, out, nonce, adata, ciphertextData) - if subtle.ConstantTimeCompare(ret[sep:], tag) == 1 { - ret = ret[:sep] + tag := o.crypt(dec, out, nonce, adata, ciphertextData) + if subtle.ConstantTimeCompare(tag, ciphertext[sep:]) == 1 { return ret, nil } for i := range out { @@ -137,7 +136,8 @@ func (o *ocb) Open(dst, nonce, ciphertext, adata []byte) ([]byte, error) { } // On instruction enc (resp. dec), crypt is the encrypt (resp. decrypt) -// function. It returns the resulting plain/ciphertext with the tag appended. +// function. It writes the resulting plain/ciphertext into Y and returns +// the tag. func (o *ocb) crypt(instruction int, Y, nonce, adata, X []byte) []byte { // // Consider X as a sequence of 128-bit blocks @@ -220,27 +220,23 @@ func (o *ocb) crypt(instruction int, Y, nonce, adata, X []byte) []byte { // P_* || bit(1) || zeroes(127) - len(P_*) switch instruction { case enc: - paddedY := append(chunkX, byte(128)) - paddedY = append(paddedY, make([]byte, blockSize-len(chunkX)-1)...) - byteutil.XorBytesMut(checksum, paddedY) + byteutil.XorBytesMut(checksum, chunkX) + checksum[len(chunkX)] ^= 128 case dec: - paddedX := append(chunkY, byte(128)) - paddedX = append(paddedX, make([]byte, blockSize-len(chunkY)-1)...) - byteutil.XorBytesMut(checksum, paddedX) + byteutil.XorBytesMut(checksum, chunkY) + checksum[len(chunkY)] ^= 128 } byteutil.XorBytes(tag, checksum, offset) byteutil.XorBytesMut(tag, o.mask.lDol) o.block.Encrypt(tag, tag) byteutil.XorBytesMut(tag, o.hash(adata)) - copy(Y[blockSize*m+len(chunkY):], tag[:o.tagSize]) } else { byteutil.XorBytes(tag, checksum, offset) byteutil.XorBytesMut(tag, o.mask.lDol) o.block.Encrypt(tag, tag) byteutil.XorBytesMut(tag, o.hash(adata)) - copy(Y[blockSize*m:], tag[:o.tagSize]) } - return Y + return tag[:o.tagSize] } // This hash function is used to compute the tag. Per design, on empty input it diff --git a/openpgp/packet/aead_crypter.go b/openpgp/packet/aead_crypter.go index a85a9789e..a05b7ad27 100644 --- a/openpgp/packet/aead_crypter.go +++ b/openpgp/packet/aead_crypter.go @@ -125,7 +125,7 @@ func (ar *aeadDecrypter) openChunk(data []byte) ([]byte, error) { } nonce := ar.computeNextNonce() - plainChunk, err := ar.aead.Open(nil, nonce, data, adata) + plainChunk, err := ar.aead.Open(data[:0:len(data)], nonce, data, adata) if err != nil { return nil, errors.ErrAEADTagVerification } @@ -243,7 +243,7 @@ func (aw *aeadEncrypter) sealChunk(data []byte) ([]byte, error) { } nonce := aw.computeNextNonce() - encrypted := aw.aead.Seal(nil, nonce, data, adata) + encrypted := aw.aead.Seal(data[:0:len(data)], nonce, data, adata) aw.bytesProcessed += len(data) if err := aw.aeadCrypter.incrementIndex(); err != nil { return nil, err From 04cfaf2d9d69903d32db0b4012ad102ebff4a511 Mon Sep 17 00:00:00 2001 From: Daniel Huigens Date: Tue, 10 Dec 2024 15:34:20 +0100 Subject: [PATCH 4/6] Reuse plaintext slice for ciphertext when encrypting --- ocb/ocb.go | 31 ++++++------ openpgp/packet/aead_crypter.go | 47 ++++++++++--------- openpgp/packet/aead_encrypted_test.go | 5 +- .../packet/symmetrically_encrypted_aead.go | 8 +++- 4 files changed, 49 insertions(+), 42 deletions(-) diff --git a/ocb/ocb.go b/ocb/ocb.go index 1ed58f7c9..24f893017 100644 --- a/ocb/ocb.go +++ b/ocb/ocb.go @@ -109,9 +109,10 @@ func (o *ocb) Seal(dst, nonce, plaintext, adata []byte) []byte { if len(nonce) > o.nonceSize { panic("crypto/ocb: Incorrect nonce length given to OCB") } - ret, out := byteutil.SliceForAppend(dst, len(plaintext)+o.tagSize) - tag := o.crypt(enc, out, nonce, adata, plaintext) - copy(out[len(plaintext):], tag) + sep := len(plaintext) + ret, out := byteutil.SliceForAppend(dst, sep+o.tagSize) + tag := o.crypt(enc, out[:sep], nonce, adata, plaintext) + copy(out[sep:], tag) return ret } @@ -194,13 +195,14 @@ func (o *ocb) crypt(instruction int, Y, nonce, adata, X []byte) []byte { byteutil.XorBytesMut(offset, o.mask.L[bits.TrailingZeros(uint(i+1))]) blockX := X[i*blockSize : (i+1)*blockSize] blockY := Y[i*blockSize : (i+1)*blockSize] - byteutil.XorBytes(blockY, blockX, offset) switch instruction { case enc: + byteutil.XorBytesMut(checksum, blockX) + byteutil.XorBytes(blockY, blockX, offset) o.block.Encrypt(blockY, blockY) byteutil.XorBytesMut(blockY, offset) - byteutil.XorBytesMut(checksum, blockX) case dec: + byteutil.XorBytes(blockY, blockX, offset) o.block.Decrypt(blockY, blockY) byteutil.XorBytesMut(blockY, offset) byteutil.XorBytesMut(checksum, blockY) @@ -216,26 +218,23 @@ func (o *ocb) crypt(instruction int, Y, nonce, adata, X []byte) []byte { o.block.Encrypt(pad, offset) chunkX := X[blockSize*m:] chunkY := Y[blockSize*m : len(X)] - byteutil.XorBytes(chunkY, chunkX, pad[:len(chunkX)]) - // P_* || bit(1) || zeroes(127) - len(P_*) switch instruction { case enc: byteutil.XorBytesMut(checksum, chunkX) checksum[len(chunkX)] ^= 128 + byteutil.XorBytes(chunkY, chunkX, pad[:len(chunkX)]) + // P_* || bit(1) || zeroes(127) - len(P_*) case dec: + byteutil.XorBytes(chunkY, chunkX, pad[:len(chunkX)]) + // P_* || bit(1) || zeroes(127) - len(P_*) byteutil.XorBytesMut(checksum, chunkY) checksum[len(chunkY)] ^= 128 } - byteutil.XorBytes(tag, checksum, offset) - byteutil.XorBytesMut(tag, o.mask.lDol) - o.block.Encrypt(tag, tag) - byteutil.XorBytesMut(tag, o.hash(adata)) - } else { - byteutil.XorBytes(tag, checksum, offset) - byteutil.XorBytesMut(tag, o.mask.lDol) - o.block.Encrypt(tag, tag) - byteutil.XorBytesMut(tag, o.hash(adata)) } + byteutil.XorBytes(tag, checksum, offset) + byteutil.XorBytesMut(tag, o.mask.lDol) + o.block.Encrypt(tag, tag) + byteutil.XorBytesMut(tag, o.hash(adata)) return tag[:o.tagSize] } diff --git a/openpgp/packet/aead_crypter.go b/openpgp/packet/aead_crypter.go index a05b7ad27..54fbca74d 100644 --- a/openpgp/packet/aead_crypter.go +++ b/openpgp/packet/aead_crypter.go @@ -20,7 +20,6 @@ type aeadCrypter struct { chunkIndex []byte // Chunk counter packetTag packetType // SEIP packet (v2) or AEAD Encrypted Data packet bytesProcessed int // Amount of plaintext bytes encrypted/decrypted - buffer bytes.Buffer // Buffered bytes across chunks } // computeNonce takes the incremental index and computes an eXclusive OR with @@ -60,10 +59,11 @@ func (wo *aeadCrypter) incrementIndex() error { // aeadDecrypter reads and decrypts bytes. It buffers extra decrypted bytes when // necessary, similar to aeadEncrypter. type aeadDecrypter struct { - aeadCrypter // Embedded ciphertext opener - reader io.Reader // 'reader' is a partialLengthReader + aeadCrypter // Embedded ciphertext opener + reader io.Reader // 'reader' is a partialLengthReader chunkBytes []byte - peekedBytes []byte // Used to detect last chunk + peekedBytes []byte // Used to detect last chunk + buffer bytes.Buffer // Buffered decrypted bytes } // Read decrypts bytes and reads them into dst. It decrypts when necessary and @@ -163,27 +163,29 @@ func (ar *aeadDecrypter) validateFinalTag(tag []byte) error { type aeadEncrypter struct { aeadCrypter // Embedded plaintext sealer writer io.WriteCloser // 'writer' is a partialLengthWriter + chunkBytes []byte + offset int } // Write encrypts and writes bytes. It encrypts when necessary and buffers extra // plaintext bytes for next call. When the stream is finished, Close() MUST be // called to append the final tag. func (aw *aeadEncrypter) Write(plaintextBytes []byte) (n int, err error) { - // Append plaintextBytes to existing buffered bytes - n, err = aw.buffer.Write(plaintextBytes) - if err != nil { - return n, err - } - // Encrypt and write chunks - for aw.buffer.Len() >= aw.chunkSize { - plainChunk := aw.buffer.Next(aw.chunkSize) - encryptedChunk, err := aw.sealChunk(plainChunk) - if err != nil { - return n, err - } - _, err = aw.writer.Write(encryptedChunk) - if err != nil { - return n, err + for n != len(plaintextBytes) { + copied := copy(aw.chunkBytes[aw.offset:aw.chunkSize], plaintextBytes[n:]) + n += copied + aw.offset += copied + + if aw.offset == aw.chunkSize { + encryptedChunk, err := aw.sealChunk(aw.chunkBytes[:aw.offset]) + if err != nil { + return n, err + } + _, err = aw.writer.Write(encryptedChunk) + if err != nil { + return n, err + } + aw.offset = 0 } } return @@ -195,9 +197,8 @@ func (aw *aeadEncrypter) Write(plaintextBytes []byte) (n int, err error) { func (aw *aeadEncrypter) Close() (err error) { // Encrypt and write a chunk if there's buffered data left, or if we haven't // written any chunks yet. - if aw.buffer.Len() > 0 || aw.bytesProcessed == 0 { - plainChunk := aw.buffer.Bytes() - lastEncryptedChunk, err := aw.sealChunk(plainChunk) + if aw.offset > 0 || aw.bytesProcessed == 0 { + lastEncryptedChunk, err := aw.sealChunk(aw.chunkBytes[:aw.offset]) if err != nil { return err } @@ -243,7 +244,7 @@ func (aw *aeadEncrypter) sealChunk(data []byte) ([]byte, error) { } nonce := aw.computeNextNonce() - encrypted := aw.aead.Seal(data[:0:len(data)], nonce, data, adata) + encrypted := aw.aead.Seal(data[:0], nonce, data, adata) aw.bytesProcessed += len(data) if err := aw.aeadCrypter.incrementIndex(); err != nil { return nil, err diff --git a/openpgp/packet/aead_encrypted_test.go b/openpgp/packet/aead_encrypted_test.go index d3e569b0f..e2b328a08 100644 --- a/openpgp/packet/aead_encrypted_test.go +++ b/openpgp/packet/aead_encrypted_test.go @@ -449,6 +449,8 @@ func SerializeAEADEncrypted(w io.Writer, key []byte, config *Config) (io.WriteCl alg := aeadConf.Mode().new(blockCipher) chunkSize := decodeAEADChunkSize(aeadConf.ChunkSizeByte()) + tagLen := alg.Overhead() + chunkBytes := make([]byte, chunkSize+tagLen) return &aeadEncrypter{ aeadCrypter: aeadCrypter{ aead: alg, @@ -458,6 +460,7 @@ func SerializeAEADEncrypted(w io.Writer, key []byte, config *Config) (io.WriteCl nonce: nonce, packetTag: packetTypeAEADEncrypted, }, - writer: writer, + writer: writer, + chunkBytes: chunkBytes, }, nil } diff --git a/openpgp/packet/symmetrically_encrypted_aead.go b/openpgp/packet/symmetrically_encrypted_aead.go index 99ec6fc57..3ddc4fe4a 100644 --- a/openpgp/packet/symmetrically_encrypted_aead.go +++ b/openpgp/packet/symmetrically_encrypted_aead.go @@ -133,16 +133,20 @@ func serializeSymmetricallyEncryptedAead(ciphertext io.WriteCloser, cipherSuite aead, nonce := getSymmetricallyEncryptedAeadInstance(cipherSuite.Cipher, cipherSuite.Mode, inputKey, salt, prefix) + chunkSize := decodeAEADChunkSize(chunkSizeByte) + tagLen := aead.Overhead() + chunkBytes := make([]byte, chunkSize+tagLen) return &aeadEncrypter{ aeadCrypter: aeadCrypter{ aead: aead, - chunkSize: decodeAEADChunkSize(chunkSizeByte), + chunkSize: chunkSize, associatedData: prefix, nonce: nonce, chunkIndex: nonce[len(nonce)-8:], packetTag: packetTypeSymmetricallyEncryptedIntegrityProtected, }, - writer: ciphertext, + writer: ciphertext, + chunkBytes: chunkBytes, }, nil } From df3ee02d77623299d88df3d8a29486b548300d72 Mon Sep 17 00:00:00 2001 From: Daniel Huigens Date: Tue, 10 Dec 2024 15:42:57 +0100 Subject: [PATCH 5/6] Buffer decrypted bytes more efficiently --- openpgp/packet/aead_crypter.go | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/openpgp/packet/aead_crypter.go b/openpgp/packet/aead_crypter.go index 54fbca74d..5e4604656 100644 --- a/openpgp/packet/aead_crypter.go +++ b/openpgp/packet/aead_crypter.go @@ -3,7 +3,6 @@ package packet import ( - "bytes" "crypto/cipher" "encoding/binary" "io" @@ -59,11 +58,11 @@ func (wo *aeadCrypter) incrementIndex() error { // aeadDecrypter reads and decrypts bytes. It buffers extra decrypted bytes when // necessary, similar to aeadEncrypter. type aeadDecrypter struct { - aeadCrypter // Embedded ciphertext opener - reader io.Reader // 'reader' is a partialLengthReader + aeadCrypter // Embedded ciphertext opener + reader io.Reader // 'reader' is a partialLengthReader chunkBytes []byte - peekedBytes []byte // Used to detect last chunk - buffer bytes.Buffer // Buffered decrypted bytes + peekedBytes []byte // Used to detect last chunk + buffer []byte // Buffered decrypted bytes } // Read decrypts bytes and reads them into dst. It decrypts when necessary and @@ -71,8 +70,10 @@ type aeadDecrypter struct { // and an error. func (ar *aeadDecrypter) Read(dst []byte) (n int, err error) { // Return buffered plaintext bytes from previous calls - if ar.buffer.Len() > 0 { - return ar.buffer.Read(dst) + if len(ar.buffer) > 0 { + n = copy(dst, ar.buffer) + ar.buffer = ar.buffer[n:] + return } // Read a chunk @@ -92,12 +93,8 @@ func (ar *aeadDecrypter) Read(dst []byte) (n int, err error) { } // Return decrypted bytes, buffering if necessary - if len(dst) < len(decrypted) { - n = copy(dst, decrypted[:len(dst)]) - ar.buffer.Write(decrypted[len(dst):]) - } else { - n = copy(dst, decrypted) - } + n = copy(dst, decrypted) + ar.buffer = decrypted[n:] return } From 1fd5ec842fd37133206b23cba68eda6ae4b905e3 Mon Sep 17 00:00:00 2001 From: Daniel Huigens Date: Mon, 16 Dec 2024 12:33:24 +0100 Subject: [PATCH 6/6] Add tests for reusing buffer in OCB en/decryption --- ocb/ocb_test.go | 114 +++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 108 insertions(+), 6 deletions(-) diff --git a/ocb/ocb_test.go b/ocb/ocb_test.go index d3fc4cf57..974a0ac97 100644 --- a/ocb/ocb_test.go +++ b/ocb/ocb_test.go @@ -127,8 +127,20 @@ func TestEncryptDecryptRFC7253TestVectors(t *testing.T) { adata, _ := hex.DecodeString(test.header) targetPt, _ := hex.DecodeString(test.plaintext) targetCt, _ := hex.DecodeString(test.ciphertext) - ct := ocbInstance.Seal(nil, nonce, targetPt, adata) // Encrypt + ct := ocbInstance.Seal(nil, nonce, targetPt, adata) + if !bytes.Equal(ct, targetCt) { + t.Errorf( + `RFC7253 Test vectors Encrypt error (ciphertexts don't match): + Got: + %X + Want: + %X`, ct, targetCt) + } + // Encrypt reusing buffer + pt := make([]byte, len(targetPt) + ocbInstance.Overhead()) + copy(pt, targetPt) + ct = ocbInstance.Seal(pt[:0], nonce, pt[:len(targetPt)], adata) if !bytes.Equal(ct, targetCt) { t.Errorf( `RFC7253 Test vectors Encrypt error (ciphertexts don't match): @@ -138,14 +150,14 @@ func TestEncryptDecryptRFC7253TestVectors(t *testing.T) { %X`, ct, targetCt) } // Decrypt - pt, err := ocbInstance.Open(nil, nonce, targetCt, adata) + pt, err := ocbInstance.Open(nil, nonce, ct, adata) if err != nil { t.Errorf( `RFC7253 Valid ciphertext was refused decryption: plaintext %X nonce %X header %X - ciphertext %X`, targetPt, nonce, adata, targetCt) + ciphertext %X`, targetPt, nonce, adata, ct) } if !bytes.Equal(pt, targetPt) { t.Errorf( @@ -155,6 +167,24 @@ func TestEncryptDecryptRFC7253TestVectors(t *testing.T) { Want: %X`, pt, targetPt) } + // Decrypt reusing buffer + pt, err = ocbInstance.Open(ct[:0], nonce, ct, adata) + if err != nil { + t.Errorf( + `RFC7253 Valid ciphertext was refused decryption: + plaintext %X + nonce %X + header %X + ciphertext %X`, targetPt, nonce, adata, ct) + } + if !bytes.Equal(pt, targetPt) { + t.Errorf( + `RFC7253 test vectors Decrypt error (plaintexts don't match): + Got: + %X + Want: + %X`, targetPt, pt) + } } } @@ -182,7 +212,30 @@ func TestEncryptDecryptRFC7253TagLen96(t *testing.T) { Want: %X`, ct, targetCt) } - pt, err := ocbInstance.Open(nil, nonce, targetCt, adata) + pt := make([]byte, len(targetPt) + ocbInstance.Overhead()) + copy(pt, targetPt) + ct = ocbInstance.Seal(pt[:0], nonce, pt[:len(targetPt)], adata) + if !bytes.Equal(ct, targetCt) { + t.Errorf( + `RFC7253 test tagLen96 error (ciphertexts don't match): + Got: + %X + Want: + %X`, ct, targetCt) + } + pt, err = ocbInstance.Open(nil, nonce, ct, adata) + if err != nil { + t.Errorf(`RFC7253 test tagLen96 was refused decryption`) + } + if !bytes.Equal(pt, targetPt) { + t.Errorf( + `RFC7253 test tagLen96 error (plaintexts don't match): + Got: + %X + Want: + %X`, pt, targetPt) + } + pt, err = ocbInstance.Open(ct[:0], nonce, ct, adata) if err != nil { t.Errorf(`RFC7253 test tagLen96 was refused decryption`) } @@ -274,15 +327,47 @@ func TestEncryptDecryptGoTestVectors(t *testing.T) { %X`, ct, targetCt) } + // Encrypt reusing buffer + pt := make([]byte, len(targetPt) + ocbInstance.Overhead()) + copy(pt, targetPt) + ct = ocbInstance.Seal(pt[:0], nonce, pt[:len(targetPt)], adata) + if !bytes.Equal(ct, targetCt) { + t.Errorf( + `Go Test vectors Encrypt error (ciphertexts don't match): + Got: + %X + Want: + %X`, ct, targetCt) + } + // Decrypt - pt, err := ocbInstance.Open(nil, nonce, targetCt, adata) + pt, err = ocbInstance.Open(nil, nonce, ct, adata) if err != nil { t.Errorf( `Valid Go ciphertext was refused decryption: plaintext %X nonce %X header %X - ciphertext %X`, targetPt, nonce, adata, targetCt) + ciphertext %X`, targetPt, nonce, adata, ct) + } + if !bytes.Equal(pt, targetPt) { + t.Errorf( + `Go Test vectors Decrypt error (plaintexts don't match): + Got: + %X + Want: + %X`, pt, targetPt) + } + + // Decrypt reusing buffer + pt, err = ocbInstance.Open(ct[:0], nonce, ct, adata) + if err != nil { + t.Errorf( + `Valid Go ciphertext was refused decryption: + plaintext %X + nonce %X + header %X + ciphertext %X`, targetPt, nonce, adata, ct) } if !bytes.Equal(pt, targetPt) { t.Errorf( @@ -333,6 +418,17 @@ func TestEncryptDecryptVectorsWithPreviousDataRandomizeSlow(t *testing.T) { `Random Encrypt/Decrypt error (plaintexts don't match)`) break } + decrypted, err = ocb.Open(ct[:0], nonce, ct, header) + if err != nil { + t.Errorf( + `Decrypt refused valid tag (not displaying long output)`) + break + } + if !bytes.Equal(pt, decrypted) { + t.Errorf( + `Random Encrypt/Decrypt error (plaintexts don't match)`) + break + } } } @@ -369,6 +465,12 @@ func TestRejectTamperedCiphertextRandomizeSlow(t *testing.T) { "Tampered ciphertext was not refused decryption (OCB did not return an error)") return } + _, err = ocb.Open(tampered[:0], nonce, tampered, header) + if err == nil { + t.Errorf( + "Tampered ciphertext was not refused decryption (OCB did not return an error)") + return + } } func TestParameters(t *testing.T) {