Windows Pageant: display RSA/DSA cert bit counts.

The test in the Pageant list box code for whether we should display
the bit count of a key was done by checking specifically for ssh_rsa
or ssh_dsa, which of course meant that it didn't catch the certified
versions of those keys.

Now there's yet another footling ssh_keyalg method that asks the
question 'is it worth displaying the bit count?', to which RSA and DSA
answer yes, and the opensshcert family delegates to its base key type,
so that RSA and DSA certified keys also answer yes.

(This isn't the same as ssh_key_public_bits(alg, blob) >= 0. All
supported public key algorithms _can_ display a bit count if called
on. But only in RSA and DSA is it configurable, and therefore worth
bothering to print in the list box.)

Also in this commit, I've fixed a bug in the certificate
implementation of public_bits, which was passing a wrongly formatted
public blob to the underlying key. (Done by factoring out the code
from opensshcert_new_shared which constructed the _correct_ public
blob, and reusing it in public_bits to do the same job.)

crypto/dsa.c

@@ -511,6 +511,7 @@ const ssh_keyalg ssh_dsa = {
     .supported_flags = nullkey_supported_flags,
     .alternate_ssh_id = nullkey_alternate_ssh_id,
     .alg_desc = dsa_alg_desc,
+    .variable_size = nullkey_variable_size_yes,
     .ssh_id = "ssh-dss",
     .cache_id = "dss",
 };

crypto/ecc-ssh.c

@@ -1284,6 +1284,7 @@ const ssh_keyalg ssh_ecdsa_ed25519 = {
     .supported_flags = nullkey_supported_flags,
     .alternate_ssh_id = nullkey_alternate_ssh_id,
     .alg_desc = ec_alg_desc,
+    .variable_size = nullkey_variable_size_no,
     .ssh_id = "ssh-ed25519",
     .cache_id = "ssh-ed25519",
     .extra = &sign_extra_ed25519,
@@ -1312,6 +1313,7 @@ const ssh_keyalg ssh_ecdsa_ed448 = {
     .supported_flags = nullkey_supported_flags,
     .alternate_ssh_id = nullkey_alternate_ssh_id,
     .alg_desc = ec_alg_desc,
+    .variable_size = nullkey_variable_size_no,
     .ssh_id = "ssh-ed448",
     .cache_id = "ssh-ed448",
     .extra = &sign_extra_ed448,
@@ -1344,6 +1346,7 @@ const ssh_keyalg ssh_ecdsa_nistp256 = {
     .supported_flags = nullkey_supported_flags,
     .alternate_ssh_id = nullkey_alternate_ssh_id,
     .alg_desc = ec_alg_desc,
+    .variable_size = nullkey_variable_size_no,
     .ssh_id = "ecdsa-sha2-nistp256",
     .cache_id = "ecdsa-sha2-nistp256",
     .extra = &sign_extra_nistp256,
@@ -1376,6 +1379,7 @@ const ssh_keyalg ssh_ecdsa_nistp384 = {
     .supported_flags = nullkey_supported_flags,
     .alternate_ssh_id = nullkey_alternate_ssh_id,
     .alg_desc = ec_alg_desc,
+    .variable_size = nullkey_variable_size_no,
     .ssh_id = "ecdsa-sha2-nistp384",
     .cache_id = "ecdsa-sha2-nistp384",
     .extra = &sign_extra_nistp384,
@@ -1408,6 +1412,7 @@ const ssh_keyalg ssh_ecdsa_nistp521 = {
     .supported_flags = nullkey_supported_flags,
     .alternate_ssh_id = nullkey_alternate_ssh_id,
     .alg_desc = ec_alg_desc,
+    .variable_size = nullkey_variable_size_no,
     .ssh_id = "ecdsa-sha2-nistp521",
     .cache_id = "ecdsa-sha2-nistp521",
     .extra = &sign_extra_nistp521,

crypto/openssh-certs.c

@@ -217,6 +217,7 @@ static unsigned opensshcert_supported_flags(const ssh_keyalg *self);
 static const char *opensshcert_alternate_ssh_id(const ssh_keyalg *self,
                                                 unsigned flags);
 static char *opensshcert_alg_desc(const ssh_keyalg *self);
+static bool opensshcert_variable_size(const ssh_keyalg *self);
 static const ssh_keyalg *opensshcert_related_alg(const ssh_keyalg *self,
                                                  const ssh_keyalg *base);
 
@@ -271,6 +272,7 @@ static const ssh_keyalg *opensshcert_related_alg(const ssh_keyalg *self,
         .supported_flags = opensshcert_supported_flags,                 \
         .alternate_ssh_id = opensshcert_alternate_ssh_id,               \
         .alg_desc = opensshcert_alg_desc,                               \
+        .variable_size = opensshcert_variable_size,                     \
         .related_alg = opensshcert_related_alg,                         \
         .ssh_id = ssh_alg_id_prefix "-cert-v01@openssh.com",            \
         .cache_id = "opensshcert-" ssh_key_id_prefix,                   \
@@ -287,6 +289,26 @@ static const ssh_keyalg *const opensshcert_all_keyalgs[] = {
 };
 #undef KEYALG_LIST_ENTRY
 
+static strbuf *get_base_public_blob(BinarySource *src,
+                                    const opensshcert_extra *extra)
+{
+    strbuf *basepub = strbuf_new();
+    put_stringz(basepub, extra->base_key_ssh_id);
+
+    /* Make the base public key blob out of the public key
+     * material in the certificate. This invocation of the
+     * blobtrans system doesn't do any format translation, but it
+     * does ensure that the right amount of data is copied so that
+     * src ends up in the right position to read the remaining
+     * certificate fields. */
+    BLOBTRANS_DECLARE(bt);
+    blobtrans_read(bt, src, extra->pub_fmt);
+    blobtrans_write(bt, BinarySink_UPCAST(basepub), extra->pub_fmt);
+    blobtrans_clear(bt);
+
+    return basepub;
+}
+
 static opensshcert_key *opensshcert_new_shared(
     const ssh_keyalg *self, ptrlen blob, strbuf **basepub_out)
 {
@@ -304,21 +326,7 @@ static opensshcert_key *opensshcert_new_shared(
     ck->sshk.vt = self;
 
     ck->nonce = strbuf_dup(get_string(src));
-    strbuf *basepub = strbuf_new();
-    {
-        put_stringz(basepub, extra->base_key_ssh_id);
-
-        /* Make the base public key blob out of the public key
-         * material in the certificate. This invocation of the
-         * blobtrans system doesn't do any format translation, but it
-         * does ensure that the right amount of data is copied so that
-         * src ends up in the right position to read the remaining
-         * certificate fields. */
-        BLOBTRANS_DECLARE(bt);
-        blobtrans_read(bt, src, extra->pub_fmt);
-        blobtrans_write(bt, BinarySink_UPCAST(basepub), extra->pub_fmt);
-        blobtrans_clear(bt);
-    }
+    strbuf *basepub = get_base_public_blob(src, extra);
     ck->serial = get_uint64(src);
     ck->type = get_uint32(src);
     ck->key_id = strbuf_dup(get_string(src));
@@ -877,8 +885,11 @@ static int opensshcert_pubkey_bits(const ssh_keyalg *self, ptrlen blob)
 
     get_string(src);                   /* key type */
     get_string(src);                   /* nonce */
-    return ssh_key_public_bits(
-        self->base_alg, make_ptrlen(get_ptr(src), get_avail(src)));
+    strbuf *basepub = get_base_public_blob(src, self->extra);
+    int bits = ssh_key_public_bits(
+        self->base_alg, ptrlen_from_strbuf(basepub));
+    strbuf_free(basepub);
+    return bits;
 }
 
 static unsigned opensshcert_supported_flags(const ssh_keyalg *self)
@@ -908,6 +919,11 @@ static char *opensshcert_alg_desc(const ssh_keyalg *self)
     return our_desc;
 }
 
+static bool opensshcert_variable_size(const ssh_keyalg *self)
+{
+    return ssh_keyalg_variable_size(self->base_alg);
+}
+
 static const ssh_keyalg *opensshcert_related_alg(const ssh_keyalg *self,
                                                  const ssh_keyalg *base)
 {

crypto/rsa.c

@@ -883,6 +883,7 @@ static const struct ssh2_rsa_extra
     .base_key = nullkey_base_key,               \
     .pubkey_bits = rsa2_pubkey_bits,            \
     .alg_desc = rsa2_alg_desc,                  \
+    .variable_size = nullkey_variable_size_yes, \
     .cache_id = "rsa2"
 
 const ssh_keyalg ssh_rsa = {

ssh.h

@@ -858,6 +858,7 @@ struct ssh_keyalg {
     unsigned (*supported_flags) (const ssh_keyalg *self);
     const char *(*alternate_ssh_id) (const ssh_keyalg *self, unsigned flags);
     char *(*alg_desc)(const ssh_keyalg *self);
+    bool (*variable_size)(const ssh_keyalg *self);
     /* The following methods can be NULL if !is_certificate */
     const ssh_keyalg *(*related_alg)(const ssh_keyalg *self,
                                      const ssh_keyalg *base);
@@ -928,6 +929,8 @@ static inline const char *ssh_keyalg_alternate_ssh_id(
 { return self->alternate_ssh_id(self, flags); }
 static inline char *ssh_keyalg_desc(const ssh_keyalg *self)
 { return self->alg_desc(self); }
+static inline bool ssh_keyalg_variable_size(const ssh_keyalg *self)
+{ return self->variable_size(self); }
 static inline const ssh_keyalg *ssh_keyalg_related_alg(
     const ssh_keyalg *self, const ssh_keyalg *base)
 { return self->related_alg(self, base); }
@@ -936,6 +939,8 @@ static inline const ssh_keyalg *ssh_keyalg_related_alg(
 unsigned nullkey_supported_flags(const ssh_keyalg *self);
 const char *nullkey_alternate_ssh_id(const ssh_keyalg *self, unsigned flags);
 ssh_key *nullkey_base_key(ssh_key *key);
+bool nullkey_variable_size_no(const ssh_keyalg *self);
+bool nullkey_variable_size_yes(const ssh_keyalg *self);
 
 /* Utility functions implemented centrally */
 ssh_key *ssh_key_clone(ssh_key *key);

utils/nullkey.c

@@ -17,3 +17,6 @@ ssh_key *nullkey_base_key(ssh_key *key)
     /* When a key is not certified, it is its own base */
     return key;
 }
+
+bool nullkey_variable_size_no(const ssh_keyalg *self) { return false; }
+bool nullkey_variable_size_yes(const ssh_keyalg *self) { return true; }

windows/pageant.c

@@ -372,18 +372,11 @@ static void keylist_update_callback(
             put_datapl(disp->alg, keytype_word);
         }
 
-        put_datapl(disp->bits, ptrlen_get_word(&fingerprint, " "));
-        put_datapl(disp->hash, ptrlen_get_word(&fingerprint, " "));
+        ptrlen bits_word = ptrlen_get_word(&fingerprint, " ");
+        if (ssh_keyalg_variable_size(alg))
+            put_datapl(disp->bits, bits_word);
 
-        /*
-         * But we don't display the bit count if the algorithm isn't
-         * one of the ones where it can vary. That way, those
-         * algorithm names (which are generally longer) can safely
-         * overlap into the bits column without colliding with
-         * pointless text.
-         */
-        if (!(alg == &ssh_dsa || alg == &ssh_rsa))
-            strbuf_clear(disp->bits);
+        put_datapl(disp->hash, ptrlen_get_word(&fingerprint, " "));
       }
     }