Hi Tyler
i looked at your patch more in deep and i found
some issues.
Ok, the key lock should be released before requesting
the FEK decryption from ecryptfsd. However, the kernel
and the userspace parts of eCryptfs may see different
authentication tokens because the key is not locked.
Other comments are inline.
On Tuesday, July 19, 2011 07:58:14 AM Tyler Hicks wrote:
> Fixes a regression caused by b5695d04634fa4ccca7dcbc05bb4a66522f02e0b
>
> Kernel keyring keys containing eCryptfs authentication tokens should not
> be write locked when calling out to ecryptfsd to wrap and unwrap file
> encryption keys. The eCryptfs kernel code can not hold the key's write
> lock because ecryptfsd needs to request the key after receiving such a
> request from the kernel.
>
> Without this fix, all file opens and creates will timeout and fail when
> using the eCryptfs PKI infrastructure. This is not an issue when using
> passphrase-based mount keys, which is the most widely deployed eCryptfs
> configuration.
>
> Signed-off-by: Tyler Hicks <tyhicks@xxxxxxxxxxxxxxxxxx>
> Cc: Roberto Sassu <roberto.sassu@xxxxxxxxx>
> Cc: <da_fox@xxxxxxxxxxxxxxxxx>
> ---
> fs/ecryptfs/keystore.c | 55 +++++++++++++++++++++++++++---------------------
> 1 files changed, 31 insertions(+), 24 deletions(-)
>
> diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c
> index 27a7fef..c533ef6 100644
> --- a/fs/ecryptfs/keystore.c
> +++ b/fs/ecryptfs/keystore.c
> @@ -1138,13 +1138,16 @@ ecryptfs_get_auth_tok_sig(char **sig, struct ecryptfs_auth_tok *auth_tok)
>
> /**
> * decrypt_pki_encrypted_session_key - Decrypt the session key with the given auth_tok.
> + * @auth_tok_key: The authentication token key to unlock and put when done with
> + * @auth_tok
> * @auth_tok: The key authentication token used to decrypt the session key
> * @crypt_stat: The cryptographic context
> *
> * Returns zero on success; non-zero error otherwise.
> */
> static int
> -decrypt_pki_encrypted_session_key(struct ecryptfs_auth_tok *auth_tok,
> +decrypt_pki_encrypted_session_key(struct key *auth_tok_key,
> + struct ecryptfs_auth_tok *auth_tok,
> struct ecryptfs_crypt_stat *crypt_stat)
> {
> u8 cipher_code = 0;
> @@ -1159,10 +1162,14 @@ decrypt_pki_encrypted_session_key(struct ecryptfs_auth_tok *auth_tok,
> if (rc) {
> printk(KERN_ERR "Unrecognized auth tok type: [%d]\n",
> auth_tok->token_type);
> + up_write(&(auth_tok_key->sem));
> + key_put(auth_tok_key);
> goto out;
> }
> rc = write_tag_64_packet(auth_tok_sig, &(auth_tok->session_key),
> &payload, &payload_len);
> + up_write(&(auth_tok_key->sem));
> + key_put(auth_tok_key);
The key should be locked another time after ecryptfs_wait_for_response()
because you are modifying the authentication token.
> if (rc) {
> ecryptfs_printk(KERN_ERR, "Failed to write tag 64 packet\n");
> goto out;
> @@ -1868,11 +1875,6 @@ int ecryptfs_parse_packet_set(struct ecryptfs_crypt_stat *crypt_stat,
> * just one will be sufficient to decrypt to get the FEK. */
> find_next_matching_auth_tok:
> found_auth_tok = 0;
> - if (auth_tok_key) {
> - up_write(&(auth_tok_key->sem));
> - key_put(auth_tok_key);
> - auth_tok_key = NULL;
> - }
> list_for_each_entry(auth_tok_list_item, &auth_tok_list, list) {
> candidate_auth_tok = &auth_tok_list_item->auth_tok;
> if (unlikely(ecryptfs_verbosity > 0)) {
> @@ -1902,6 +1904,8 @@ find_next_matching_auth_tok:
> ecryptfs_printk(KERN_ERR, "Could not find a usable "
> "authentication token\n");
> rc = -EIO;
> + up_write(&(auth_tok_key->sem));
> + key_put(auth_tok_key);
You must check whether the 'auth_tok_key' pointer is NULL,
otherwise a NULL pointer exception arises if a key request
fails.
> goto out_wipe_list;
> }
> found_matching_auth_tok:
> @@ -1909,7 +1913,8 @@ found_matching_auth_tok:
> memcpy(&(candidate_auth_tok->token.private_key),
> &(matching_auth_tok->token.private_key),
> sizeof(struct ecryptfs_private_key));
> - rc = decrypt_pki_encrypted_session_key(candidate_auth_tok,
> + rc = decrypt_pki_encrypted_session_key(auth_tok_key,
> + candidate_auth_tok,
> crypt_stat);
> } else if (candidate_auth_tok->token_type == ECRYPTFS_PASSWORD) {
> memcpy(&(candidate_auth_tok->token.password),
> @@ -1917,6 +1922,8 @@ found_matching_auth_tok:
> sizeof(struct ecryptfs_password));
> rc = decrypt_passphrase_encrypted_session_key(
> candidate_auth_tok, crypt_stat);
> + up_write(&(auth_tok_key->sem));
> + key_put(auth_tok_key);
> }
> if (rc) {
> struct ecryptfs_auth_tok_list_item *auth_tok_list_item_tmp;
> @@ -1956,15 +1963,12 @@ found_matching_auth_tok:
> out_wipe_list:
> wipe_auth_tok_list(&auth_tok_list);
> out:
> - if (auth_tok_key) {
> - up_write(&(auth_tok_key->sem));
> - key_put(auth_tok_key);
> - }
> return rc;
> }
>
> static int
> -pki_encrypt_session_key(struct ecryptfs_auth_tok *auth_tok,
> +pki_encrypt_session_key(struct key *auth_tok_key,
> + struct ecryptfs_auth_tok *auth_tok,
> struct ecryptfs_crypt_stat *crypt_stat,
> struct ecryptfs_key_record *key_rec)
> {
> @@ -1979,6 +1983,8 @@ pki_encrypt_session_key(struct ecryptfs_auth_tok *auth_tok,
> crypt_stat->cipher,
> crypt_stat->key_size),
> crypt_stat, &payload, &payload_len);
> + up_write(&(auth_tok_key->sem));
> + key_put(auth_tok_key);
> if (rc) {
> ecryptfs_printk(KERN_ERR, "Error generating tag 66 packet\n");
> goto out;
> @@ -2008,6 +2014,8 @@ out:
> * write_tag_1_packet - Write an RFC2440-compatible tag 1 (public key) packet
> * @dest: Buffer into which to write the packet
> * @remaining_bytes: Maximum number of bytes that can be writtn
> + * @auth_tok_key: The authentication token key to unlock and put when done with
> + * @auth_tok
> * @auth_tok: The authentication token used for generating the tag 1 packet
> * @crypt_stat: The cryptographic context
> * @key_rec: The key record struct for the tag 1 packet
> @@ -2018,7 +2026,7 @@ out:
> */
> static int
> write_tag_1_packet(char *dest, size_t *remaining_bytes,
> - struct ecryptfs_auth_tok *auth_tok,
> + struct key *auth_tok_key, struct ecryptfs_auth_tok *auth_tok,
> struct ecryptfs_crypt_stat *crypt_stat,
> struct ecryptfs_key_record *key_rec, size_t *packet_size)
> {
> @@ -2039,12 +2047,15 @@ write_tag_1_packet(char *dest, size_t *remaining_bytes,
> memcpy(key_rec->enc_key,
> auth_tok->session_key.encrypted_key,
> auth_tok->session_key.encrypted_key_size);
> + up_write(&(auth_tok_key->sem));
> + key_put(auth_tok_key);
> goto encrypted_session_key_set;
> }
> if (auth_tok->session_key.encrypted_key_size == 0)
> auth_tok->session_key.encrypted_key_size =
> auth_tok->token.private_key.key_size;
> - rc = pki_encrypt_session_key(auth_tok, crypt_stat, key_rec);
> + rc = pki_encrypt_session_key(auth_tok_key, auth_tok, crypt_stat,
> + key_rec);
> if (rc) {
> printk(KERN_ERR "Failed to encrypt session key via a key "
> "module; rc = [%d]\n", rc);
> @@ -2421,6 +2432,8 @@ ecryptfs_generate_key_packet_set(char *dest_base,
> &max, auth_tok,
> crypt_stat, key_rec,
> &written);
> + up_write(&(auth_tok_key->sem));
> + key_put(auth_tok_key);
> if (rc) {
> ecryptfs_printk(KERN_WARNING, "Error "
> "writing tag 3 packet\n");
> @@ -2438,8 +2451,8 @@ ecryptfs_generate_key_packet_set(char *dest_base,
> }
> (*len) += written;
> } else if (auth_tok->token_type == ECRYPTFS_PRIVATE_KEY) {
> - rc = write_tag_1_packet(dest_base + (*len),
> - &max, auth_tok,
> + rc = write_tag_1_packet(dest_base + (*len), &max,
> + auth_tok_key, auth_tok,
> crypt_stat, key_rec, &written);
> if (rc) {
> ecryptfs_printk(KERN_WARNING, "Error "
> @@ -2448,14 +2461,13 @@ ecryptfs_generate_key_packet_set(char *dest_base,
> }
> (*len) += written;
> } else {
> + up_write(&(auth_tok_key->sem));
> + key_put(auth_tok_key);
> ecryptfs_printk(KERN_WARNING, "Unsupported "
> "authentication token type\n");
> rc = -EINVAL;
> goto out_free;
> }
> - up_write(&(auth_tok_key->sem));
> - key_put(auth_tok_key);
> - auth_tok_key = NULL;
> }
> if (likely(max > 0)) {
> dest_base[(*len)] = 0x00;
> @@ -2468,11 +2480,6 @@ out_free:
> out:
> if (rc)
> (*len) = 0;
> - if (auth_tok_key) {
> - up_write(&(auth_tok_key->sem));
> - key_put(auth_tok_key);
> - }
> -
> mutex_unlock(&crypt_stat->keysig_list_mutex);
> return rc;
> }
>
I think a safer solution may be to pass all the data required
for the external decryption from the kernel side to ecryptfsd,
so that the latter can perform the operation without
requesting the key.
Roberto Sassu
--
To unsubscribe from this list: send the line "unsubscribe ecryptfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
