don't check minimum key size when disabled

[Gibheer]

This fixes an issue that the key size was checked even when the config option was disabled. This led to errors on systems where ssh-keygen didn't report the key options in the expected format.

The other commit removes some older comments which are not correct anymore.

[sapk]

Is this related to a issue encounter ? because https://github.com/go-gitea/gitea/blob/master/models/ssh_key.go#L278 would block at start the check.
Placing the check for size in SSHNativeParsePublicKey would disable the check for not built-in ssh server.

For https://github.com/Gibheer/gitea/blob/ab376ba9fb202b3c9189c57fa69476ae0f18216f/models/ssh_key.go#L268 👍

[Gibheer]

Yes, this is because of an issue encountered in gogs ( #4507 ).
When you are running gitea on an older system where openssh is a tad older, the output format of ssh-keygen was different. This shouldn't be a problem when you don't want to check the minimum key size at all.
But the way it was before, ssh-keygen was called and the output parsed before the config option was checked if that should be done at all. By moving the code line a couple lines up, we have the correct order by first checking the config option and then proceeding with parsing the key.

[sapk]

yes but it disable the check for running with OS ssh server and not the internal. The solution could be to check the version of ssh-keygen for OS ssh server case and make the proper analyze based on that. We could also error on too old ssh-keygen version and suggest to use the internal ssh server in place.

[sapk]

We could also just warn that we couldn't check the size of key in too old version of ssh-keygen.

[Gibheer]

I wouldn't like to just warn the admin about the issue. If he wants to use the feature, he should be able to recognize the problem through errors. Then he can decide if he wants to upgrade or disable the feature, pretty easy.
The issue does not happen with the inbuilt server, as the check for the inbuilt server will never call ssh-keygen.
As for figuring out the version used on the server - I don't think there are that many old versions still in use. CentOS 5 is using an old version, but these servers should be upgraded regardless. Adding such a check might also cause problems in the future, when the format is changed again.
In that case, the admin is able to just disable the feature for some time or downgrade the installation again until we have a fix. But adding version checks would result in maintenance load with every openssh release,as we would need to add the new version to a list or something like that.

[lafriks]

LGTM

[lunny]

LGTM

[codecov-io]

Codecov Report

❗ No coverage uploaded for pull request base (master@eca05b0). Click here to learn what that means.
The diff coverage is 0%.

@@            Coverage Diff            @@
##             master    #1754   +/-   ##
=========================================
  Coverage          ?   27.21%           
=========================================
  Files             ?       88           
  Lines             ?    17348           
  Branches          ?        0           
=========================================
  Hits              ?     4721           
  Misses            ?    11942           
  Partials          ?      685

Impacted Files	Coverage Δ
models/ssh_key.go	10.6% <0%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eca05b0...ab4034e. Read the comment docs.