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 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

@@ -38,6 +38,8 @@
 #define OSTREE_SIGN_ED25519_SECKEY_SIZE \
   (OSTREE_SIGN_ED25519_SEED_SIZE + OSTREE_SIGN_ED25519_PUBKEY_SIZE)
 
+#define FIXED_KEY_SIZES (defined (USE_LIBSODIUM))
+
 typedef enum
 {
   ED25519_OK,
@@ -49,9 +51,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);
@@ -97,6 +99,9 @@ _ostree_sign_ed25519_init (OstreeSignEd25519 *self)
 #endif
 }
 
+#if FIXED_KEY_SIZES
+// 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)
 {
@@ -106,6 +111,7 @@ validate_length (gsize found, gsize expected, GError **error)
       error, "Ill-formed input: expected %" G_GSIZE_FORMAT " bytes, got %" G_GSIZE_FORMAT " bytes",
       expected, found);
 }
+#endif
 
 static gboolean
 _ostree_sign_ed25519_is_initialized (OstreeSignEd25519 *self, GError **error)
@@ -137,19 +143,33 @@ 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);
@@ -158,8 +178,12 @@ ostree_sign_ed25519_data (OstreeSign *self, GBytes *data, GBytes **signature,
 
   size_t len;
   if (EVP_DigestSignInit (ctx, NULL, NULL, NULL, pkey)
-      && EVP_DigestSign (ctx, sig, &len, g_bytes_get_data (data, NULL), g_bytes_get_size (data)))
-    sig_size = len;
+      && EVP_DigestSign (ctx, NULL, &len, g_bytes_get_data (data, NULL), g_bytes_get_size (data)))
+    {
+      sig = g_malloc0 (len);
+      if (EVP_DigestSign (ctx, sig, &len, g_bytes_get_data (data, NULL), g_bytes_get_size (data)))
+        sig_size = len;
+    }
 
   EVP_PKEY_free (pkey);
   EVP_MD_CTX_free (ctx);
@@ -173,12 +197,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)
@@ -223,29 +241,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 FIXED_KEY_SIZES
       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)
             {
@@ -255,14 +271,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);
                 }
@@ -321,22 +340,23 @@ ostree_sign_ed25519_clear_keys (OstreeSign *self, GError **error)
   /* Clear secret key */
   if (sign->secret_key != NULL)
     {
-      explicit_bzero (sign->secret_key, OSTREE_SIGN_ED25519_SECKEY_SIZE);
-      g_free (sign->secret_key);
+      gsize size;
+      gpointer data = g_bytes_unref_to_data (sign->secret_key, &size);
+      explicit_bzero (data, 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;
     }
 
@@ -375,10 +395,12 @@ ostree_sign_ed25519_set_sk (OstreeSign *self, GVariant *secret_key, GError **err
       return glnx_throw (error, "Unknown ed25519 secret key type");
     }
 
+#if FIXED_KEY_SIZES
   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;
 }
@@ -430,17 +452,20 @@ ostree_sign_ed25519_add_pk (OstreeSign *self, GVariant *public_key, GError **err
       return glnx_throw (error, "Unknown ed25519 public key type");
     }
 
+#if FIXED_KEY_SIZES
   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;
@@ -461,17 +486,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 FIXED_KEY_SIZES
   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,20 +69,29 @@ 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 (public_key_size < OSTREE_SIGN_ED25519_PUBKEY_SIZE
+      || public_key_size > OSTREE_SIGN_METADATA_ED25519_MAX_SIZE)
+    return glnx_throw (error,
+                       "Invalid public key of %" G_GSIZE_FORMAT " bytes, expected %" G_GSIZE_FORMAT
+                       " <= len <= %" G_GSIZE_FORMAT,
+                       public_key_size, (gsize)OSTREE_SIGN_ED25519_PUBKEY_SIZE,
+                       (gsize)OSTREE_SIGN_METADATA_ED25519_MAX_SIZE);
+
+  if (signature_size < OSTREE_SIGN_ED25519_SIG_SIZE
+      || signature_size > OSTREE_SIGN_METADATA_ED25519_MAX_SIZE)
+    return glnx_throw (error,
+                       "Invalid signature of %" G_GSIZE_FORMAT " bytes, expected %" G_GSIZE_FORMAT
+                       " <= len <= %" G_GSIZE_FORMAT,
+                       signature_size, (gsize)OSTREE_SIGN_ED25519_SIG_SIZE,
+                       (gsize)OSTREE_SIGN_METADATA_ED25519_MAX_SIZE);
+
 #if defined(HAVE_LIBSODIUM)
   // 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.
@@ -92,16 +106,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 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
 
@@ -38,6 +39,8 @@
 #define OSTREE_SIGN_METADATA_ED25519_KEY "ostree.sign.ed25519"
 // The variant type
 #define OSTREE_SIGN_METADATA_ED25519_TYPE "aay"
+// Maximum size of metadata in bytes, in sync with OSTREE_MAX_METADATA_SIZE
+#define OSTREE_SIGN_METADATA_ED25519_MAX_SIZE (128 * 1024 * 1024)
 
 bool otcore_ed25519_init (void);
 gboolean otcore_validate_ed25519_signature (GBytes *data, GBytes *pubkey, GBytes *signature,