9 ms·

Cool! (not a security professional) One of the reasons in my understanding that projects don't deviate from older languages was because they give you some contr

Cool! (not a security professional) One of the reasons in my understanding that projects don't deviate from older languages was because they give you some control around compilation that would be necessary to thwart timing-based attacks. Does this consider timing-based attacks or is that at a lower-level library?

m_sahaf 4y ago
Not very too level. I had to take that into consideration at some parts. For example, the static username_password provider calls a hasher defined in Caddy which uses `subtle.ConstantTimeCompare` function used. At other places, I don't return early (when possible) on auth failures to avoid timing attacks. That said, I'd love to know if there are places where I fell short.
gunapologist99 4y ago
Excellent point; it does appear that the author did consider timing attacks in at least one location: https://github.com/mohammed90/caddy-ssh/blob/master/internal/ssh/ssh.go#L126 https://github.com/mohammed90/caddy-ssh/blob/master/internal...
- m_sahaf 4y ago
  To be fair, this bit is borrowed/forked from github.com/gliderlabs/ssh.
  - thefreeman 4y ago
    Curious why you would just copy the entire gilderlabs/ssh package into your repo instead of referencing it as a module? How do you plan to keep up to date with bug / security fixes?
    - francislavoie 4y ago
      Some of the changes necessary were too invasive/breaking to gliderlabs/ssh, such as https://github.com/gliderlabs/ssh/pull/161 https://github.com/gliderlabs/ssh/pull/161 so making a copy ended up making more sense, I think. That's my understanding from what Mohammed's told me, anyways.
      - m_sahaf 4y ago
        That's right. Moreover, the activity no the repo has been a bit stagnant (no disrespect to the maintainers, they are likely busy with other projects or life). Other projects have opted to fork the repo, rewrite the module repo path, and maintain their fork (e.g. https://github.com/tailscale/ssh/ https://github.com/tailscale/ssh/, but I've seen many others too). I didn't want to maintain a fork, so a copy under internal/ in my own repo means it's firm-fork (between soft and hard) where I can confidently break its APIs I'm the only one depending on without worry. It will slowly morph to fit the needs of the project only. The maintainers gliderlabs/ssh have plans for a new version with more ergonomic APIs but the work hasn't started yet. I didn't want to wait either.
enneff 4y ago
I have reviewed some of the crypto code in the Go standard library that this is built with, and there is use of constant time primitives in there so it is at least possible and some attention has been spent on it.

m_sahaf 4y ago

Not very too level. I had to take that into consideration at some parts. For example, the static username_password provider calls a hasher defined in Caddy which uses `subtle.ConstantTimeCompare` function used. At other places, I don't return early (when possible) on auth failures to avoid timing attacks. That said, I'd love to know if there are places where I fell short.

gunapologist99 4y ago

Excellent point; it does appear that the author did consider timing attacks in at least one location: https://github.com/mohammed90/caddy-ssh/blob/master/internal/ssh/ssh.go#L126 https://github.com/mohammed90/caddy-ssh/blob/master/internal...

m_sahaf 4y ago
To be fair, this bit is borrowed/forked from github.com/gliderlabs/ssh.
- thefreeman 4y ago
  Curious why you would just copy the entire gilderlabs/ssh package into your repo instead of referencing it as a module? How do you plan to keep up to date with bug / security fixes?
  - francislavoie 4y ago
    Some of the changes necessary were too invasive/breaking to gliderlabs/ssh, such as https://github.com/gliderlabs/ssh/pull/161 https://github.com/gliderlabs/ssh/pull/161 so making a copy ended up making more sense, I think. That's my understanding from what Mohammed's told me, anyways.
    - m_sahaf 4y ago
      That's right. Moreover, the activity no the repo has been a bit stagnant (no disrespect to the maintainers, they are likely busy with other projects or life). Other projects have opted to fork the repo, rewrite the module repo path, and maintain their fork (e.g. https://github.com/tailscale/ssh/ https://github.com/tailscale/ssh/, but I've seen many others too). I didn't want to maintain a fork, so a copy under internal/ in my own repo means it's firm-fork (between soft and hard) where I can confidently break its APIs I'm the only one depending on without worry. It will slowly morph to fit the needs of the project only. The maintainers gliderlabs/ssh have plans for a new version with more ergonomic APIs but the work hasn't started yet. I didn't want to wait either.

enneff 4y ago

I have reviewed some of the crypto code in the Go standard library that this is built with, and there is use of constant time primitives in there so it is at least possible and some attention has been spent on it.