Fix bugs found via coverity (thanks to Bhaskar Jain)

 * lasso/id-wsf-2.0/data_service.c: fix uninitialized res variable in
   lasso_idwsf2_data_service_process_query_response_soap_fault_msg.
 * lasso/xml/saml-2.0/saml2_assertion.c: fix uninitialized rc variable
   in get_xmlNode.
 * lasso/saml-2.0/login.c:
   in lasso_saml20_login_accept_sso check for ni and ni->Format
   null-ness before dereferencing, remove idp_ni which is not used
   anymore.
   remote all use of federation->remote_nameIdentifier, SAML 2.0 only
   need one NameID, and it will be local_nameIdentifier.
 * lasso/xml/xml.c:
   in lasso_node_traversal, check null-ness of node before dereferencing
   it, add check for class null-ness also.
 * lasso/id-ff/provider.c:
   in lasso_provider_get_first_http_method, remove useless check for t2
   null-ness -- if found is TRUE, t1 and t2 cannot be null.
 * lasso/xml/tools.c:
   in lasso_sign_node, add documentation, check for private_key_file and
   xmlnode null-ness.
   in lasso_get_public_key_from_private_key_file, add a cleanup phase,
   check for cert variabl null-ness befor appending, count the number of
   certificates added.
   in lasso_query_verify_signature, check that URL unescaping and base64
   decoding are succesfull before using the decoded strings.
 * lasso/saml-2.0/name_id_management.c:
   in lasso_name_id_management_validate_request, fix mis-handling of
   federation, if federation does not match request name_id, return
   UNKNOWN_PRINCIPAL.

7 files changed, 91 insertions, 59 deletions

diff --git a/lasso/id-ff/provider.c b/lasso/id-ff/provider.c
index d41ab0e0..4aa9cae1 100644
--- a/lasso/id-ff/provider.c
+++ b/lasso/id-ff/provider.c
@@ -179,7 +179,7 @@ lasso_provider_get_first_http_method(LassoProvider *provider,
 	char *protocol_profile_prefix;
 	GList *local_supported_profiles;
 	GList *remote_supported_profiles;
-	GList *t1, *t2 = NULL;
+	GList *t1 = NULL, *t2 = NULL;
 	gboolean found;
 
 	g_return_val_if_fail(LASSO_IS_PROVIDER(provider), LASSO_HTTP_METHOD_NONE);
@@ -221,7 +221,7 @@ lasso_provider_get_first_http_method(LassoProvider *provider,
 	if (found) {
 		if (g_str_has_suffix(t2->data, "http"))
 			return LASSO_HTTP_METHOD_REDIRECT;
-		if (t2 && g_str_has_suffix(t2->data, "soap"))
+		if (g_str_has_suffix(t2->data, "soap"))
 			return LASSO_HTTP_METHOD_SOAP;
 		g_assert_not_reached();
 	}
diff --git a/lasso/id-wsf-2.0/data_service.c b/lasso/id-wsf-2.0/data_service.c
index 0364d6a7..70bf5a60 100644
--- a/lasso/id-wsf-2.0/data_service.c
+++ b/lasso/id-wsf-2.0/data_service.c
@@ -360,7 +360,7 @@ lasso_idwsf2_data_service_process_query_response_soap_fault_msg(LassoIdWsf2DataS
 	LassoSoapFault *fault;
 	LassoIdWsf2Sb2RedirectRequest *redirect_request = NULL;
 	GList *iter;
-	int res;
+	int res = 0;
 
 	if (! LASSO_IS_SOAP_FAULT(LASSO_PROFILE(profile)->response)) {
 		/* Should not happen as it should be checked in caller */
diff --git a/lasso/saml-2.0/login.c b/lasso/saml-2.0/login.c
index cad38187..e0da4562 100644
--- a/lasso/saml-2.0/login.c
+++ b/lasso/saml-2.0/login.c
@@ -708,13 +708,8 @@ lasso_saml20_login_build_assertion(LassoLogin *login,
 				LASSO_SAML2_NAME_IDENTIFIER_FORMAT_ENCRYPTED) == 0) {
 			provider->private_data->encryption_mode |= LASSO_ENCRYPTION_MODE_NAMEID;
 		}
-		if (federation->remote_nameIdentifier) {
-			assertion->Subject->NameID = g_object_ref(
-					federation->remote_nameIdentifier);
-		} else {
-			assertion->Subject->NameID = g_object_ref(
-					federation->local_nameIdentifier);
-		}
+		lasso_assign_gobject(assertion->Subject->NameID,
+				federation->local_nameIdentifier);
 	}
 
 	/* Encrypt NameID */
@@ -1230,7 +1225,7 @@ lasso_saml20_login_accept_sso(LassoLogin *login)
 	LassoProfile *profile;
 	LassoSaml2Assertion *assertion;
 	GList *previous_assertions, *t;
-	LassoSaml2NameID *ni, *idp_ni = NULL;
+	LassoSaml2NameID *ni;
 	LassoFederation *federation;
 
 	profile = LASSO_PROFILE(login);
@@ -1269,14 +1264,10 @@ lasso_saml20_login_accept_sso(LassoLogin *login)
 	}
 
 	/* create federation, only if nameidentifier format is Federated */
-	if (strcmp(ni->Format, LASSO_SAML2_NAME_IDENTIFIER_FORMAT_PERSISTENT) == 0) {
+	if (ni && ni->Format && strcmp(ni->Format, LASSO_SAML2_NAME_IDENTIFIER_FORMAT_PERSISTENT) == 0) {
 		federation = lasso_federation_new(LASSO_PROFILE(login)->remote_providerID);
-		if (ni != NULL && idp_ni != NULL) {
-			federation->local_nameIdentifier = g_object_ref(ni);
-			federation->remote_nameIdentifier = g_object_ref(idp_ni);
-		} else {
-			federation->remote_nameIdentifier = g_object_ref(ni);
-		}
+
+		lasso_assign_gobject(federation->local_nameIdentifier, ni);
 		/* add federation in identity */
 		lasso_identity_add_federation(LASSO_PROFILE(login)->identity, federation);
 	}
diff --git a/lasso/saml-2.0/name_id_management.c b/lasso/saml-2.0/name_id_management.c
index c204ea50..c7fe4eb9 100644
--- a/lasso/saml-2.0/name_id_management.c
+++ b/lasso/saml-2.0/name_id_management.c
@@ -173,9 +173,9 @@ lasso_name_id_management_validate_request(LassoNameIdManagement *name_id_managem
 {
 	LassoProfile *profile = NULL;
 	LassoProvider *remote_provider = NULL;
-	LassoFederation *federation = NULL;
 	LassoSamlp2StatusResponse *response = NULL;
 	LassoSaml2NameID *name_id = NULL;
+	LassoFederation *federation = NULL;
 	int rc = 0;
 
 	lasso_bad_param(NAME_ID_MANAGEMENT, name_id_management);
@@ -186,24 +186,43 @@ lasso_name_id_management_validate_request(LassoNameIdManagement *name_id_managem
 	if (rc)
 		goto cleanup;
 
+	/* Get the federation */
+	federation = lasso_identity_get_federation(profile->identity,
+			remote_provider->ProviderID);
+	if (LASSO_IS_FEDERATION(federation) == FALSE) {
+		rc = critical_error(LASSO_PROFILE_ERROR_FEDERATION_NOT_FOUND);
+		goto cleanup;
+	}
 
 	/* Get the name identifier */
 	name_id = LASSO_SAMLP2_MANAGE_NAME_ID_REQUEST(profile->request)->NameID;
-	if (name_id == NULL) {
+	if (! LASSO_IS_SAML2_NAME_ID(name_id)) {
 		message(G_LOG_LEVEL_CRITICAL,
 				"Name identifier not found in name id management request");
 		lasso_saml20_profile_set_response_status(
 				profile,
 				LASSO_SAML2_STATUS_CODE_UNKNOWN_PRINCIPAL);
-		return LASSO_PROFILE_ERROR_NAME_IDENTIFIER_NOT_FOUND;
+		rc = LASSO_PROFILE_ERROR_NAME_IDENTIFIER_NOT_FOUND;
+		goto cleanup;
 	}
 
+	/* Check it matches */
+	if (! lasso_federation_verify_name_identifier(federation, (LassoNode*)name_id)) {
+		lasso_saml20_profile_set_response_status(
+				profile,
+				LASSO_SAML2_STATUS_CODE_UNKNOWN_PRINCIPAL);
+		rc = LASSO_PROFILE_ERROR_FEDERATION_NOT_FOUND;
+		goto cleanup;
+	}
+
+	/* Ok it matches, now apply modifications */
 	if (LASSO_SAMLP2_MANAGE_NAME_ID_REQUEST(profile->request)->Terminate) {
 		/* defederation */
-		lasso_identity_remove_federation(profile->identity, profile->remote_providerID);
+		lasso_identity_remove_federation(profile->identity, remote_provider->ProviderID);
 	} else {
 		/* name registration */
 		LassoSaml2NameID *new_name_id;
+
 		new_name_id = LASSO_SAML2_NAME_ID(lasso_saml2_name_id_new());
 		new_name_id->Format = g_strdup(name_id->Format);
 		new_name_id->NameQualifier = g_strdup(name_id->NameQualifier);
@@ -226,11 +245,10 @@ lasso_name_id_management_validate_request(LassoNameIdManagement *name_id_managem
 			new_name_id->content = g_strdup(
 				LASSO_SAMLP2_MANAGE_NAME_ID_REQUEST(profile->request)->NewID);
 		}
-
-		if (federation->local_nameIdentifier)
-			lasso_node_destroy(LASSO_NODE(federation->local_nameIdentifier));
-		federation->local_nameIdentifier = g_object_ref(new_name_id);
-		profile->identity->is_dirty = TRUE;
+		/* Get federation */
+		lasso_assign_gobject(federation->local_nameIdentifier, new_name_id);
+		/* Set identity is_dirty */
+		lasso_identity_add_federation(profile->identity, federation);
 	}
 cleanup:
 	lasso_release_gobject(response);
diff --git a/lasso/xml/saml-2.0/saml2_assertion.c b/lasso/xml/saml-2.0/saml2_assertion.c
index 0b5acf1f..17ef06f9 100644
--- a/lasso/xml/saml-2.0/saml2_assertion.c
+++ b/lasso/xml/saml-2.0/saml2_assertion.c
@@ -119,7 +119,7 @@ get_xmlNode(LassoNode *node, gboolean lasso_dump)
 {
 	LassoSaml2Assertion *assertion = LASSO_SAML2_ASSERTION(node);
 	xmlNode *xmlnode;
-	int rc;
+	int rc = 0;
 
 	xmlnode = parent_class->get_xmlNode(node, lasso_dump);
 
diff --git a/lasso/xml/tools.c b/lasso/xml/tools.c
index 30bcd51e..8924927c 100644
--- a/lasso/xml/tools.c
+++ b/lasso/xml/tools.c
@@ -315,73 +315,72 @@ lasso_get_public_key_from_private_key_file(const char *private_key_file)
 xmlSecKeysMngrPtr
 lasso_load_certs_from_pem_certs_chain_file(const char* pem_certs_chain_file)
 {
-	xmlSecKeysMngrPtr keys_mngr;
-	GIOChannel *gioc;
-	gchar *line;
+	xmlSecKeysMngrPtr keys_mngr = NULL;
+	GIOChannel *gioc = NULL;
+	gchar *line = NULL;
 	gsize len, pos;
 	GString *cert = NULL;
 	gint ret;
+	gint certificates = 0;
 
 	/* No file just return NULL */
-	if (! pem_certs_chain_file || strlen(pem_certs_chain_file) == 0) {
-		return NULL;
-	}
+	goto_cleanup_if_fail (pem_certs_chain_file && strlen(pem_certs_chain_file) != 0);
 	gioc = g_io_channel_new_file(pem_certs_chain_file, "r", NULL);
 	if (! gioc) {
 		message(G_LOG_LEVEL_WARNING, "Cannot open chain file %s", pem_certs_chain_file);
-		return NULL;
+		goto cleanup;
 	}
 
-	/* create keys manager */
 	keys_mngr = xmlSecKeysMngrCreate();
 	if (keys_mngr == NULL) {
 		message(G_LOG_LEVEL_CRITICAL,
 				lasso_strerror(LASSO_DS_ERROR_KEYS_MNGR_CREATION_FAILED));
-		return NULL;
+		goto cleanup;
 	}
+
 	/* initialize keys manager */
 	if (xmlSecCryptoAppDefaultKeysMngrInit(keys_mngr) < 0) {
 		message(G_LOG_LEVEL_CRITICAL,
 				lasso_strerror(LASSO_DS_ERROR_KEYS_MNGR_INIT_FAILED));
 		xmlSecKeysMngrDestroy(keys_mngr);
-		return NULL;
+		goto cleanup;
 	}
 
 	while (g_io_channel_read_line(gioc, &line, &len, &pos, NULL) == G_IO_STATUS_NORMAL) {
-		if (g_strstr_len(line, 64, "BEGIN CERTIFICATE") != NULL) {
+		if (line != NULL && g_strstr_len(line, 64, "BEGIN CERTIFICATE") != NULL) {
 			cert = g_string_new(line);
-		} else if (g_strstr_len(line, 64, "END CERTIFICATE") != NULL) {
+		} else if (cert != NULL && line != NULL && g_strstr_len(line, 64, "END CERTIFICATE") != NULL) {
 			g_string_append(cert, line);
 			/* load the new certificate found in the keys manager */
+			/* create keys manager */
 			ret = xmlSecCryptoAppKeysMngrCertLoadMemory(keys_mngr,
 					(const xmlSecByte*) cert->str,
 					(xmlSecSize) cert->len,
 					xmlSecKeyDataFormatPem,
 					xmlSecKeyDataTypeTrusted);
-			g_string_free(cert, TRUE);
-			cert = NULL;
 			if (ret < 0) {
-				if (line) {
-					g_free(line);
-					xmlSecKeysMngrDestroy(keys_mngr);
-				}
-				g_io_channel_shutdown(gioc, TRUE, NULL);
-				return NULL;
+				goto cleanup;
 			}
+			certificates++;
+			g_string_free(cert, TRUE);
+			cert = NULL;
 		} else if (cert != NULL && line != NULL && line[0] != '\0') {
 			g_string_append(cert, line);
-		} else {
-			debug("Empty line found in the CA certificate chain file");
 		}
 		/* free last line read */
-		if (line != NULL) {
-			g_free(line);
-			line = NULL;
-		}
+		lasso_release_string(line);
 	}
 
-	g_io_channel_shutdown(gioc, TRUE, NULL);
-	g_io_channel_unref(gioc);
+cleanup:
+	if (gioc) {
+		g_io_channel_shutdown(gioc, TRUE, NULL);
+		g_io_channel_unref(gioc);
+	}
+	if (cert)
+		g_string_free(cert, TRUE);
+	if (certificates == 0)
+		lasso_release_key_manager(keys_mngr);
+	lasso_release_string(line);
 
 	return keys_mngr;
 }
@@ -591,7 +590,7 @@ lasso_query_verify_signature(const char *query, const xmlSecKey *sender_public_k
 	 * covered by the signature */
 
 	str_split = g_strsplit(query, "&Signature=", 0);
-	if (str_split[1] == NULL) {
+	if (str_split[0] == NULL || str_split[1] == NULL) {
 		g_strfreev(str_split);
 		return LASSO_DS_ERROR_SIGNATURE_NOT_FOUND;
 	}
@@ -644,7 +643,10 @@ lasso_query_verify_signature(const char *query, const xmlSecKey *sender_public_k
 	/* get signature (unescape + base64 decode) */
 	signature = xmlMalloc(key_size+1);
 	b64_signature = (char*)xmlURIUnescapeString(str_split[1], 0, NULL);
-	xmlSecBase64Decode((xmlChar*)b64_signature, signature, key_size+1);
+	if (b64_signature == NULL || xmlSecBase64Decode((xmlChar*)b64_signature, signature, key_size+1) < 0) {
+		ret = LASSO_DS_ERROR_INVALID_SIGNATURE;
+		goto done;
+	}
 
 	/* compute signature digest */
 	digest = lasso_sha1(str_split[0]);
@@ -760,6 +762,21 @@ error_code(G_GNUC_UNUSED GLogLevelFlags level, int error, ...)
 }
 
 
+/**
+ * lasso_sign_node:
+ * @xmlnode: the xmlnode to sign
+ * @id_attr_name: (allow-none): an ID attribute to reference the xmlnode in the signature
+ * @id_value: (allow-none): value of the ID attribute
+ * @private_key_file: the path to a key file, or the key itself PEM encoded.
+ * @certificate_file: (allow-none): the path to a certificate file to place in the KeyInfo, or the certificate
+ * itself PEM encoded.
+ *
+ * Sign an xmlnode, use the given attribute to reference or create an envelopped signature,
+ * eventually place a certificate in the KeyInfo node. The signature template must already be
+ * present on the xmlnode.
+ *
+ * Return value: 0 if successful, an error code otherwise.
+ */
 int
 lasso_sign_node(xmlNode *xmlnode, const char *id_attr_name, const char *id_value,
 		const char *private_key_file, const char *certificate_file)
@@ -769,6 +786,9 @@ lasso_sign_node(xmlNode *xmlnode, const char *id_attr_name, const char *id_value
 	xmlSecDSigCtx *dsig_ctx;
 	xmlAttr *id_attr = NULL;
 
+	if (private_key_file == NULL || xmlnode == NULL)
+		return LASSO_PARAM_ERROR_BAD_TYPE_OR_NULL_OBJ;
+
 	sign_tmpl = xmlSecFindNode(xmlnode, xmlSecNodeSignature, xmlSecDSigNs);
 	if (sign_tmpl == NULL)
 		return LASSO_DS_ERROR_SIGNATURE_TEMPLATE_NOT_FOUND;
diff --git a/lasso/xml/xml.c b/lasso/xml/xml.c
index e9cb0d73..59013d3c 100644
--- a/lasso/xml/xml.c
+++ b/lasso/xml/xml.c
@@ -874,8 +874,11 @@ lasso_node_traversal(LassoNode *node, void (*do_to_node)(LassoNode *node, Snippe
 	LassoNodeClass *class;
 	struct XmlSnippet *snippet;
 
+	if (node == NULL) {
+		return;
+	}
 	class = LASSO_NODE_GET_CLASS(node);
-	if (class->node_data == NULL || node == NULL || do_to_node == NULL) {
+	if (class == NULL || class->node_data == NULL || do_to_node == NULL) {
 		return;
 	}
 	do_to_node(node, type);