sign: Support keys embedded with public key algorithm

The current commit signing mechanism assumes raw Ed25519 key format
for both public and private keys. That requires custom processing of
keys after generated with openssl tools, and also lacks cryptographic
agility[1]; when Ed25519 becomes vulnerable, it would not be
straightforward to migrate to other algorithms.

This patch switches to using the standard key formats natively
supported by OpenSSL (PKCS#8 and SubjectPublicKeyInfo) and capable of
embedding algorithm identifier, while the support for the original key
format is preserved for backward compatibility.

As a PoC of the feature, this adds a couple of new tests using Ed448,
instead of Ed25519, in tests/test-signed-commit.sh.

1. https://en.wikipedia.org/wiki/Cryptographic_agility

Signed-off-by: Daiki Ueno <[email protected]>

configure.ac

@@ -457,6 +457,10 @@ if test x$with_openssl != xno || test x$with_ed25519_libsodium != xno; then
    OSTREE_FEATURES="$OSTREE_FEATURES sign-ed25519"
 fi
 
+if test x$with_openssl != xno; then
+   OSTREE_FEATURES="$OSTREE_FEATURES sign-pkcs8"
+fi
+
 dnl begin gnutls; in contrast to openssl this one only
 dnl supports --with-crypto=gnutls
 GNUTLS_DEPENDENCY="gnutls >= 3.5.0"

src/libostree/ostree-sign-ed25519.c

@@ -48,9 +48,9 @@ struct _OstreeSignEd25519
 {
   GObject parent;
   ed25519_state state;
-  guchar *secret_key;  /* malloc'd buffer of length OSTREE_SIGN_ED25519_SECKEY_SIZE */
-  GList *public_keys;  /* malloc'd buffer of length OSTREE_SIGN_ED25519_PUBKEY_SIZE */
-  GList *revoked_keys; /* malloc'd buffer of length OSTREE_SIGN_ED25519_PUBKEY_SIZE */
+  GBytes *secret_key;
+  GList *public_keys;  /* GBytes */
+  GList *revoked_keys; /* GBytes */
 };
 
 static void ostree_sign_ed25519_iface_init (OstreeSignInterface *self);
@@ -96,15 +96,19 @@ _ostree_sign_ed25519_init (OstreeSignEd25519 *self)
 #endif
 }
 
+#if defined(USE_LIBSODIUM)
+// Strictly verify pubkey and signature lengths, as libsodium can
+// only handle raw ed25519 public key and signatures.
 static gboolean
 validate_length (gsize found, gsize expected, GError **error)
 {
   if (found == expected)
     return TRUE;
   return glnx_throw (
       error, "Ill-formed input: expected %" G_GSIZE_FORMAT " bytes, got %" G_GSIZE_FORMAT " bytes",
-      found, expected);
+      expected, found);
 }
+#endif
 
 static gboolean
 _ostree_sign_ed25519_is_initialized (OstreeSignEd25519 *self, GError **error)
@@ -136,27 +140,43 @@ ostree_sign_ed25519_data (OstreeSign *self, GBytes *data, GBytes **signature,
   if (sign->secret_key == NULL)
     return glnx_throw (error, "Not able to sign: secret key is not set");
 
+#if defined(USE_LIBSODIUM) || defined(USE_OPENSSL)
+  gsize secret_key_size;
+  const guint8 *secret_key_buf = g_bytes_get_data (sign->secret_key, &secret_key_size);
+#endif
+
   unsigned long long sig_size = 0;
-  g_autofree guchar *sig = g_malloc0 (OSTREE_SIGN_ED25519_SIG_SIZE);
+  g_autofree guchar *sig = NULL;
 
 #if defined(USE_LIBSODIUM)
+  sig = g_malloc0 (OSTREE_SIGN_ED25519_SIG_SIZE);
   if (crypto_sign_detached (sig, &sig_size, g_bytes_get_data (data, NULL), g_bytes_get_size (data),
-                            sign->secret_key))
+                            secret_key_buf))
     sig_size = 0;
 #elif defined(USE_OPENSSL)
   EVP_MD_CTX *ctx = EVP_MD_CTX_new ();
   if (!ctx)
     return glnx_throw (error, "openssl: failed to allocate context");
-  EVP_PKEY *pkey = EVP_PKEY_new_raw_private_key (EVP_PKEY_ED25519, NULL, sign->secret_key,
-                                                 OSTREE_SIGN_ED25519_SEED_SIZE);
+
+  // Try PKCS8 encoded private key first.
+  const unsigned char *p = secret_key_buf;
+  EVP_PKEY *pkey = d2i_AutoPrivateKey (NULL, &p, secret_key_size);
+
+  // Try raw ed25519 private key if the length matches.
+  if (pkey == NULL && secret_key_size == OSTREE_SIGN_ED25519_SECKEY_SIZE)
+    pkey = EVP_PKEY_new_raw_private_key (EVP_PKEY_ED25519, NULL, secret_key_buf,
+                                         OSTREE_SIGN_ED25519_SEED_SIZE);
+
   if (!pkey)
     {
       EVP_MD_CTX_free (ctx);
-      return glnx_throw (error, "openssl: Failed to initialize ed5519 key");
+      return glnx_throw (error, "openssl: Failed to initialize ed25519 key");
     }
 
   size_t len;
   if (EVP_DigestSignInit (ctx, NULL, NULL, NULL, pkey)
+      && EVP_DigestSign (ctx, NULL, &len, g_bytes_get_data (data, NULL), g_bytes_get_size (data))
+      && (sig = g_malloc0 (len)) != NULL
       && EVP_DigestSign (ctx, sig, &len, g_bytes_get_data (data, NULL), g_bytes_get_size (data)))
     sig_size = len;
 
@@ -172,12 +192,6 @@ ostree_sign_ed25519_data (OstreeSign *self, GBytes *data, GBytes **signature,
   return TRUE;
 }
 
-static gint
-_compare_ed25519_keys (gconstpointer a, gconstpointer b)
-{
-  return memcmp (a, b, OSTREE_SIGN_ED25519_PUBKEY_SIZE);
-}
-
 gboolean
 ostree_sign_ed25519_data_verify (OstreeSign *self, GBytes *data, GVariant *signatures,
                                  char **out_success_message, GError **error)
@@ -222,29 +236,27 @@ ostree_sign_ed25519_data_verify (OstreeSign *self, GBytes *data, GVariant *signa
       g_autoptr (GVariant) child = g_variant_get_child_value (signatures, i);
       g_autoptr (GBytes) signature = g_variant_get_data_as_bytes (child);
 
+#if defined(USE_LIBSODIUM)
       if (!validate_length (g_bytes_get_size (signature), OSTREE_SIGN_ED25519_SIG_SIZE, error))
         return glnx_prefix_error (error, "Invalid signature");
-
-      g_autofree char *hex = g_malloc0 (OSTREE_SIGN_ED25519_PUBKEY_SIZE * 2 + 1);
+#endif
 
       g_debug ("Read signature %d: %s", (gint)i, g_variant_print (child, TRUE));
 
-      for (GList *public_key = sign->public_keys; public_key != NULL; public_key = public_key->next)
+      for (GList *l = sign->public_keys; l != NULL; l = l->next)
         {
+          GBytes *public_key = l->data;
           /* TODO: use non-list for tons of revoked keys? */
-          if (g_list_find_custom (sign->revoked_keys, public_key->data, _compare_ed25519_keys)
-              != NULL)
+          if (g_list_find_custom (sign->revoked_keys, public_key, g_bytes_compare) != NULL)
             {
-              ot_bin2hex (hex, public_key->data, OSTREE_SIGN_ED25519_PUBKEY_SIZE);
+              g_autofree char *hex = g_malloc0 (g_bytes_get_size (public_key) * 2 + 1);
+              ot_bin2hex (hex, g_bytes_get_data (public_key, NULL), g_bytes_get_size (public_key));
               g_debug ("Skip revoked key '%s'", hex);
               continue;
             }
 
           bool valid = false;
-          // Wrap the pubkey in a GBytes as that's what this API wants
-          g_autoptr (GBytes) public_key_bytes
-              = g_bytes_new_static (public_key->data, OSTREE_SIGN_ED25519_PUBKEY_SIZE);
-          if (!otcore_validate_ed25519_signature (data, public_key_bytes, signature, &valid, error))
+          if (!otcore_validate_ed25519_signature (data, public_key, signature, &valid, error))
             return FALSE;
           if (!valid)
             {
@@ -254,14 +266,17 @@ ostree_sign_ed25519_data_verify (OstreeSign *self, GBytes *data, GVariant *signa
               else
                 g_string_append (invalid_signatures, "; ");
               n_invalid_signatures++;
-              ot_bin2hex (hex, public_key->data, OSTREE_SIGN_ED25519_PUBKEY_SIZE);
+              g_autofree char *hex = g_malloc0 (g_bytes_get_size (public_key) * 2 + 1);
+              ot_bin2hex (hex, g_bytes_get_data (public_key, NULL), g_bytes_get_size (public_key));
               g_string_append_printf (invalid_signatures, "key '%s'", hex);
             }
           else
             {
               if (out_success_message)
                 {
-                  ot_bin2hex (hex, public_key->data, OSTREE_SIGN_ED25519_PUBKEY_SIZE);
+                  g_autofree char *hex = g_malloc0 (g_bytes_get_size (public_key) * 2 + 1);
+                  ot_bin2hex (hex, g_bytes_get_data (public_key, NULL),
+                              g_bytes_get_size (public_key));
                   *out_success_message = g_strdup_printf (
                       "ed25519: Signature verified successfully with key '%s'", hex);
                 }
@@ -320,22 +335,23 @@ ostree_sign_ed25519_clear_keys (OstreeSign *self, GError **error)
   /* Clear secret key */
   if (sign->secret_key != NULL)
     {
-      memset (sign->secret_key, 0, OSTREE_SIGN_ED25519_SECKEY_SIZE);
-      g_free (sign->secret_key);
+      gsize size;
+      gpointer data = g_bytes_unref_to_data (sign->secret_key, &size);
+      memset (data, 0, size);
       sign->secret_key = NULL;
     }
 
   /* Clear already loaded trusted keys */
   if (sign->public_keys != NULL)
     {
-      g_list_free_full (sign->public_keys, g_free);
+      g_list_free_full (sign->public_keys, (GDestroyNotify)g_bytes_unref);
       sign->public_keys = NULL;
     }
 
   /* Clear already loaded revoked keys */
   if (sign->revoked_keys != NULL)
     {
-      g_list_free_full (sign->revoked_keys, g_free);
+      g_list_free_full (sign->revoked_keys, (GDestroyNotify)g_bytes_unref);
       sign->revoked_keys = NULL;
     }
 
@@ -374,10 +390,12 @@ ostree_sign_ed25519_set_sk (OstreeSign *self, GVariant *secret_key, GError **err
       return glnx_throw (error, "Unknown ed25519 secret key type");
     }
 
+#if defined(USE_LIBSODIUM)
   if (!validate_length (n_elements, OSTREE_SIGN_ED25519_SECKEY_SIZE, error))
     return glnx_prefix_error (error, "Invalid ed25519 secret key");
+#endif
 
-  sign->secret_key = g_steal_pointer (&secret_key_buf);
+  sign->secret_key = g_bytes_new_take (g_steal_pointer (&secret_key_buf), n_elements);
 
   return TRUE;
 }
@@ -429,17 +447,20 @@ ostree_sign_ed25519_add_pk (OstreeSign *self, GVariant *public_key, GError **err
       return glnx_throw (error, "Unknown ed25519 public key type");
     }
 
+#if defined(USE_LIBSODIUM)
   if (!validate_length (n_elements, OSTREE_SIGN_ED25519_PUBKEY_SIZE, error))
     return glnx_prefix_error (error, "Invalid ed25519 public key");
+#endif
 
-  g_autofree char *hex = g_malloc0 (OSTREE_SIGN_ED25519_PUBKEY_SIZE * 2 + 1);
+  g_autofree char *hex = g_malloc0 (n_elements * 2 + 1);
   ot_bin2hex (hex, key, n_elements);
   g_debug ("Read ed25519 public key = %s", hex);
 
-  if (g_list_find_custom (sign->public_keys, key, _compare_ed25519_keys) == NULL)
+  g_autoptr (GBytes) key_bytes = g_bytes_new_static (key, n_elements);
+  if (g_list_find_custom (sign->public_keys, key_bytes, g_bytes_compare) == NULL)
     {
-      gpointer newkey = g_memdup2 (key, n_elements);
-      sign->public_keys = g_list_prepend (sign->public_keys, newkey);
+      GBytes *new_key_bytes = g_bytes_new (key, n_elements);
+      sign->public_keys = g_list_prepend (sign->public_keys, new_key_bytes);
     }
 
   return TRUE;
@@ -460,17 +481,20 @@ _ed25519_add_revoked (OstreeSign *self, GVariant *revoked_key, GError **error)
   gsize n_elements = 0;
   g_autofree guint8 *key = g_base64_decode (rk_ascii, &n_elements);
 
+#if defined(USE_LIBSODIUM)
   if (!validate_length (n_elements, OSTREE_SIGN_ED25519_PUBKEY_SIZE, error))
     return glnx_prefix_error (error, "Incorrect ed25519 revoked key");
+#endif
 
-  g_autofree char *hex = g_malloc0 (OSTREE_SIGN_ED25519_PUBKEY_SIZE * 2 + 1);
+  g_autofree char *hex = g_malloc0 (n_elements * 2 + 1);
   ot_bin2hex (hex, key, n_elements);
   g_debug ("Read ed25519 revoked key = %s", hex);
 
-  if (g_list_find_custom (sign->revoked_keys, key, _compare_ed25519_keys) == NULL)
+  g_autoptr (GBytes) key_bytes = g_bytes_new_static (key, n_elements);
+  if (g_list_find_custom (sign->revoked_keys, key, g_bytes_compare) == NULL)
     {
-      gpointer newkey = g_memdup2 (key, n_elements);
-      sign->revoked_keys = g_list_prepend (sign->revoked_keys, newkey);
+      GBytes *new_key_bytes = g_bytes_new (key, n_elements);
+      sign->revoked_keys = g_list_prepend (sign->revoked_keys, new_key_bytes);
     }
 
   return TRUE;

src/libotcore/otcore-ed25519-verify.c

@@ -51,7 +51,12 @@ otcore_ed25519_init (void)
  * `out_valid` will be set to `false`.
  *
  * If the signature is correct, `out_valid` will be `true`.
- * */
+ *
+ * Note: when OpenSSL is enabled, public key is not restricted to ed25519 but
+ * something else if encoded in the X.509 SubjectPublicKeyInfo format. In that
+ * case, however, the hash algorithm is implicitly determined and thus
+ * unrestricted key types, e.g., raw RSA or ECDSA are not supported.
+ */
 gboolean
 otcore_validate_ed25519_signature (GBytes *data, GBytes *public_key, GBytes *signature,
                                    bool *out_valid, GError **error)
@@ -64,21 +69,24 @@ otcore_validate_ed25519_signature (GBytes *data, GBytes *public_key, GBytes *sig
   // It is OK for error to be NULL, though according to GError rules.
 
 #if defined(HAVE_LIBSODIUM) || defined(HAVE_OPENSSL)
-  // And strictly verify pubkey and signature lengths
-  if (g_bytes_get_size (public_key) != OSTREE_SIGN_ED25519_PUBKEY_SIZE)
-    return glnx_throw (error, "Invalid public key of %" G_GSIZE_FORMAT " expected %" G_GSIZE_FORMAT,
-                       (gsize)g_bytes_get_size (public_key),
-                       (gsize)OSTREE_SIGN_ED25519_PUBKEY_SIZE);
-  const guint8 *public_key_buf = g_bytes_get_data (public_key, NULL);
-  if (g_bytes_get_size (signature) != OSTREE_SIGN_ED25519_SIG_SIZE)
-    return glnx_throw (
-        error, "Invalid signature length of %" G_GSIZE_FORMAT " bytes, expected %" G_GSIZE_FORMAT,
-        (gsize)g_bytes_get_size (signature), (gsize)OSTREE_SIGN_ED25519_SIG_SIZE);
-  const guint8 *signature_buf = g_bytes_get_data (signature, NULL);
+  gsize public_key_size;
+  const guint8 *public_key_buf = g_bytes_get_data (public_key, &public_key_size);
 
+  gsize signature_size;
+  const guint8 *signature_buf = g_bytes_get_data (signature, &signature_size);
 #endif
 
 #if defined(HAVE_LIBSODIUM)
+  // Strictly verify pubkey and signature lengths, as libsodium can
+  // only handle raw ed25519 public key and signatures.
+  if (public_key_size != OSTREE_SIGN_ED25519_PUBKEY_SIZE)
+    return glnx_throw (error, "Invalid public key of %" G_GSIZE_FORMAT " expected %" G_GSIZE_FORMAT,
+                       public_key_size, (gsize)OSTREE_SIGN_ED25519_PUBKEY_SIZE);
+  if (signature_size != OSTREE_SIGN_ED25519_SIG_SIZE)
+    return glnx_throw (
+        error, "Invalid signature length of %" G_GSIZE_FORMAT " bytes, expected %" G_GSIZE_FORMAT,
+        signature_size, (gsize)OSTREE_SIGN_ED25519_SIG_SIZE);
+
   // Note that libsodium assumes the passed byte arrays for the signature and public key
   // have at least the expected length, but we checked that above.
   if (crypto_sign_verify_detached (signature_buf, g_bytes_get_data (data, NULL),
@@ -92,16 +100,23 @@ otcore_validate_ed25519_signature (GBytes *data, GBytes *public_key, GBytes *sig
   EVP_MD_CTX *ctx = EVP_MD_CTX_new ();
   if (!ctx)
     return glnx_throw (error, "openssl: failed to allocate context");
-  EVP_PKEY *pkey = EVP_PKEY_new_raw_public_key (EVP_PKEY_ED25519, NULL, public_key_buf,
-                                                OSTREE_SIGN_ED25519_PUBKEY_SIZE);
+
+  // Try SubjectPublicKeyInfo encoded public key first.
+  const unsigned char *p = public_key_buf;
+  EVP_PKEY *pkey = d2i_PUBKEY (NULL, &p, public_key_size);
+
+  // Try raw ed25519 public key if the length matches.
+  if (pkey == NULL && public_key_size == OSTREE_SIGN_ED25519_PUBKEY_SIZE)
+    pkey = EVP_PKEY_new_raw_public_key (EVP_PKEY_ED25519, NULL, public_key_buf, public_key_size);
+
   if (!pkey)
     {
       EVP_MD_CTX_free (ctx);
-      return glnx_throw (error, "openssl: Failed to initialize ed5519 key");
+      return glnx_throw (error, "openssl: Failed to initialize ed25519 key");
     }
   if (EVP_DigestVerifyInit (ctx, NULL, NULL, NULL, pkey) != 0
-      && EVP_DigestVerify (ctx, signature_buf, OSTREE_SIGN_ED25519_SIG_SIZE,
-                           g_bytes_get_data (data, NULL), g_bytes_get_size (data))
+      && EVP_DigestVerify (ctx, signature_buf, signature_size, g_bytes_get_data (data, NULL),
+                           g_bytes_get_size (data))
              != 0)
     {
       *out_valid = true;

src/libotcore/otcore.h

@@ -27,6 +27,7 @@
 #define USE_LIBSODIUM
 #elif defined(HAVE_OPENSSL)
 #include <openssl/evp.h>
+#include <openssl/x509.h>
 #define USE_OPENSSL
 #endif