allow native and ssh-keygen public key check #2637
Conversation
This commit adds the possibibility to use either the native golang libraries or ssh-keygen to check public keys. The check is adjusted depending on the settings, so that only supported keys are let through. This commit also brings back the blacklist feature, which was removed in 7ef9a05. This allows to blacklist algorythms or keys based on the key length. This works with the native and the ssh-keygen way. Because of gogs#2179 it also includes a way to adjust the path to ssh-keygen and the working directory for ssh-keygen. With this, sysadmins should be able to adjust the settings in a way, that SELinux is okay with it. In the worst case, they can switch to the native implementation and only loose support for ed25519 keys at the moment. There are some other places which need adjustment to utilize the parameters and the native implementation, but this sets the ground work.
unknwon
changed the title
|
PS: build is failing... |
The old API was using []byte, but was changed to string without running the tests again. It also sets the variables from the configuration to make them work. Maybe there is a better way to do this.
|
Now this error I have no idea how to fix that. I tried with openssh 5.9 and that returns "is not a public key file" for ed25519 public keys. But what is travis using in this case, that it returns "file too long"? I would like to keep the tests, but if it breaks the build, it is kind of stupid. Should I delete them or should I just remove the ed25519 key from the test? |
TravisCI is too old for ed25519, so it can't be tested correctly.
|
I guess we need special instructions to tell Travis (but maybe not possible?) support ed25519 key. Is this PR finished now? |
|
Not yet. I want to adjust all occurences of ssh-keygen to use the new variables. But if you want, you could pull this request in and the other stuff will be done extra. |
models/ssh_key.go
Outdated
| var sshOpLocker = sync.Mutex{} | ||
| var ( | ||
| sshOpLocker = sync.Mutex{} | ||
| SSH_UNKNOWN_KEY_TYPE = fmt.Errorf("unknown key type") |
There was a problem hiding this comment.
Variable should not use ALL_CAPS.
|
What is the different between this PR and revert old approach back? |
|
This is a combination of the old behavior and the new one. At the end it should be possible to use the go native ssh library only without any dependency on ssh-keygen at all. Currently, there are still some places in need of ssh-keygen. |
modules/setting/setting.go
Outdated
| HTTP Scheme = "http" | ||
| HTTPS Scheme = "https" | ||
| FCGI Scheme = "fcgi" | ||
| SSH_PUBLICKEY_CHECK_NATIVE = "native" |
There was a problem hiding this comment.
This const section is for Scheme, do not mixed up with them.
|
ping @Gibheer |
The contants were placed in the same section as the scheme ones, which may lead to confusion.
DisableSSH doesn't check the kind of ssh server to use, so that was wrong. Use StartSSHServer instead.
|
@unknwon sorry, took a while longer. I fixed the stuff you found and replaced the ssh-keygen call with the new variable from setting. You can merge this for now, as I do not know when I have the time to do the rest (making ssh-keygen only necessary when the system sshd is used). |
Gibheer
changed the title
unknwon
added
status: waits for review
|
Thanks! |
allow native and ssh-keygen public key check
Improve test cases, config settings, also show SSH config settings on admin config panel.
2 participants
Hi,
this is the first part of fixing #2179, namely the SELinux and ssh-keygen problem. The first commit is pretty large, but adding public keys now works again using ssh-keygen.
Please do not merge yet, as there is still some stuff missing. I introduced new settings to make it possible to adjust the working directory for ssh-keygen and even adjust the ssh-keygen path. I have to update all occurrences of ssh-keygen for that still.
But if possible, I welcome everyone to test the patch :)