Jump to conversation
Unresolved conversations (0)
Nice work!

Nice work!

All of your conversations have been resolved.

Resolved conversations (19)
@lunny lunny Jun 4, 2022
blank line.
integrations/api_httpsig_test.go
@lunny lunny Jun 4, 2022
Copyright head
integrations/api_httpsig_test.go
@lunny lunny Jun 3, 2022
```go if asymkey_model.IsErrKeyNotExist(err) { } else if err != nil ```
Outdated
services/auth/httpsign.go
@lunny lunny Jun 3, 2022
We don't need this check now because every route has owned auth group.
Outdated
services/auth/httpsign.go
@6543 6543 Jun 1, 2022
https://github.com/go-ap/httpsig look more maintained
go.mod
6543 42wim
@zeripath zeripath Jun 1, 2022
Generally better to put the header keys in Canonical form. ```suggestion if len(req.Header.Get("X-Ssh-Certificate")) != 0 { ```
Outdated
services/auth/httpsign.go
@zeripath zeripath Jun 1, 2022
```suggestion log.Debug("VerifyPubKey on request from %s: failed: %v", req.RemoteAddr, err) log.Warn("Failed authentication attempt from %s", req.RemoteAddr) ```
Outdated
services/auth/httpsign.go
@zeripath zeripath Jun 1, 2022
```suggestion log.Debug("VerifyCert on request from %s: failed: %v", req.RemoteAddr, err) log.Warn("Failed authentication attempt from %s", req.RemoteAddr) ```
Outdated
services/auth/httpsign.go
@zeripath zeripath May 30, 2022
```suggestion log.Debug("Failed authentication attempt from %s: VerifyCert failed: %v", req.RemoteAddr, err) ```
Outdated
services/auth/httpsign.go
@zeripath zeripath May 30, 2022
There are other algorithms for ssh-rsa... We should be checking against rsa-sha2-256 and rsa-sha2-512 too. RSA_SHA1 is dying as it's likely broken.
Outdated
services/auth/httpsign.go
42wim
@zeripath zeripath May 30, 2022
Needs a comment. Perhaps something like below? ```suggestion // doVerify iterates across the provided public keys attempting the verify the current request against each key in turn func doVerify(verifier httpsig.Verifier, publickeys []ssh.PublicKey) error { ```
Outdated
services/auth/httpsign.go
@zeripath zeripath May 30, 2022
Marshal `auth` once and once only. Do not repeatedly run `auth.Marshal()`. (I've just noticed that modules/ssh/ssh.go does the same thing - can we change it there too?) ```suggestion IsUserAuthority: func(auth ssh.PublicKey) bool { marshaled := auth.Marshal() for _, k := range setting.SSH.TrustedUserCAKeysParsed { if bytes.Equal(marshaled, k.Marshal()) { return true } } return false }, ```
Outdated
services/auth/httpsign.go
42wim zeripath
@lunny lunny Feb 13, 2022
break?
Outdated
services/auth/httpsign.go
zeripath 42wim
@lunny lunny Feb 13, 2022
Move this to line 106
Outdated
services/auth/httpsign.go
@lunny lunny Feb 13, 2022
```suggestion cert, ok := pk.(*ssh.Certificate) if !ok { return validpk, fmt.Errorf("no certificate found") } ```
Outdated
services/auth/httpsign.go
@lunny lunny Feb 13, 2022
blank line is needed.
services/auth/httpsign.go
@zeripath zeripath Jan 17, 2022
Line 86 uses "x-ssh-certificate". Are you sure that this shouldn't also be "X-SSH-Certificate" too?
services/auth/httpsign.go
42wim
@a1012112796 a1012112796 Nov 16, 2021
why need two logs?
Outdated
services/auth/httpsign.go
zeripath 42wim
@lunny lunny Nov 6, 2021
return err
Outdated
services/auth/httpsign.go
42wim zeripath