Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reimplement bindings for session_hostkey and knownhost_checkp #67

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

portnov
Copy link
Owner

@portnov portnov commented Feb 9, 2022

refs #66

Although this is a bug fix, this changes Haskell type signatures of
exported functions.

refs #66

Although this is a bug fix, this changes Haskell type signatures of
exported functions.
@Hjdskes
Copy link
Contributor

Hjdskes commented Feb 9, 2022

This will do! I chose a slightly different approach that I was going to upstream after some more testing. Adapting that to fit your code, I'd end up at this:

{# fun libssh2_session_hostkey as getHostKey_
  { toPointer `Session', alloca- `CULong' peek*, alloca- `CInt' peek* } -> `Ptr CChar' id #}

-- | An SSH public host key.
data HostKey = HostKey
  { base64Key :: Text
    -- ^ The base64-encoded host key.
  , keyType :: KnownHostType
    -- ^ The host key type.
  }
  deriving (Eq)

-- | Get the public host key.
getHostKey :: Session -> IO HostKey
getHostKey session = do
  (cBytes, cBytesLen, intKeyType) <- getHostKey_ session
  -- TODO: is fromEnum safe to convert a CULong to an Int? Seems to work for our uses.
  keyBytes <- encodeBase64 <$> BSS.packCStringLen (cBytes, fromEnum cBytesLen)
  pure (HostKey keyBytes (int2kht intKeyType))

As you can see, it's pretty much equivalent. I also have some Show instances for these that print the host key in the SSH known_hosts format (save for the host name):

instance Show HostKey where
  show (HostKey base64Key keyType) = show keyType ++ " " ++ T.unpack base64Key

instance Show KnownHostType where
  show KEY_SSHRSA = "ssh-rsa"
  show KEY_SSHDSS = "ssh-dss"
  ...

I noticed libssh2-hs does not support other host key ciphers such as ED25519. Should I open a PR for that? I need it for my project :)

(hostkey, _keylen, _keytype) <- getHostKey s
result <- checkKnownHost kh host port hostkey [TYPE_PLAIN, KEYENC_RAW]
(hostkey, _keytype) <- getHostKey s
result <- checkKnownHost kh host port hostkey [TYPE_PLAIN, KEYENC_RAW, KEY_RSA1, KEY_SSHRSA, KEY_SSHDSS]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we also need KEYENC_BASE64, since the keys in the known hosts file are base64 encoded.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I needed this in my implementation at least.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should allow the user of this function to pass the flags, and add it as a parameter to the function.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought maybe it would be better to do

(hostkey, keytype) <- getHostKey s
result <- checkKnownHost kh host port hostkey [keytype]

what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is insufficient; the typemask details not just what key type is it, but also what format the host name is and what format the key is in. See
https://www.libssh2.org/libssh2_knownhost_check.html

For example, we'd pass [TYPE_PLAIN, KEYENC_BASE64, KEY_SSHRSA] to specify that the host name is plain (not hashed), that the key is base64 encoded and that the key is an ssh-rsa key.

@Hjdskes
Copy link
Contributor

Hjdskes commented Feb 10, 2022

Looking at https://github.com/libssh2/libssh2/blob/de7a74aff24c47b2f2e9815f0a98598195d602e4/include/libssh2.h, I think you are targeting the wrong values for KnownHostType. Or rather, those values are not the ones used for libssh2_session_hostkey; those value are found here (compared to here where you're currently fetching the values from).

Feel free to copy this which I have in my local branch:

data HostKeyType =
    UNKNOWN
  | RSA
  | DSS
  | ECDSA_256
  | ECDSA_384
  | ECDSA_521
  | ED25519
  deriving (Enum, Eq, Ord)

instance Show HostKeyType where
  show UNKNOWN = "unknown"
  show RSA = "ssh-rsa"
  show DSS = "ssh-dss"
  show ECDSA_256 = "ecdsa-sha2-nistp256"
  show ECDSA_384 = "ecdsa-sha2-nistp384"
  show ECDSA_521 = "ecdsa-sha2-nistp521"
  show ED25519 = "ssh-ed25519"

int2hostKeyType :: Integral n => n -> HostKeyType
int2hostKeyType = toEnum . fromIntegral

@Hjdskes
Copy link
Contributor

Hjdskes commented Feb 15, 2022

Hey @portnov, is there anything I can do to help get this merged? :)

@portnov
Copy link
Owner Author

portnov commented Feb 15, 2022

You can create your own PR, based on this, with changes that you suggested. Unfortunately I did not have time to make them.

@Hjdskes Hjdskes mentioned this pull request Feb 15, 2022
@Hjdskes
Copy link
Contributor

Hjdskes commented Feb 15, 2022

Thanks! I opened #69.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants