Re: [PATCH] ssh signing: support non ssh-* keytypes

[Junio C Hamano <gitster@pobox.com>]

Fabian Stelzer <fs@gigacodes.de> writes:

> +/* Determines wether key contains a literal ssh key or a path to a file */
> +static int is_literal_ssh_key(const char *key) {
> +	return (
> +		starts_with(key, "ssh-") ||
> +		starts_with(key, "ecdsa-") ||
> +		starts_with(key, "sk-ssh-") ||
> +		starts_with(key, "sk-ecdsa-")
> +	);
> +}

A more forward looking thing you could do is to 

 (1) grandfather the convention "any string that begins with 'ssh-'
     is taken as a ssh literal key".

 (2) refrain from spreading such an unstructured mess by picking a
     reserved prefix, say "ssh-key::" and have all other kinds of
     ssh keys use the convention.

making the above function look more like

    static int is_literal_ssh_key(const char *string, const char **key)
    {
	if (skip_prefix(string, "ssh-key::", key)
	    return 1;
	if (starts_with(string, "ssh-")) {
	    key = string;
	    return 1;
	}
	return 0;
    }

so that the caller can extract the literal key from the string that
specifies either the literal key or path to the file.  This will
futureproof us in two axis.  When SSH adds types of keys using new
algo, we do not have to add it to is_literal_ssh_key() function.
Also when another crypto suite other than GPG and SSH comes, we
won't repeat the "bare 'ssh-' prefix is reserved by ssh, and
different kind in the same suite may have to consume more reserved
prefixes" mistake---it would make it more natural for us to pick
"literal keys from any variant of that new FOO crypto suite are
written with 'foo-key::' prefix" if we did so right now.  It would
have been better if we didn't have to do the grandfathering, but I
am assuming that the ship has already sailed?

> @@ -719,7 +729,7 @@ static char *get_ssh_key_fingerprint(const char *signing_key)
>  	 * With SSH Signing this can contain a filename or a public key
>  	 * For textual representation we usually want a fingerprint
>  	 */
> -	if (starts_with(signing_key, "ssh-")) {
> +	if (is_literal_ssh_key(signing_key)) {
> 		strvec_pushl(&ssh_keygen.args, "ssh-keygen", "-lf", "-", NULL);
> 		ret = pipe_command(&ssh_keygen, signing_key,
> 				   strlen(signing_key), &fingerprint_stdout, 0,

This part needs a bit of adjustment if we go the
"is_literal_ssh_key() is not just a boolean but is used to strip the
prefix to signal the kind of key" route, but the necessary
adjustment should be trivial.