Use ErrKeyUnableToVerify if fail to calc fingerprint in ssh-keygen

[zeripath]

Fix #3985

Signed-off-by: Andrew Thornton [email protected]

[sapk]

Looks good to me but would be better with a new test case here:

gitea/models/ssh_key_test.go

Line 159 in f9f2c16

{"ecdsa-384", "SHA256:4qfJOgJDtUd8BrEjyVNdI8IgjiZKouztVde43aDhe1E", "ecdsa-sha2-nistp384 AAAAE2VjZHNhLXNoYTItbmlzdHAzODQAAAAIbmlzdHAzODQAAABhBINmioV+XRX1Fm9Qk2ehHXJ2tfVxW30ypUWZw670Zyq5GQfBAH6xjygRsJ5wWsHXBsGYgFUXIHvMKVAG1tpw7s6ax9oA+dJOJ7tj+vhn8joFqT+sg3LYHgZkHrfqryRasQ== nocomment"},

and even better with an integration test using :
https://github.com/go-gitea/gitea/blob/f9f2c163b12f60a6bf9c3652cdc4bdf59e3d53c4/integrations/api_helper_for_declarative_test.go#L140:6

[zeripath]

unfortunately I don't have an example otherwise I'd have done the tests

[zeripath]

I found one in https://github.com/Ganapati/RsaCtfTool/blob/master/examples/weak_public.pub#L1

[sapk]

@zeripath I have created a rsa 1023 bit public key

ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgFSbX7pXBc5xW5ByF4RB6Lr8HbfW0/LXq90UjWpRSR6z+HEfN/4m8jRF5Vl/GCtu86YSi9au1TeZ0kHgTTCgQIv5vc+zZJkx4mbzpQ+WzEYB9SET4W3AZBv3diJVXQX6KuMICgq30PeVB4PsZoyBasjSS422pCCgFDpH/l6JgYlJ

[sapk]

I have just seen your message and you can use either.
I have tested the key I provided with ssh-keygen:

ssh-keygen -lf rsa_test.pub
rsa_test.pub is not a public key file.

[codecov-io]

Codecov Report

Merging #10863 into master will increase coverage by 0.01%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master   #10863      +/-   ##
==========================================
+ Coverage   43.42%   43.43%   +0.01%     
==========================================
  Files         593      593              
  Lines       83262    83268       +6     
==========================================
+ Hits        36155    36168      +13     
+ Misses      42615    42614       -1     
+ Partials     4492     4486       -6     

Impacted Files	Coverage Δ
models/ssh_key.go	50.64% <0.00%> (-0.22%)	⬇️
routers/user/setting/keys.go	12.38% <0.00%> (-0.34%)	⬇️
modules/queue/workerpool.go	56.93% <0.00%> (-2.14%)	⬇️
modules/git/repo.go	48.11% <0.00%> (+0.83%)	⬆️
modules/git/command.go	89.56% <0.00%> (+2.60%)	⬆️
services/pull/check.go	53.04% <0.00%> (+3.04%)	⬆️
modules/process/manager.go	78.31% <0.00%> (+3.61%)	⬆️
modules/git/utils.go	70.14% <0.00%> (+4.47%)	⬆️
modules/indexer/stats/db.go	59.37% <0.00%> (+9.37%)	⬆️

---

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 f9f2c16...b5d799f. Read the comment docs.

[zeripath]

OK so the issue is that calcFingerprintNative will happily parse both of those keys but calcFingerprintSSHKeygen will not.

[zeripath]

Happily however, testing with both those keys has shown to me that I missed passing up the error upstream.

[zeripath]

So I've discovered that CheckPublicKeyString doesn't seem actually checking the length correctly. - that's because it's not set on by default!

[zeripath]

Ah, if we have Native SSH key gen we should probably have the minimum key sizes set by default to at least what they are in ssh keygen

[sapk]

@zeripath yes the standard go lib for rsa is not restricting size (we should check it).
That how I generate the rsa for the test.