Twitter
72
February 14, 2014
PEFS Security Audit
This is the result of a short 13-hour security audit of Private Encrypted File System (PEFS).
Thanks to Matt Olander for funding this audit.
-----------------------------------------------------------------------
PEFS Security Audit
Taylor Hornby
February 07, 2014
-----------------------------------------------------------------------
1. Introduction
This report documents the results of a 13-hour security audit on
Private Encrypted File System (PEFS). The audit uncovered several
minor problems, some of which may compromise confidentiality.
1.1. What is PEFS?
PEFS [1] is an encrypted filesystem similar to EncFS and eCryptFS.
PEFS is unlike block-level encryption (e.g. TrueCrypt) in that the
ciphertext mirrors the directory structure of the plaintext.
1.2. Audit Scope
The audit was performed by reading the PEFS source code [2]. The git
commit hash of the code that was audited is:
fb7c4a188cfa7ba69987a1a61104c56f63db5395
This audit focused primarily on PEFS's cryptographic design and
implementation. This includes PEFS's use of crypto primitives, but
not the implementations of the primitives themselves (i.e. the AES
implementation). Some integer overflow issues were discovered, but
looking for memory corruption vulnerabilities was not a priority of
the audit. Bugs in the file system implementation (pefs_vnops.c) were
also out of scope.
2. Issues
This section covers all of the security issues discovered during the
audit. They are each given a rating in three variables:
Exploitability: How hard is it for an attacker to exploit?
Security Impact: How bad is it if an attacker can exploit it?
Confirmation: Has the vulnerability been confirmed to exist?
2.1. HMAC and Other Secrets Not Compared in Constant Time
Exploitability: Low
Security Impact: Low
Confirmation: Confirmed by inspecting the code.
On line 357 of pefs_key.c, memcmp() is used to verify an HMAC. This
introduces a timing attack that an attacker might be able to use to
compute the HMAC of an arbitrary message. This is the HMAC used when
encrypting child keys for the key chains feature, not the VMAC (which
is compared in constant time).
There are several other non-constant-time comparisons. I am not sure
if they are relevant to security:
- The comparison of ptk_tweak in pefs_tkey_cmp() in pefs_vnops.c.
- Comparison of what might be cleartext filenames in
pefs_enccn_parsedir() in pefs_vnops.c.
- The comparisons of pk_keyid in pefs_crypto.c, pefs_ctl.c, and
pefs_keychain.c.
In general, when secrets are compared for equality, it should be done
in constant time so that the running time does not leak information
about their actual values.
2.2. Same Key Used for HMAC and Encryption
Exploitability: Very Low
Security Impact: Low
Confirmation: Confirmed by inspecting the code.
This issue affects the key chain system, which is an optional PEFS
feature. If the key chains feature is not used, the file encryption
is unaffected.
The same key is used for encryption and authentication in pefs_key.c.
This happens in the pefs_key_cipher() procedure. The variable 'key'
is created by HMACing a distinguisher string with another key called
pxk_key. The resulting 'key' is used twice: Once to encrypt the data,
and a second time to HMAC the ciphertext.
This does not create immediate problems, but using the same key for
multiple purposes is a bad practice. Instead, use HKDF to generate
two separate keys for the two different purposes.
2.3. Zero IV Used with CTR Mode in pefs_key.c
Exploitability: Medium (Depends on the use case.)
Security Impact: High
Confirmation: Confirmed by inspecting the code.
This issue affects the key chain system, which is an optional PEFS
feature. If the key chains feature is not used, the file encryption
is unaffected.
The pefs_key_cipher() procedure in pefs_key.c encrypts data using AES
in CTR mode with a zero IV. This means that if multiple messages are
encrypted with the same key, they are XORed with the same keystream.
Here's an example of how this can be exploited to reveal the context
of another user's files:
The key chain graph looks like:
A -> Z
A -> S
B -> S
The 'Z' key is encrypted with a parent key that only Alice knows.
The 'S' key is encrypted with Alice's parent key as well. Bob also
has access to 'S'.
In this scenario, Bob knows S, so he can XOR the known value of
S with Alice's encryption of S to get the key stream generated by
Alice's parent key. Bob can then XOR this with Alice's Z ciphertext
to get Z. If Bob did not know S, he could learn Z XOR S by XORing the
two ciphertexts together.
This is a problem whenever a parent key encrypts two or more children
keys. Instead of using a NULL IV, a random IV should be generated at
encryption time and stored in the ciphertext (remember to HMAC the IV
too!).
2.4. No Salt Used When Hashing Password
Exploitability: Medium
Security Impact: Medium
Confirmation: Confirmed by inspecting the code.
When deriving a key from a password, PEFS does not use salt. This is
necessary, otherwise an attacker can use pre-computation attacks
(lookup tables, rainbow tables, etc.) to significantly speed up the
process of cracking PEFS-encrypted directories with weak passwords.
Not using a salt was an explicit design decision to avoid having to
store metadata. I recommend adding an (optional?) mode with metadata
to support having a salt, so that if it fits the user's use case,
they can benefit from the additional security.
2.5. Ambiguous Filename Encryption Padding
Exploitability: Very Low
Security Impact: Very Low
Confirmation: Confirmed by inspecting the source code.
The filename encryption routine pads the plaintext with null bytes.
This is fine, since file names can't contain null bytes in UNIX, but
if this code is reused for something else, it could become a problem.
Pad the plaintext unambiguously by using a padding mode like PKCS#7.
2.6. Filename Encryption is Not Randomized When Files are Renamed
Exploitability: Medium (Depends on how the user renames files.)
Security Impact: Low
Confirmation: Confirmed by inspecting the source code.
PEFS uses a random 64-bit value, called the tweak, to differentiate
between encrypted files. This value is actually re-used for three
things. It's used as part of the XTS mode tweak, to "randomize" the
filename encryption, and as the VMAC IV (after being encrypted).
For the filename encryption, the tweak value is used as a substitute
for an IV. Filenames are encrypted by first prefixing the tweak, then
encrypting them in CBC mode with a zero IV.
This is a problem, because the tweak does not change when files are
renamed. With CBC mode, the IV has to be chosen randomly at the time
of encryption, or it isn't IND-CPA secure. If the filename is
changed, the new ciphertext will be identical to the old ciphertext
up to the block that contains the first difference. See the third
part of this image for an awesome visual demonstration:
https://pbs.twimg.com/media/BekWHdVCYAAgsSm.jpg:large
There are plans to solve this problem by encrypting filenames with
wide-block encryption like EME2. This should be an adequate solution,
but it needs more analysis.
There may be other problems with the tweak design as well. More
analysis is recommended in Section 5.3
2.7. Part of Files Encrypted with CTR Mode
Exploitability: High
Security Impact: Medium-High
Confirmation: Confirmed by inspecting the code.
If a file's size in bytes is not evenly divisible by 16, the last
block of bytes (up to 15 bytes) is encrypted with CTR mode. If the
last block is ever changed, and an attacker has the ciphertexts from
before and after the change, they can XOR both ciphertexts together
to learn the XOR of the old and new plaintexts. If the attacker knows
(or can guess) the old plaintext, they can get the new plaintext.
The relevant code is in xts_smallblock() in pefs_xts.c.
The only good remediation for this (that I can think of) involves
increasing the file size so that it is divisible by 16 bytes. I think
the severity vulnerability justifies the increase in complexity.
2.8. gf_mul128() Does Not Execute In Constant Time
Exploitability: Low
Security Impact: Low
Confirmation: Confirmed by inspecting the source code.
The gf_mul128() procedure, part of the XTS implementation in
pefs_xts.c, branches on a value related to its input, which could
enable side channel attacks:
static __inline void
gf_mul128(uint64_t *dst, const uint64_t *src)
{
static const uint8_t gf_128_fdbk = 0x87;
int carry;
carry = shl128(dst, src);
if (carry != 0) // <-- time difference could leak 'carry'
((uint8_t *)dst)[0] ^= gf_128_fdbk;
}
I don't know if this leaks enough information to break the encryption
(very probably not), but it's better to be safe and do it in constant
time anyway. A constant time conditional XOR is given in [3].
2.9. Memory Corruption / Integer Overflows
Exploitability: Unknown
Security Impact: High (If exploitable)
Confirmation: Not confirmed.
I noticed several bits of code with integer overflow and
signed/unsigned problems. I didn't have time to check if any of these
are exploitable, or are even values controlled by the attacker, but
I will list them here to err on the side of caution:
- In pefs_name_encrypt(), the "+ 1" in the file name size check
could overflow the integer and make the check pass even if
the filename is too long:
r = PEFS_NAME_NTOP_SIZE(pefs_name_padsize(size)) + 1;
if (r > MAXNAMLEN) {
return (-ENAMETOOLONG);
}
- In pefs_enccn_set(), the variable encname_len is a *signed*
integer. It is checked to be less than MAXPATHLEN then passed to
memcpy(). When it gets passed to memcpy(), it is cast to
a size_t, which is unsigned. If encname_len has a negative value,
it will pass the first check, then memcpy will interpret it as
a large unsigned value:
if (encname_len >= MAXPATHLEN)
panic("pefs_enccn_set: invalid encrypted name length: %d",
encname_len);
// ...
memcpy(pec->pec_buf, encname, encname_len);
((char *) pec->pec_buf)[encname_len] = '\0';
pec->pec_cn.cn_namelen = encname_len;
- Several possibilities for integer overflows in pefs_xbase64.c:
if (datalength + 1 > targsize)
return (-1);
// ...
target[tarindex] |= (pos - Base64) >> 4;
if ((size_t)tarindex + 1 < targsize)
target[tarindex+1] =
((pos - Base64) & 0x0f) << 4 ;
// ...
target[tarindex] |= (pos - Base64) >> 2;
if ((size_t)tarindex + 1 < targsize)
target[tarindex+1] =
((pos - Base64) & 0x03) << 6;
- Another in pefs_write_int() that could cause the
vnode_pager_setsize() call to be skipped, which might lead to
memory corruption down the road:
nsize = fsize;
MPASS(uio->uio_offset <= fsize);
if (uio->uio_offset + uio->uio_resid > nsize) {
PEFSDEBUG("pefs_write: extend: 0x%jx (old size: 0x%jx)\n",
uio->uio_offset + uio->uio_resid, nsize);
nsize = uio->uio_offset + uio->uio_resid;
vnode_pager_setsize(vp, nsize);
}
3. Commendations
PEFS does a lot of things right. It uses standard constructions like
XTS mode, PBKDF2, and HKDF. It also diligently zeroes buffers that
once contained sensitive information. This makes auditing easier.
Correction 12/05/2014: XTS mode is probably not the ideal option, see
Thomas Ptacek's blog post for good reasons why:
http://sockpuppet.org/blog/2014/04/30/you-dont-want-xts/
4. Recommendations
There are some things that PEFS could do better:
- Instead of re-implementing CBC mode in pefs_name_enccbc(), it
would be better to use a well-tested implementation from a crypto
library.
- Use test vectors in unit tests and runtime tests to make sure the
crypto algorithms (XTS, HKDF, PBKDF2, AES, SHA, etc.) are
correct.
- Make pefs_hkdf_expand() take a uint8_t instead of an int for the
'idx' parameter, so that there is a compiler warning when the
function is used incorrectly. Or check that 'idx' is between
0 and 255.
5. Future Work
5.1. Memory Corruption Vulnerabilities
This audit did not focus on "classic" memory corruptions
vulnerabilities. Because of the integer overflow issues documented in
Issue 2.9, I think PEFS could benefit from an audit that specifically
focuses on these concerns.
5.2. The Tweak's Triple Burden
The 64-bit random tweak is re-used for three different purposes:
1. It acts like an IV for the filename encryption.
2. It is part of the XTS mode tweak.
3. (After being Encrypted) The VMAC IV.
There was not enough time in the audit to determine whether any of
these three uses interact in negative ways that would, for example,
lead to at least one of filename encryption, file contents
encryption, or the VMAC being broken. More analysis is needed.
The VMAC's IV appears to be taken from its input, which probably
causes problems. This issue was not investigated in this audit since
the VMAC is only used as a non-cryptographic checksum, and should not
be relied on for security.
6. Conclusion
This audit found several issues, the most significant of which are
issues 2.3, 2.6, and 2.7. The possible existence of memory corruption
bugs (Issue 2.9) is worrisome as well, since this code runs in kernel
space.
PEFS would benefit from more auditing time.
7. References
[1] https://wiki.freebsd.org/PEFS
[2] https://github.com/glk/pefs
[3] http://www.iacr.org/archive/ches2009/57470001/57470001.pdf
