Make git_credtype_t a bitfield

[tiennou]

This gives git_credtype_t correct values for or-ing.

Right now client code would get GIT_CREDTYPE_SSH_PUBLICKEY (3) for GIT_CREDTYPE_USERPASS_PLAINTEXT|GIT_CREDTYPE_SSH_KEYFILE_PASSPHRASE (1|2), which would throw them into trying to understand how git_cred_ssh_publickey_new is to be used ;-).

Fixes #1852.

[carlosmn]

Why add _SIGN?

We should probably just remove PUBLICKEY, as it's not documented in libssh2, it's not a realistic usage for anybody and solves the problem of or-able values.

[tiennou]

At first I was like "let's remove it, it's undocumented", and then I was like "let's keep it, it can be useful" ;-).

I'm adding _SIGN because I wanted that to be clearer that it's not a plain "I give you a public key, you let me it" call : this one allows the user of the library to perform it's own signature step (that's what the callback is expected to do), without relying on actual files. For example, I wouldn't recommend using GIT_CREDTYPE_SSH_KEYFILE_PASSPHRASE but this one on iOS, because you would store the user's certificate/keys/crypto-stuff in Keychain, and wouldn't have to write those to temporary files so you can use GIT_CREDTYPE_SSH_KEYFILE_PASSPHRASE and the paths it takes.

So (for me at least), that's a realistic use case, and if there's interest I can try and make documentation clearer on what's expected (as well as an example, if necessary, but that'll take me near Keychain.framework which isn't the nicest API to use...)

[carlosmn]

If you're going to use the name to describe what it does rather than the name of the libssh2 functionality, then go all the way, but it's going to stay libssh2-speicific, so I don't quite see the advanteage of naming it something slightly different from its libssh2 name.

But the name of this value isn't so important because it doesn't describe a value that a user would see. Which way the signature happens are not different supported authentication mechanisms, so it's only going to be kept inside the git_cred struct, which I don't think we should even be exporting, but that's a different issue.

I don't see each user doing their own cryptographic signature function as a realistic use-case. Does that Keychain.framework provide you with such a function that you can use on the data? Is there anything on iOS that actually does git stuff? All I hear are people who seem excited that this would even be possible, but no-one seems to actually use it.

[tiennou]

I'm speaking of "realistic use case" here, but I don't actually need it right now (the other method taking key files is sufficient for what I'm doing — unrealistic tests of credential functionality).

But the name of this value isn't so important because it doesn't describe a value that a user would see. Which way the signature happens are not different supported authentication mechanisms, so it's only going to be kept inside the git_cred struct, which I don't think we should even be exporting, but that's a different issue.

I'm not sure what user refers to, but if you're referring to libgit2 users (us developers, in opposition with "end-users") this is passed in git_cred_acquire_cb as allowed_types. Are you talking about git_cred being "mostly" public ? The only access points I can see from the outside are the various git_cred*_new functions...

Does that Keychain.framework provide you with such a function that you can use on the data?

SecKeyRawSign, here

Is there anything on iOS that actually does git stuff?

Sorry, I don't get it... The handful of clients written by developers that use libgit2/Òbjective-Git` for their functionality ?

Your call on tearing down GIT_CREDTYPE_SSH_PUBLICKEY completely, because as you say it's not documented — though not that hard to learn what it does by Reading the Source (Luke). About the git_credtype_t issue I do think the values should read as I changed them (even if the clash with _SSH_PUBLICKEY goes away with it).

[isaac]

@carlosmn @tiennou - I am developing an iOS app on top of libgit2/ObjectiveGit that needs to authenticate with SSH. I was thinking about using GIT_CREDTYPE_SSH_KEYFILE_PASSPHRASE because I don't really know how GIT_CREDTYPE_SSH_PUBLICKEY works.

@tiennou - do you think writing the keys from the Keychain to temporary files would be a bad idea from a security point of view?

[tiennou]

(Sorry, accidentally clicked commit while trying to dismiss the @ menu).

@isaac you can take a look at libgit2/objective-git#254, specifically this commit. I haven't been able to test it because I'm on OS X and SSH handles my keys (so I'm using GIT_CREDTYPE_SSH_KEYFILE_PASSPHRASE), so if you have access to an iOS device and are willing to test that would be appreciated. Sadly, you're pretty on your own on the Keychain part, mostly because I have more experience with OS X keychain (which is less "limited"). The only thing I can tell you about is the SecKeyRawSign above which eerily reminded me of the libssh2 function, but I'm not sure of it's availability (EDIT: It's not).

About the security issue, doing what you propose is the only way to authenticate SSH using GIT_CREDTYPE_SSH_KEYFILE_PASSPHRASE. And the temporary write file should do the trick provided :

• the key is still passworded if there was a password entered by the user (note that keys are not always passworded).
• the key exported from Keychain still suits libssh2.

EDIT: Looks like iOS keychain API is pretty much toned down. So you'll have to cheat and store the key in a password, so you don't really care about saving unpassworded keys, you don't have any other way. HTH.

[isaac]

@tiennou - thanks! I should have some time to test it out this weekend.

[tiennou]

Force-pushed to remove the _SIGN_ change. @carlosmn Opinions ?

[carlosmn]

/** */ to make a documentation comment.

[ethomson]

Closed via #1903.