Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ZFS Encryption #4329

Closed
wants to merge 10 commits into from

several one-line fixes and name cleanups

  • Loading branch information...
tcaputi committed Jan 18, 2017
commit 71434a257245ffedc1fd128cf2896a9ab23d8b6b
Copy path View file
@@ -179,7 +179,7 @@ object_from_path(const char *dataset, const char *path, struct stat64 *statbuf,
*/
sync();

err = dmu_objset_own(dataset, DMU_OST_ZFS, B_TRUE, B_TRUE, FTAG, &os);
err = dmu_objset_own(dataset, DMU_OST_ZFS, B_TRUE, B_FALSE, FTAG, &os);
if (err != 0) {
(void) fprintf(stderr, "cannot open dataset '%s': %s\n",
dataset, strerror(err));
@@ -189,7 +189,7 @@ object_from_path(const char *dataset, const char *path, struct stat64 *statbuf,
record->zi_objset = dmu_objset_id(os);
record->zi_object = statbuf->st_ino;

dmu_objset_disown(os, B_TRUE, FTAG);
dmu_objset_disown(os, B_FALSE, FTAG);

return (0);
}
@@ -267,7 +267,7 @@ calculate_range(const char *dataset, err_type_t type, int level, char *range,
* size.
*/
if ((err = dmu_objset_own(dataset, DMU_OST_ANY,
B_TRUE, B_TRUE, FTAG, &os)) != 0) {
B_TRUE, B_FALSE, FTAG, &os)) != 0) {
(void) fprintf(stderr, "cannot open dataset '%s': %s\n",
dataset, strerror(err));
goto out;
@@ -329,7 +329,7 @@ calculate_range(const char *dataset, err_type_t type, int level, char *range,
dnode_rele(dn, FTAG);
}
if (os)
dmu_objset_disown(os, B_TRUE, FTAG);
dmu_objset_disown(os, B_FALSE, FTAG);

return (ret);
}
Copy path View file
@@ -60,7 +60,7 @@ typedef struct dsl_crypto_params {
zfs_ioc_crypto_cmd_t cp_cmd;

/* the encryption algorithm */
uint64_t cp_crypt;
enum zio_encrypt cp_crypt;

/* the salt, if the keysource is of type passphrase */
uint64_t cp_salt;
Copy path View file
@@ -1257,19 +1257,21 @@ spa_keystore_rewrap(const char *dsname, dsl_crypto_params_t *dcp)
}

int
dmu_objset_create_encryption_check(dsl_dir_t *pdd, dsl_crypto_params_t *dcp)
dmu_objset_create_encryption_check(dsl_dir_t *parentdd,
dsl_crypto_params_t *dcp)
{
int ret;
dsl_wrapping_key_t *wkey = NULL;
uint64_t cmd = 0, salt = 0, iters = 0;
uint64_t salt = 0, iters = 0;
zfs_ioc_crypto_cmd_t cmd = ZFS_IOC_KEY_CMD_NONE;
uint64_t pcrypt, crypt = ZIO_CRYPT_INHERIT;
const char *keysource = NULL;

if (!spa_feature_is_enabled(pdd->dd_pool->dp_spa,
if (!spa_feature_is_enabled(parentdd->dd_pool->dp_spa,
SPA_FEATURE_ENCRYPTION) && dcp)
return (SET_ERROR(EINVAL));

ret = dsl_prop_get_dd(pdd, zfs_prop_to_name(ZFS_PROP_ENCRYPTION),
ret = dsl_prop_get_dd(parentdd, zfs_prop_to_name(ZFS_PROP_ENCRYPTION),
8, 1, &pcrypt, NULL, B_FALSE);
if (ret != 0)
return (ret);
@@ -1297,12 +1299,13 @@ dmu_objset_create_encryption_check(dsl_dir_t *pdd, dsl_crypto_params_t *dcp)
return (SET_ERROR(EINVAL));
if (keysource && strncmp(keysource, "passphrase", 10) == 0 &&

This comment has been minimized.

Copy link
@ahrens

ahrens Jan 14, 2017

Contributor

You are checking that the keysource starts with "passphrase". E.g. "passphrasefoo" will match. Is that intentional? If so, I would suggest that you declare "passphrase" elsewhere, and use strlen(passphrasevar) here. If it's unintentional, use strcmp().

This comment has been minimized.

Copy link
@tcaputi

tcaputi Jan 14, 2017

Author Contributor

This is intentional. The keysource will be something like passphrase,prompt or passphrase,file:///home/tom/key or 'hex,prompt`. This is just a check to make sure that passphrases have the pbkdf2 parameters.

This comment has been minimized.

Copy link
@ahrens

ahrens Jan 14, 2017

Contributor

Where does the kernel check that the keysource is valid? I don't see that happening here or in dsl_crypto_params_create_nvlist().

This comment has been minimized.

Copy link
@tcaputi

tcaputi Jan 14, 2017

Author Contributor

It probably should have more rigorous checks. I will add them. The property isnt actually used by the kernel at all. It is used by libzfs to figure out where it should load the keys from.

(!salt || !iters))
return (SET_ERROR(EINVAL));
if (cmd)

This comment has been minimized.

Copy link
@ahrens

ahrens Jan 14, 2017

Contributor

indentation is wrong (should be 2 tabs), or (more likely) there's a line missing above. I'm surprised that cstyle -cpP didn't catch this.

This comment has been minimized.

Copy link
@ahrens

ahrens Jan 14, 2017

Contributor

This would be more clear as cmd != ZFS_IOC_KEY_CMD_NONE

This comment has been minimized.

Copy link
@tcaputi

tcaputi Jan 14, 2017

Author Contributor

This is actually a bug. I must have accidentally rebased away the line that should go there. 1306 should have another return (SET_ERROR(EINVAL));. My tests probably haven't shown this because it is also protected by userspace.

return (SET_ERROR(EINVAL));

if (!wkey && pcrypt != ZIO_CRYPT_OFF) {
ret = spa_keystore_wkey_hold_ddobj(pdd->dd_pool->dp_spa,
pdd->dd_object, FTAG, &wkey);
ret = spa_keystore_wkey_hold_ddobj(parentdd->dd_pool->dp_spa,
parentdd->dd_object, FTAG, &wkey);
if (ret != 0)
return (SET_ERROR(EACCES));

@@ -1313,7 +1316,7 @@ dmu_objset_create_encryption_check(dsl_dir_t *pdd, dsl_crypto_params_t *dcp)
}

int
dmu_objset_clone_encryption_check(dsl_dir_t *pdd, dsl_dir_t *odd,
dmu_objset_clone_encryption_check(dsl_dir_t *parentdd, dsl_dir_t *origindd,
dsl_crypto_params_t *dcp)
{
int ret;
@@ -1322,17 +1325,17 @@ dmu_objset_clone_encryption_check(dsl_dir_t *pdd, dsl_dir_t *odd,
uint64_t pcrypt, ocrypt, crypt = ZIO_CRYPT_INHERIT;
const char *keysource = NULL;

if (!spa_feature_is_enabled(pdd->dd_pool->dp_spa,
if (!spa_feature_is_enabled(parentdd->dd_pool->dp_spa,
SPA_FEATURE_ENCRYPTION) && dcp)
return (SET_ERROR(EINVAL));

ret = dsl_prop_get_dd(pdd, zfs_prop_to_name(ZFS_PROP_ENCRYPTION), 8, 1,
&pcrypt, NULL, B_FALSE);
ret = dsl_prop_get_dd(parentdd, zfs_prop_to_name(ZFS_PROP_ENCRYPTION),
8, 1, &pcrypt, NULL, B_FALSE);
if (ret != 0)
return (ret);

ret = dsl_prop_get_dd(odd, zfs_prop_to_name(ZFS_PROP_ENCRYPTION), 8, 1,
&ocrypt, NULL, B_FALSE);
ret = dsl_prop_get_dd(origindd, zfs_prop_to_name(ZFS_PROP_ENCRYPTION),
8, 1, &ocrypt, NULL, B_FALSE);
if (ret != 0)
return (ret);

@@ -1358,8 +1361,8 @@ dmu_objset_clone_encryption_check(dsl_dir_t *pdd, dsl_dir_t *odd,

/* origin wrapping key must be present, if it is encrypted */
if (ocrypt != ZIO_CRYPT_OFF) {
ret = spa_keystore_wkey_hold_ddobj(pdd->dd_pool->dp_spa,
odd->dd_object, FTAG, &wkey);
ret = spa_keystore_wkey_hold_ddobj(parentdd->dd_pool->dp_spa,
origindd->dd_object, FTAG, &wkey);
if (ret != 0)
return (SET_ERROR(EACCES));

@@ -1368,8 +1371,8 @@ dmu_objset_clone_encryption_check(dsl_dir_t *pdd, dsl_dir_t *odd,

/* parent's wrapping key must be present if a new one isn't specified */
if (!wkey && pcrypt != ZIO_CRYPT_OFF) {
ret = spa_keystore_wkey_hold_ddobj(pdd->dd_pool->dp_spa,
pdd->dd_object, FTAG, &wkey);
ret = spa_keystore_wkey_hold_ddobj(parentdd->dd_pool->dp_spa,
parentdd->dd_object, FTAG, &wkey);
if (ret != 0)
return (SET_ERROR(EACCES));

Copy path View file
@@ -950,7 +950,7 @@ dsl_destroy_head(const char *name)
* remove the objects from open context so that the txg sync
* is not too long.
*/
error = dmu_objset_own(name, DMU_OST_ANY, B_FALSE, B_TRUE,
error = dmu_objset_own(name, DMU_OST_ANY, B_FALSE, B_FALSE,
FTAG, &os);
if (error == 0) {
uint64_t obj;
@@ -963,7 +963,7 @@ dsl_destroy_head(const char *name)
(void) dmu_free_long_object(os, obj);
/* sync out all frees */
txg_wait_synced(dmu_objset_pool(os), 0);
dmu_objset_disown(os, B_TRUE, FTAG);
dmu_objset_disown(os, B_FALSE, FTAG);
}
}

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.