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

Support argon2id password hash algo #637

Merged
merged 7 commits into from
May 22, 2024
Merged

Support argon2id password hash algo #637

merged 7 commits into from
May 22, 2024

Conversation

feiniks
Copy link
Contributor

@feiniks feiniks commented Nov 17, 2023

No description provided.

}

int
pdkdf2_sha256_derive_key (const char *data_in, int in_len, int version,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不需要把兼容多种 enc 版本的代码复制过来。这个 password-hash.c 的代码应该重新写。对于 pbkdf2_256 算法应该只支持使用 repo_salt 的情况。(参数名字叫 salt 就行了,把这个函数看做一个通用的工具函数。)概念上,你应该把新写的这些代码和原有的 derive_key 函数完全分开来看。新写的函数就只是用来处理指定了 hash_algo 的情况,且仅用于计算 password hash,不用于 derive 加密 random key 用的 key 和 iv。

另外函数名字 pbkdf2 写错了。


int
argon2id_derive_key (const char *data_in, int in_len, int version,
const char *repo_salt,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个函数也是不需要考虑 version 问题,直接接受传入的 salt 即可。

另外,这两个 derive_key 函数应该定义为 static 吧。

}

void
pwd_hash_init (const char *algo, const char *params_str, PwdHashParams *params)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个函数应该没有必要了。需要生成 pwd_hash 或者验证密码的代码,只需要读取相应的配置字符串,传入给 pwd_hash_derive_key 函数就行了。使用 pwd_hash 模块的代码自己去判断是否要使用新的 password hash 算法。

int
seafile_derive_key (const char *data_in, int in_len, int version,
const char *repo_salt,
const char *algo, const char *params_str,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个函数不用改了。

@@ -107,6 +90,8 @@ int
seafile_generate_random_key (const char *passwd,
int version,
const char *repo_salt,
const char *algo,
const char *params,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个函数不用改。

@@ -139,6 +124,8 @@ void
seafile_generate_magic (int version, const char *repo_id,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个函数不用改。应该有一个新的函数用来基于 pwd_algo 生成 pwd_hash。

@@ -193,11 +190,12 @@ int
seafile_decrypt_repo_enc_key (int enc_version,
const char *passwd, const char *random_key,
const char *repo_salt,
const char *algo, const char *params_str,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

后面这两个函数也不用改。

return -1;
}
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这块的逻辑需要根据我的评论重新写一下。是否使用新的 password hash 算法和加密版本应该无关。

@@ -633,12 +636,18 @@ commit_to_json_object (SeafCommit *commit)

if (commit->encrypted) {
json_object_set_int_member (object, "enc_version", commit->enc_version);
if (commit->enc_version >= 1)
// If pwd_hash is setted, the magic field is no longer included in the commit of the newly created repo.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If pwd_hash is set

@@ -739,6 +754,10 @@ commit_from_json_object (const char *commit_id, json_t *object)
(second_parent_id && !is_object_id_valid(second_parent_id)))
return commit;

// If pwd_hash is setted, the magic field is no longer included in the commit of the newly created repo.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个注释也改一下。

@@ -800,6 +819,11 @@ commit_from_json_object (const char *commit_id, json_t *object)
commit->random_key = g_strdup (random_key);
if (enc_version >= 3)
commit->salt = g_strdup(salt);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

上面的代码,如果 pwd_hash != NULL,那么不要设置 magic。这样数据结构会干净一点,也更一致。


// pwd_hash_init is used to init default pwd hash algorithms.
void
pwd_hash_init (const char *algo, const char *params_str, PwdHashParams *params)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个函数名字改为 parse_pwd_hash_params 吧。

@@ -696,6 +698,8 @@ GObject *
seafile_generate_magic_and_random_key(int enc_version,
const char* repo_id,
const char *passwd,
const char *pwd_hash_algo,
const char *pwd_hash_params,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个 rpc 不需要传入这两个参数。这两个参数应该是底层内部确定的。新建的资料库应该总是使用配置文件中的 pwd_hash 算法和参数;如果没有配置,那么就是用旧的 magic 计算方式。

seafile_crypt_get_pwd_hash_algo ();

const char *
seafile_crypt_get_pwd_hash_params ();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这两个函数的名字加上 default 吧。

@@ -146,6 +149,11 @@ func Get(id string) *Repo {
repo.RandomKey = commit.RandomKey
repo.Salt = commit.Salt
}
if commit.PwdHash != "" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commit to repo 和 repo to commit 的逻辑应该和 C 部分的一致吧。就是如果 PwdHash 不为空,那么 Magic 就应该为空。

const char *pwd_hash;
const char *pwd_hash_algo;
const char *pwd_hash_params;
} RepoCryptCompat;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个改为 RepoCryptInfo 吧。

@killing killing merged commit 96996b7 into master May 22, 2024
2 checks passed
@killing killing deleted the enc_hash branch May 22, 2024 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants