sign: fix memory leaks and code cleanup

Return `const char *` instead of copy of the string -- this allow to
avoid unneeded copying and memory leaks in some constructions.
Minor code cleanup and optimisations.

Signed-off-by: Denis Pynkin <denis.pynkin@collabora.com>
This commit is contained in:
Denis Pynkin 2019-09-05 02:04:25 +03:00
parent 5fc2ddff30
commit 557f423609
7 changed files with 65 additions and 85 deletions

View File

@ -1519,28 +1519,26 @@ ostree_verify_unwritten_commit (OtPullData *pull_data,
gboolean ret = FALSE; gboolean ret = FALSE;
g_autoptr(GBytes) signed_data = g_variant_get_data_as_bytes (commit); g_autoptr(GBytes) signed_data = g_variant_get_data_as_bytes (commit);
/* list all signature types in detached metadata and check if signed by any? */ /* list all signature types in detached metadata and check if signed by any? */
g_auto(GStrv) names = ostree_sign_list_names(); g_auto (GStrv) names = ostree_sign_list_names();
for (guint i=0; i < g_strv_length (names); i++) for (guint i=0; i < g_strv_length (names); i++)
{ {
g_autoptr (OstreeSign) sign = NULL; g_autoptr (OstreeSign) sign = NULL;
g_autoptr (GError) local_error = NULL;
g_autoptr (GVariant) signatures = NULL; g_autoptr (GVariant) signatures = NULL;
g_autofree gchar *signature_key = NULL; const gchar *signature_key = NULL;
g_autofree GVariantType *signature_format = NULL; GVariantType *signature_format = NULL;
g_autofree gchar *pk_ascii = NULL; g_autofree gchar *pk_ascii = NULL;
g_autofree gchar *pk_file = NULL; g_autofree gchar *pk_file = NULL;
if ((sign = ostree_sign_get_by_name (names[i], error)) == NULL) if ((sign = ostree_sign_get_by_name (names[i], &local_error)) == NULL)
{ continue;
g_clear_error (error);
continue;
}
signature_key = ostree_sign_metadata_key (sign); signature_key = ostree_sign_metadata_key (sign);
signature_format = (GVariantType *) ostree_sign_metadata_format (sign); signature_format = (GVariantType *) ostree_sign_metadata_format (sign);
signatures = g_variant_lookup_value (detached_metadata, signatures = g_variant_lookup_value (detached_metadata,
signature_key, signature_key,
signature_format); signature_format);
if (!signatures) if (!signatures)
continue; continue;
@ -1558,8 +1556,8 @@ ostree_verify_unwritten_commit (OtPullData *pull_data,
g_variant_builder_add (builder, "{sv}", "filename", g_variant_new_string (pk_file)); g_variant_builder_add (builder, "{sv}", "filename", g_variant_new_string (pk_file));
options = g_variant_builder_end (builder); options = g_variant_builder_end (builder);
if (!ostree_sign_load_pk (sign, options, error)) if (!ostree_sign_load_pk (sign, options, &local_error))
g_clear_error (error); g_clear_error (&local_error);
} }
/* Override key if it is set explicitly */ /* Override key if it is set explicitly */
@ -1583,27 +1581,23 @@ ostree_verify_unwritten_commit (OtPullData *pull_data,
pk = g_variant_new_fixed_array (G_VARIANT_TYPE_BYTE, key, key_len, sizeof(guchar)); pk = g_variant_new_fixed_array (G_VARIANT_TYPE_BYTE, key, key_len, sizeof(guchar));
} }
if (!ostree_sign_set_pk (sign, pk, error)) if (!ostree_sign_set_pk (sign, pk, &local_error))
g_clear_error (error); continue;
} }
/* Set return to true if any sign fit */ /* Set return to true if any sign fit */
if (ostree_sign_metadata_verify (sign, if (ostree_sign_metadata_verify (sign,
signed_data, signed_data,
signatures, signatures,
error &local_error
)) ))
ret = TRUE; ret = TRUE;
else
g_clear_error (error);
} }
/* Mark the commit as verified to avoid double verification /* Mark the commit as verified to avoid double verification
* see process_verify_result () for rationale */ * see process_verify_result () for rationale */
if (ret) if (ret)
{
g_hash_table_add (pull_data->verified_commits, g_strdup (checksum)); g_hash_table_add (pull_data->verified_commits, g_strdup (checksum));
}
else else
g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED,
"Can't verify commit"); "Can't verify commit");
@ -1946,17 +1940,15 @@ scan_commit_object (OtPullData *pull_data,
gboolean ret = FALSE; gboolean ret = FALSE;
/* list all signature types in detached metadata and check if signed by any? */ /* list all signature types in detached metadata and check if signed by any? */
g_auto (GStrv) names = ostree_sign_list_names(); g_auto (GStrv) names = ostree_sign_list_names();
for (guint i=0; i < g_strv_length (names); i++) for (char **iter=names; iter && *iter; iter++)
{ {
g_autoptr (OstreeSign) sign = NULL; g_autoptr (OstreeSign) sign = NULL;
g_autoptr (GError) local_error = NULL;
g_autofree gchar *pk_ascii = NULL; g_autofree gchar *pk_ascii = NULL;
g_autofree gchar *pk_file = NULL; g_autofree gchar *pk_file = NULL;
if ((sign = ostree_sign_get_by_name (names[i], error)) == NULL) if ((sign = ostree_sign_get_by_name (*iter, &local_error)) == NULL)
{ continue;
g_clear_error (error);
continue;
}
/* Load keys for remote from file */ /* Load keys for remote from file */
ostree_repo_get_remote_option (pull_data->repo, ostree_repo_get_remote_option (pull_data->repo,
@ -1972,8 +1964,8 @@ scan_commit_object (OtPullData *pull_data,
g_variant_builder_add (builder, "{sv}", "filename", g_variant_new_string (pk_file)); g_variant_builder_add (builder, "{sv}", "filename", g_variant_new_string (pk_file));
options = g_variant_builder_end (builder); options = g_variant_builder_end (builder);
if (!ostree_sign_load_pk (sign, options, error)) if (!ostree_sign_load_pk (sign, options, &local_error))
g_clear_error (error); g_clear_error (&local_error);
} }
ostree_repo_get_remote_option (pull_data->repo, ostree_repo_get_remote_option (pull_data->repo,
@ -1996,8 +1988,8 @@ scan_commit_object (OtPullData *pull_data,
pk = g_variant_new_fixed_array (G_VARIANT_TYPE_BYTE, key, key_len, sizeof(guchar)); pk = g_variant_new_fixed_array (G_VARIANT_TYPE_BYTE, key, key_len, sizeof(guchar));
} }
if (!ostree_sign_set_pk (sign, pk, error)) if (!ostree_sign_set_pk (sign, pk, &local_error))
g_clear_error (error); continue;
} }
@ -2006,10 +1998,8 @@ scan_commit_object (OtPullData *pull_data,
pull_data->repo, pull_data->repo,
checksum, checksum,
cancellable, cancellable,
error)) &local_error))
ret = TRUE; ret = TRUE;
else
g_clear_error (error);
} }
if (!ret) if (!ret)

View File

@ -108,30 +108,26 @@ gboolean ostree_sign_dummy_data (OstreeSign *self,
return TRUE; return TRUE;
} }
gchar * ostree_sign_dummy_get_name (OstreeSign *self) const gchar * ostree_sign_dummy_get_name (OstreeSign *self)
{ {
g_debug ("%s enter", __FUNCTION__); g_debug ("%s enter", __FUNCTION__);
g_return_val_if_fail (OSTREE_IS_SIGN (self), FALSE); g_return_val_if_fail (OSTREE_IS_SIGN (self), FALSE);
g_autofree gchar *name = g_strdup(OSTREE_SIGN_DUMMY_NAME); return OSTREE_SIGN_DUMMY_NAME;
return g_steal_pointer (&name);
} }
gchar * ostree_sign_dummy_metadata_key (OstreeSign *self) const gchar * ostree_sign_dummy_metadata_key (OstreeSign *self)
{ {
g_debug ("%s enter", __FUNCTION__); g_debug ("%s enter", __FUNCTION__);
g_autofree gchar *key = g_strdup(OSTREE_SIGN_METADATA_DUMMY_KEY); return OSTREE_SIGN_METADATA_DUMMY_KEY;
return g_steal_pointer (&key);
} }
gchar * ostree_sign_dummy_metadata_format (OstreeSign *self) const gchar * ostree_sign_dummy_metadata_format (OstreeSign *self)
{ {
g_debug ("%s enter", __FUNCTION__); g_debug ("%s enter", __FUNCTION__);
g_autofree gchar *type = g_strdup(OSTREE_SIGN_METADATA_DUMMY_TYPE); return OSTREE_SIGN_METADATA_DUMMY_TYPE;
return g_steal_pointer (&type);
} }
gboolean ostree_sign_dummy_metadata_verify (OstreeSign *self, gboolean ostree_sign_dummy_metadata_verify (OstreeSign *self,

View File

@ -39,7 +39,7 @@ G_DECLARE_FINAL_TYPE (OstreeSignDummy,
SIGN_DUMMY, SIGN_DUMMY,
GObject) GObject)
gchar * ostree_sign_dummy_get_name (OstreeSign *self); const gchar * ostree_sign_dummy_get_name (OstreeSign *self);
gboolean ostree_sign_dummy_data (OstreeSign *self, gboolean ostree_sign_dummy_data (OstreeSign *self,
GBytes *data, GBytes *data,
@ -47,8 +47,8 @@ gboolean ostree_sign_dummy_data (OstreeSign *self,
GCancellable *cancellable, GCancellable *cancellable,
GError **error); GError **error);
gchar * ostree_sign_dummy_metadata_key (OstreeSign *self); const gchar * ostree_sign_dummy_metadata_key (OstreeSign *self);
gchar * ostree_sign_dummy_metadata_format (OstreeSign *self); const gchar * ostree_sign_dummy_metadata_format (OstreeSign *self);
gboolean ostree_sign_dummy_metadata_verify (OstreeSign *self, gboolean ostree_sign_dummy_metadata_verify (OstreeSign *self,
GBytes *data, GBytes *data,

View File

@ -112,7 +112,7 @@ gboolean ostree_sign_ed25519_data (OstreeSign *self,
OstreeSignEd25519 *sign = ostree_sign_ed25519_get_instance_private(OSTREE_SIGN_ED25519(self)); OstreeSignEd25519 *sign = ostree_sign_ed25519_get_instance_private(OSTREE_SIGN_ED25519(self));
#ifdef HAVE_LIBSODIUM #ifdef HAVE_LIBSODIUM
g_autofree guchar *sig = NULL; guchar *sig = NULL;
#endif #endif
if ((sign->initialized != TRUE) || (sign->secret_key == NULL)) if ((sign->initialized != TRUE) || (sign->secret_key == NULL))
@ -137,37 +137,33 @@ gboolean ostree_sign_ed25519_data (OstreeSign *self,
goto err; goto err;
} }
*signature = g_bytes_new (sig, sig_size); *signature = g_bytes_new_take (sig, sig_size);
return TRUE; return TRUE;
#endif /* HAVE_LIBSODIUM */ #endif /* HAVE_LIBSODIUM */
err: err:
return FALSE; return FALSE;
} }
gchar * ostree_sign_ed25519_get_name (OstreeSign *self) const gchar * ostree_sign_ed25519_get_name (OstreeSign *self)
{ {
g_debug ("%s enter", __FUNCTION__); g_debug ("%s enter", __FUNCTION__);
g_return_val_if_fail (OSTREE_IS_SIGN (self), FALSE); g_return_val_if_fail (OSTREE_IS_SIGN (self), FALSE);
g_autofree gchar *name = g_strdup (OSTREE_SIGN_ED25519_NAME); return OSTREE_SIGN_ED25519_NAME;
return g_steal_pointer (&name);
} }
gchar * ostree_sign_ed25519_metadata_key (OstreeSign *self) const gchar * ostree_sign_ed25519_metadata_key (OstreeSign *self)
{ {
g_debug ("%s enter", __FUNCTION__); g_debug ("%s enter", __FUNCTION__);
g_autofree gchar *key = g_strdup(OSTREE_SIGN_METADATA_ED25519_KEY); return OSTREE_SIGN_METADATA_ED25519_KEY;
return g_steal_pointer (&key);
} }
gchar * ostree_sign_ed25519_metadata_format (OstreeSign *self) const gchar * ostree_sign_ed25519_metadata_format (OstreeSign *self)
{ {
g_debug ("%s enter", __FUNCTION__); g_debug ("%s enter", __FUNCTION__);
g_autofree gchar *type = g_strdup (OSTREE_SIGN_METADATA_ED25519_TYPE); return OSTREE_SIGN_METADATA_ED25519_TYPE;
return g_steal_pointer (&type);
} }
gboolean ostree_sign_ed25519_metadata_verify (OstreeSign *self, gboolean ostree_sign_ed25519_metadata_verify (OstreeSign *self,
@ -187,7 +183,7 @@ gboolean ostree_sign_ed25519_metadata_verify (OstreeSign *self,
g_set_error_literal (error, g_set_error_literal (error,
G_IO_ERROR, G_IO_ERROR_FAILED, G_IO_ERROR, G_IO_ERROR_FAILED,
"signature: ed25519: commit have no signatures of my type"); "signature: ed25519: commit have no signatures of my type");
goto err; goto out;
} }
if (!g_variant_is_of_type (signatures, (GVariantType *) OSTREE_SIGN_METADATA_ED25519_TYPE)) if (!g_variant_is_of_type (signatures, (GVariantType *) OSTREE_SIGN_METADATA_ED25519_TYPE))
@ -195,14 +191,14 @@ gboolean ostree_sign_ed25519_metadata_verify (OstreeSign *self,
g_set_error_literal (error, g_set_error_literal (error,
G_IO_ERROR, G_IO_ERROR_FAILED, G_IO_ERROR, G_IO_ERROR_FAILED,
"signature: ed25519: wrong type passed for verification"); "signature: ed25519: wrong type passed for verification");
goto err; goto out;
} }
if (sign->initialized != TRUE) if (sign->initialized != TRUE)
{ {
g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED,
"Not able to verify: libsodium library isn't initialized properly"); "Not able to verify: libsodium library isn't initialized properly");
goto err; goto out;
} }
#ifdef HAVE_LIBSODIUM #ifdef HAVE_LIBSODIUM
@ -217,7 +213,7 @@ gboolean ostree_sign_ed25519_metadata_verify (OstreeSign *self,
options = g_variant_builder_end (builder); options = g_variant_builder_end (builder);
if (!ostree_sign_ed25519_load_pk (self, options, error)) if (!ostree_sign_ed25519_load_pk (self, options, error))
goto err; goto out;
} }
g_debug ("verify: data hash = 0x%x", g_bytes_hash(data)); g_debug ("verify: data hash = 0x%x", g_bytes_hash(data));
@ -259,9 +255,8 @@ gboolean ostree_sign_ed25519_metadata_verify (OstreeSign *self,
"Not able to verify: no valid signatures found"); "Not able to verify: no valid signatures found");
#endif /* HAVE_LIBSODIUM */ #endif /* HAVE_LIBSODIUM */
out:
return ret; return ret;
err:
return FALSE;
} }
gboolean gboolean
@ -312,7 +307,6 @@ gboolean ostree_sign_ed25519_set_sk (OstreeSign *self,
#ifdef HAVE_LIBSODIUM #ifdef HAVE_LIBSODIUM
OstreeSignEd25519 *sign = ostree_sign_ed25519_get_instance_private(OSTREE_SIGN_ED25519(self)); OstreeSignEd25519 *sign = ostree_sign_ed25519_get_instance_private(OSTREE_SIGN_ED25519(self));
g_autofree char * hex = NULL;
g_free (sign->secret_key); g_free (sign->secret_key);
@ -326,9 +320,6 @@ gboolean ostree_sign_ed25519_set_sk (OstreeSign *self,
goto err; goto err;
} }
hex = g_malloc0 (crypto_sign_SECRETKEYBYTES*2 + 1);
// g_debug ("Set ed25519 secret key = %s", sodium_bin2hex (hex, crypto_sign_SECRETKEYBYTES*2+1, sign->secret_key, n_elements));
return TRUE; return TRUE;
err: err:
@ -348,7 +339,7 @@ gboolean ostree_sign_ed25519_set_pk (OstreeSign *self,
/* Substitute the key(s) with a new one */ /* Substitute the key(s) with a new one */
if (sign->public_keys != NULL) if (sign->public_keys != NULL)
{ {
g_list_free_full (sign->public_keys, g_object_unref); g_list_free_full (sign->public_keys, g_free);
sign->public_keys = NULL; sign->public_keys = NULL;
} }
@ -380,9 +371,11 @@ gboolean ostree_sign_ed25519_add_pk (OstreeSign *self,
goto err; goto err;
} }
key = g_memdup (key, n_elements);
if (g_list_find (sign->public_keys, key) == NULL) if (g_list_find (sign->public_keys, key) == NULL)
sign->public_keys = g_list_prepend (sign->public_keys, key); {
gpointer newkey = g_memdup (key, n_elements);
sign->public_keys = g_list_prepend (sign->public_keys, newkey);
}
return TRUE; return TRUE;
@ -485,6 +478,7 @@ _load_pk_from_file (OstreeSign *self,
GError **error) GError **error)
{ {
g_debug ("%s enter", __FUNCTION__); g_debug ("%s enter", __FUNCTION__);
g_debug ("Processing file '%s'", filename);
g_autoptr (GFile) keyfile = NULL; g_autoptr (GFile) keyfile = NULL;
g_autoptr (GFileInputStream) key_stream_in = NULL; g_autoptr (GFileInputStream) key_stream_in = NULL;
@ -542,7 +536,7 @@ ostree_sign_ed25519_load_pk (OstreeSign *self,
/* Clear already loaded keys */ /* Clear already loaded keys */
if (sign->public_keys != NULL) if (sign->public_keys != NULL)
{ {
g_list_free_full (sign->public_keys, g_object_unref); g_list_free_full (sign->public_keys, g_free);
sign->public_keys = NULL; sign->public_keys = NULL;
} }

View File

@ -46,9 +46,9 @@ gboolean ostree_sign_ed25519_data (OstreeSign *self,
GCancellable *cancellable, GCancellable *cancellable,
GError **error); GError **error);
gchar * ostree_sign_ed25519_get_name (OstreeSign *self); const gchar * ostree_sign_ed25519_get_name (OstreeSign *self);
gchar * ostree_sign_ed25519_metadata_key (OstreeSign *self); const gchar * ostree_sign_ed25519_metadata_key (OstreeSign *self);
gchar * ostree_sign_ed25519_metadata_format (OstreeSign *self); const gchar * ostree_sign_ed25519_metadata_format (OstreeSign *self);
gboolean ostree_sign_ed25519_metadata_verify (OstreeSign *self, gboolean ostree_sign_ed25519_metadata_verify (OstreeSign *self,
GBytes *data, GBytes *data,

View File

@ -71,7 +71,7 @@ ostree_sign_default_init (OstreeSignInterface *iface)
g_debug ("OstreeSign initialization"); g_debug ("OstreeSign initialization");
} }
gchar * ostree_sign_metadata_key (OstreeSign *self) const gchar * ostree_sign_metadata_key (OstreeSign *self)
{ {
g_debug ("%s enter", __FUNCTION__); g_debug ("%s enter", __FUNCTION__);
@ -79,7 +79,7 @@ gchar * ostree_sign_metadata_key (OstreeSign *self)
return OSTREE_SIGN_GET_IFACE (self)->metadata_key (self); return OSTREE_SIGN_GET_IFACE (self)->metadata_key (self);
} }
gchar * ostree_sign_metadata_format (OstreeSign *self) const gchar * ostree_sign_metadata_format (OstreeSign *self)
{ {
g_debug ("%s enter", __FUNCTION__); g_debug ("%s enter", __FUNCTION__);
@ -134,7 +134,7 @@ ostree_sign_load_pk (OstreeSign *self,
g_debug ("%s enter", __FUNCTION__); g_debug ("%s enter", __FUNCTION__);
if (OSTREE_SIGN_GET_IFACE (self)->load_pk == NULL) if (OSTREE_SIGN_GET_IFACE (self)->load_pk == NULL)
return FALSE; return TRUE;
return OSTREE_SIGN_GET_IFACE (self)->load_pk (self, options, error); return OSTREE_SIGN_GET_IFACE (self)->load_pk (self, options, error);
} }
@ -170,8 +170,8 @@ ostree_sign_detached_metadata_append (OstreeSign *self,
g_variant_dict_init (&metadata_dict, existing_metadata); g_variant_dict_init (&metadata_dict, existing_metadata);
g_autofree gchar *signature_key = ostree_sign_metadata_key(self); const gchar *signature_key = ostree_sign_metadata_key(self);
g_autofree GVariantType *signature_format = (GVariantType *) ostree_sign_metadata_format(self); GVariantType *signature_format = (GVariantType *) ostree_sign_metadata_format(self);
signature_data = g_variant_dict_lookup_value (&metadata_dict, signature_data = g_variant_dict_lookup_value (&metadata_dict,
signature_key, signature_key,
@ -234,8 +234,8 @@ ostree_sign_commit_verify (OstreeSign *self,
g_autoptr(GVariant) signatures = NULL; g_autoptr(GVariant) signatures = NULL;
g_autofree gchar *signature_key = ostree_sign_metadata_key(self); const gchar *signature_key = ostree_sign_metadata_key(self);
g_autofree GVariantType *signature_format = (GVariantType *) ostree_sign_metadata_format(self); GVariantType *signature_format = (GVariantType *) ostree_sign_metadata_format(self);
if (metadata) if (metadata)
signatures = g_variant_lookup_value (metadata, signatures = g_variant_lookup_value (metadata,

View File

@ -47,14 +47,14 @@ G_DECLARE_INTERFACE (OstreeSign, ostree_sign, OSTREE, SIGN, GObject)
struct _OstreeSignInterface struct _OstreeSignInterface
{ {
GTypeInterface g_iface; GTypeInterface g_iface;
gchar *(* get_name) (OstreeSign *self); const gchar *(* get_name) (OstreeSign *self);
gboolean (* data) (OstreeSign *self, gboolean (* data) (OstreeSign *self,
GBytes *data, GBytes *data,
GBytes **signature, GBytes **signature,
GCancellable *cancellable, GCancellable *cancellable,
GError **error); GError **error);
gchar *(* metadata_key) (OstreeSign *self); const gchar *(* metadata_key) (OstreeSign *self);
gchar *(* metadata_format) (OstreeSign *self); const gchar *(* metadata_format) (OstreeSign *self);
gboolean (* metadata_verify) (OstreeSign *self, gboolean (* metadata_verify) (OstreeSign *self,
GBytes *data, GBytes *data,
GVariant *metadata, GVariant *metadata,
@ -90,10 +90,10 @@ gboolean ostree_sign_data (OstreeSign *self,
_OSTREE_PUBLIC _OSTREE_PUBLIC
gchar * ostree_sign_metadata_key (OstreeSign *self); const gchar * ostree_sign_metadata_key (OstreeSign *self);
_OSTREE_PUBLIC _OSTREE_PUBLIC
gchar * ostree_sign_metadata_format (OstreeSign *self); const gchar * ostree_sign_metadata_format (OstreeSign *self);
_OSTREE_PUBLIC _OSTREE_PUBLIC
GVariant * ostree_sign_detached_metadata_append (OstreeSign *self, GVariant * ostree_sign_detached_metadata_append (OstreeSign *self,