Fix known hosts check #69
Conversation
| @@ -25,7 +25,7 @@ ssh login host port command = do | |||
| public = home </> ".ssh" </> "id_rsa.pub" | |||
| private = home </> ".ssh" </> "id_rsa" | |||
| withSession host port $ \session -> do | |||
| r <- checkHost session host port known_hosts | |||
| r <- checkHost session host port known_hosts [TYPE_MASK] | |||
There was a problem hiding this comment.
I am not sure if this is the right behaviour; it basically allows everything. I opted for this because I did not want to make any more breaking changes.
| @@ -148,14 +148,17 @@ checkHost :: Session | |||
| -> String -- ^ Remote host name | |||
| -> Int -- ^ Remote port number (usually 22) | |||
| -> FilePath -- ^ Path to known_hosts file | |||
| -> [KnownHostType] -- ^ Flags specifying what format the host name is, what format the key is and what key type it is | |||
There was a problem hiding this comment.
This is a breaking change, but one that is necessary.
| kht2int KEY_ED25519 = 7 `shiftL` 18 | ||
| kht2int KEY_UNKNOWN = 15 `shiftL` 18 | ||
|
|
||
| int2kht :: CInt -> KnownHostType |
There was a problem hiding this comment.
This function is now unused, I wasn't sure whether to keep it (and possible export it) or get rid of it.
| kht2int KEY_ECDSA_384 = 5 `shiftL` 18 | ||
| kht2int KEY_ECDSA_521 = 6 `shiftL` 18 | ||
| kht2int KEY_ED25519 = 7 `shiftL` 18 | ||
| kht2int KEY_UNKNOWN = 15 `shiftL` 18 |
There was a problem hiding this comment.
If you look carefully, you see that both KEY_MASK and KEY_UNKNOWN are 15 << 18. This is true in upstream too: https://github.com/libssh2/libssh2/blob/de7a74aff24c47b2f2e9815f0a98598195d602e4/include/libssh2.h#L1023
Hjdskes
force-pushed
the
fix_known_hosts
branch
from
578b55a to
867e119
Compare
Hjdskes
force-pushed
the
fix_known_hosts
branch
2 times, most recently
from
e422310 to
81df29b
Compare
|
One point that I do not like is the use of Base64 for internal representation. Why is it needed? If one wants, he can convert to Base64 before writing to file / stdout... |
|
One reason is that treating the key like a base64-encoded string allows us to break the existing API less. Another is that, in my experience, keys are most often used base64-encoded. I will remove it, at the cost of breaking the API of |
The C function libssh2_session_hostkey returns a const char* where the first byte is (often) a NULL byte. This causes the Haskell FFI to return an empty String. Hence, we create a new FFI to libssh2_session_hostkey that returns a Ptr CChar, that we then wrap in a function that returns a base64 encoded String. This way we can capture the host key, including its NULL byte, in a proper Haskell type. Although this is a bug fix, this changes Haskell type signatures of exported functions. See portnov#66.
The user needs to be able to specify the format of the hostname, key and key type. Although this is a bug fix, this changes Haskell type signatures of exported functions. See portnov#66.
Hjdskes
force-pushed
the
fix_known_hosts
branch
from
81df29b to
0f96157
Compare
| { toPointer `Session', alloca- `Size' peek*, alloca- `CInt' peek* } -> `Ptr CChar' id #} | ||
|
|
||
| -- | Get remote host public key and its type | ||
| getHostKey :: Session -> IO (BSS.ByteString, HostKeyType) |
There was a problem hiding this comment.
This is the second breaking change.
|
Hello! Any updates here? Strict host key checking would be ideal. Anything I can help with? |
|
I hope I will be able to address this topic this weekend. |
|
I can also help test if needed, feel free to ping me :) |
|
I remember wanting to amend something here, but I couldn't recall what exactly :) |
|
Any chance we could get a Hackage release for this? 😄 |
|
Uploaded release 0.2.0.9. |
|
Thank you 🙌🏻 |
|
Oh, hmm. This is a breaking change and should have been released as |
See #66 and #67. This also consolidates #68. Comments inline.