Skip to content
This repository has been archived by the owner on Oct 10, 2023. It is now read-only.

Feature: 支持密码加密选项 #16

Open
MicroCBer opened this issue Jan 19, 2023 · 13 comments
Open

Feature: 支持密码加密选项 #16

MicroCBer opened this issue Jan 19, 2023 · 13 comments

Comments

@MicroCBer
Copy link
Contributor

No description provided.

@MaikoTan
Copy link
Member

What is a "password encryption option"?
Please clarify.

@MicroCBer
Copy link
Contributor Author

encrypt: false

@MaikoTan
Copy link
Member

MaikoTan commented Jan 27, 2023

Oh, it is just an option. Easy-peasy 🤣

But I don't know what exactly it is, or what can it do for the users.


Maiko's everyday word:

easy-peasy: when you want to say something is very easy and straightforward to do, you can say "easy-peasy".

舞子的每日一詞:

簡單得很:當你想要形容一件事情可以非常容易且直接地完成,你可以說「簡單豌豆的」。

@MicroCBer
Copy link
Contributor Author

Oh, it is just an option. Easy-peasy 🤣

But I don't know what exactly it is, or what can it do for the users.

It allows the user to encrypt the password and decrypt it when gocqhttp is launched, which can avoid direct password exposure in the config file.

The user would input the password through standard input, so a password prompt in the web console should also be implemented.

Source code about encrypt config: https://github.com/Mrs4s/go-cqhttp/blob/49a8b9bd644957b7e348d6cb41294d834116dea7/cmd/gocq/main.go#L151-L199

@MaikoTan
Copy link
Member

Oh, it is just an option. Easy-peasy 🤣

But I don't know what exactly it is, or what can it do for the users.

The user would input the password through standard input, so a password prompt in the web console should also be implemented.

Maji yabaku ne! It is no longer "just an option". 😭

So the encrypt option can not be used together with password as my understanding?

@MicroCBer
Copy link
Contributor Author

nah, it requires the user to put the password in the config at first with the encrypt option enabled. then it would check if file password.encrypt exists. if it doesn't exist, that means the password is not encrypted yet, then it would ask the user for the encryption password, encrypt the password with the encryption password, write the encrypted password into the file password.encrypt and ask the user to remove the password in password field in config.yml. So that the user should first write the password in config.yml, start gocqhttp to encrypt it, then remove the paddword from config.yml and restart gocqhttp.


Generated by koishi-plugin-github.

@MicroCBer
Copy link
Contributor Author

but well, the encrypt function does prevent the user from directly putting the password in the config file. You can check the source code, it's probably easier to understand compared to my poor english


Generated by koishi-plugin-github.

@MaikoTan
Copy link
Member

Well, in fact I don't know golang much, but I will try to read it. Merci pour la explication.

@MicroCBer
Copy link
Contributor Author

MicroCBer commented Jan 27, 2023

I didn't even learn golang once lol, but the mechanism is similar, no matter what the language is.

Edit: should use mechanism instead of logic, thank you maiko


Generated by koishi-plugin-github.

@MaikoTan
Copy link
Member

Well, I have learnt more golang and read go-cqhttp code carefully. Sorry for the TL;DR-level reply, but please let me talk about my feeling of this feature here.


So, in my understanding, in order to support this option, we should do a workflow like this:

  1. Before generating the go-cqhttp configuration file, require user to enter the password and then fill it in the yaml file.
  2. Set encrypt to true, then launch go-cqhttp once.
  3. When go-cqhttp request the encrypt key, prompt user in our console webui.
  4. Feed go-cqhttp with the key from 3).
  5. Keep tracking go-cqhttp, when it exits with 请删除配置文件中的密码后重新启动., remove the password from the configuration file, then launch go-cqhttp once again.
  6. When go-cqhttp request the encrypt key, prompt user again.
  7. Feed go-cqhttp with the key from 6).

That is, no offense, freaking complicated! I don't think we could make it done in, like three days or weeks.


And from the workflow above, you should make sure the password and encrypt options are not both set concurrently. Otherwise, the exposure prevent is not reasonable in this case.

On the other hand, when user enable this, he/she/it/whatever would be required to enter at least THRICE (even we cache the key that used in 4) and 7), still required twice).
In the mean time, the plugin should launch the go-cqhttp at least twice and always keep tracking the sub-process and its output, for choosing the correct solution.
But if there are errors shown at any part of the workflow, like the user forgot the key, the only way is to delete the password.encrypt file. That is an extra addition to this work, also (in my opinion) not a user-friendly solution.


Overall, I don't think this feature is worth to be implemented by our side.
If you really need to encrypt your password, please launch your go-cqhttp manually and independently.

I would quote what @undefined-moe said in the corresponding issue in go-cqhttp

cqhttp能够解密表明其他软件也可以解密。
建议您优化 config.json 的文件权限。

As if you could handle all of the operations above, you or a random independent Koishier should also know how to maintain your own go-cqhttp without the adorable Koishi.

Best regard.
Maiko Sinkyaet Tan

@MicroCBer
Copy link
Contributor Author

MicroCBer commented Jan 29, 2023

Well, I have learnt more...........own go-cqhttp without the adorable Koishi.

I actually think one of us (probably it's me) doesn't completely understand the source code. Let's take a look at this part.

	if len(byteKey) == 0 {
		log.Infof("密码加密已启用, 请输入Key对密码进行解密以继续: (Enter 提交)")
		cancel := make(chan struct{}, 1)
		state, _ := term.GetState(int(os.Stdin.Fd()))
		go func() {
			select {
			case <-cancel:
				return
			case <-time.After(time.Second * 45):
				log.Infof("解密key输入超时")
				time.Sleep(3 * time.Second)
				_ = term.Restore(int(os.Stdin.Fd()), state)
				os.Exit(0)
			}
		}()
		byteKey, _ = term.ReadPassword(int(os.Stdin.Fd()))
		cancel <- struct{}{}
	} else {
		log.Infof("密码加密已启用, 使用运行时传递的参数进行解密,按 Ctrl+C 取消.")
	}

	encrypt, _ := os.ReadFile("password.encrypt")
	ph, err := PasswordHashDecrypt(string(encrypt), byteKey)
	if err != nil {
		log.Fatalf("加密存储的密码损坏,请尝试重新配置密码")
	}
	copy(base.PasswordHash[:], ph)

In my understanding, it would first check if there's an argument that provides the 'password' to decrypt the login password. If so, use it to decrypt the password, else wise, ask the user to input the password in the terminal. This would solve the problem that Masnn mentioned: the 'password' to decrypt the login password is not stored and is memorized by the admin, so only if the user inputs the password in the terminal or provide it through arguments, gocqhttp would be able to decrypt the password. Elsewise, neither gocqhttp nor other software would be able to decrypt it.


Also, I'd like to share some situations when we need it:

  1. It would be helpful if Koishi backend has some critical vulnerability for arbitrary file reading.
  2. I've seen many users directly send their config.yml out when they are asking for help. This would prevent accidental password leaks.
  3. When the server was cracked, this would stop the hacker from stealing the account. Ofc this won't work if the hacker get permission of reading memory, but it still works when the hacker cannot get root permission.
  4. It protects the user who doesn't use auth plugin.

The workflow can be easily simplified with the password args. We just need to (fake code):

pwd_encrypt = ask the user for the password.
if( file "password.encrypt" not exists ){
    pwd = ask the user for qq password
    write pwd to config.yml and enable encrypt option
    start process 'gocqhttp'
    pass the password to standard input
    wait until it exits
}
remove the password in config.yml, save it.
start process 'gocqhttp' with arg `--pwd=${pwd_encrypt}`

Anyway, I think that's a good feature and I'd like to leave the issue here. You contributors take the final determination if implement this feature or not.


(5@ DXGO16ACC_EC{MO2FSN

Sincerely,
MicroBlock

@MaikoTan
Copy link
Member

Attention please, I am not saying that shouldn't you use the encrypt feature. I am just saying that it is hard to implement the workflow and the UI/UX in our side.
To be clear, there is always a way that you could launch your go-cqhttp outside independently.
Even our gentle-and-kawaii @shigma -chan would take this work, I don't think this is an urgent demand.

@MicroCBer
Copy link
Contributor Author

Attention please, I am not saying that shouldn't you use the encrypt feature. I am just saying that it is hard to implement the workflow and the UI/UX in our side. To be clear, there is always a way that you could launch your go-cqhttp outside independently. Even our gentle-and-kawaii @shigma -chan would take this work, I don't think this is an urgent demand.

Sorry if I caused a misunderstanding, I'm actually replying to the reply that you referenced of Masnn and explaining why it's not a problem here. I don't mean you said I shouldn't use encrypt feature. I agree that it's not an urgent demand but it's really good to have. The screenshot here is just for fun xD.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants