s3:libads: change ads_add_service_principal_name implementation

Previously the function 'ads_add_service_principal_name' created
the SPNs based on the machine_name and dns name passed to the function.
In order to prepare for a future patch that will also need to write
SPN(s) to the AD computer account, the function implementation will
need to be changed. Instead of the function creating the SPN(s) it
will now take the list SPN(s) to write to the AD 'machine_name' account
as an input param instead.
The name of the function has been changed to
'ads_add_service_principal_names' to reflect this. Additionally  client
code now needs to construct the SPNs to be passed into the function.

Signed-off-by: Noel Power <noel.power@suse.com>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>

3 files changed, 98 insertions, 49 deletions

diff --git a/source3/libads/ads_proto.h b/source3/libads/ads_proto.h
index a35f211c7d3..258162d7d2a 100644
--- a/source3/libads/ads_proto.h
+++ b/source3/libads/ads_proto.h
@@ -95,8 +95,8 @@ ADS_STATUS ads_get_service_principal_names(TALLOC_CTX *mem_ctx,
 					   char ***spn_array,
 					   size_t *num_spns);
 ADS_STATUS ads_clear_service_principal_names(ADS_STRUCT *ads, const char *machine_name);
-ADS_STATUS ads_add_service_principal_name(ADS_STRUCT *ads, const char *machine_name,
-                                          const char *my_fqdn, const char *spn);
+ADS_STATUS ads_add_service_principal_names(ADS_STRUCT *ads, const char *machine_name,
+                                          const char **spns);
 ADS_STATUS ads_create_machine_acct(ADS_STRUCT *ads,
 				   const char *machine_name,
 				   const char *org_unit,
diff --git a/source3/libads/kerberos_keytab.c b/source3/libads/kerberos_keytab.c
index df9ed03a1ad..fbb03402848 100644
--- a/source3/libads/kerberos_keytab.c
+++ b/source3/libads/kerberos_keytab.c
@@ -85,6 +85,80 @@ out:
 	return ret;
 }
 
+static bool fill_default_spns(TALLOC_CTX *ctx, const char *machine_name,
+                                          const char *my_fqdn, const char *spn,
+					  const char ***spns)
+{
+	char *psp1, *psp2;
+
+	if (*spns == NULL) {
+		*spns = talloc_zero_array(ctx, const char*, 3);
+		if (spns == NULL) {
+			return false;
+		}
+	}
+
+	psp1 = talloc_asprintf(ctx,
+			       "%s/%s",
+			       spn,
+			       machine_name);
+	if (psp1 == NULL) {
+		return false;
+	}
+
+	if (!strlower_m(&psp1[strlen(spn) + 1])) {
+		return false;
+	}
+	(*spns)[0] = psp1;
+
+	psp2 = talloc_asprintf(ctx,
+			       "%s/%s",
+			       spn,
+			       my_fqdn);
+	if (psp2 == NULL) {
+		return false;
+	}
+
+	if (!strlower_m(&psp2[strlen(spn) + 1])) {
+		return false;
+	}
+
+	(*spns)[1] = psp2;
+
+	return true;
+}
+
+static bool ads_set_machine_account_spns(TALLOC_CTX *ctx,
+					 ADS_STRUCT *ads,
+					 const char *service_or_spn,
+					 const char *my_fqdn)
+{
+	const char **spn_names = NULL;
+	ADS_STATUS aderr;
+	bool ok = false;
+
+	DBG_INFO("Attempting to add/update '%s'\n", service_or_spn);
+
+	ok = fill_default_spns(ctx,
+			       lp_netbios_name(),
+			       my_fqdn,
+			       service_or_spn,
+			       &spn_names);
+	if (!ok) {
+		return false;
+	}
+
+	aderr = ads_add_service_principal_names(ads,
+						lp_netbios_name(),
+						spn_names);
+	if (!ADS_ERR_OK(aderr)) {
+		DBG_WARNING("Failed to add service principal name.\n");
+		return false;
+	}
+
+	return true;
+}
+
 /**********************************************************************
  Adds a single service principal, i.e. 'host' to the system keytab
 ***********************************************************************/
@@ -114,7 +188,6 @@ int ads_keytab_add_entry(ADS_STRUCT *ads, const char *srvPrinc)
 	char *password_s = NULL;
 	char *my_fqdn;
 	TALLOC_CTX *tmpctx = NULL;
-	ADS_STATUS aderr;
 	int i;
 
 	initialize_krb5_error_table();
@@ -212,14 +285,11 @@ int ads_keytab_add_entry(ADS_STRUCT *ads, const char *srvPrinc)
 
 		if (!strequal(srvPrinc, "cifs") &&
 		    !strequal(srvPrinc, "host")) {
-			DEBUG(3, (__location__ ": Attempting to add/update "
-				  "'%s'\n", princ_s));
-
-			aderr = ads_add_service_principal_name(ads,
-					lp_netbios_name(), my_fqdn, srvPrinc);
-			if (!ADS_ERR_OK(aderr)) {
-				DEBUG(1, (__location__ ": failed to "
-					 "ads_add_service_principal_name.\n"));
+			if (!ads_set_machine_account_spns(tmpctx,
+							  ads,
+							  srvPrinc,
+							  my_fqdn)) {
+				ret = -1;
 				goto out;
 			}
 		}
diff --git a/source3/libads/ldap.c b/source3/libads/ldap.c
index 2acbb3aca11..78b813c67d0 100644
--- a/source3/libads/ldap.c
+++ b/source3/libads/ldap.c
@@ -1996,28 +1996,27 @@ done:
  * (found by hostname) in AD.
  * @param ads An initialized ADS_STRUCT
  * @param machine_name the NetBIOS name of the computer, which is used to identify the computer account.
- * @param my_fqdn The fully qualified DNS name of the machine
- * @param spn A string of the service principal to add, i.e. 'host'
+ * @param spns An array or strings for the service principals to add,
+ *        i.e. 'cifs/machine_name', 'http/machine.full.domain.com' etc.
  * @return 0 upon sucess, or non-zero if a failure occurs
  **/
 
-ADS_STATUS ads_add_service_principal_name(ADS_STRUCT *ads, const char *machine_name, 
-                                          const char *my_fqdn, const char *spn)
+ADS_STATUS ads_add_service_principal_names(ADS_STRUCT *ads,
+					   const char *machine_name,
+                                           const char **spns)
 {
 	ADS_STATUS ret;
 	TALLOC_CTX *ctx;
 	LDAPMessage *res = NULL;
-	char *psp1, *psp2;
 	ADS_MODLIST mods;
 	char *dn_string = NULL;
-	const char *servicePrincipalName[3] = {NULL, NULL, NULL};
+	const char **servicePrincipalName = spns;
 
 	ret = ads_find_machine_acct(ads, &res, machine_name);
 	if (!ADS_ERR_OK(ret) || ads_count_replies(ads, res) != 1) {
 		DEBUG(1,("ads_add_service_principal_name: WARNING: Host Account for %s not found... skipping operation.\n",
 			machine_name));
-		DEBUG(1,("ads_add_service_principal_name: WARNING: Service Principal '%s/%s@%s' has NOT been added.\n",
-			spn, machine_name, ads->config.realm));
+		DEBUG(1,("ads_add_service_principal_name: WARNING: Service Principals have NOT been added.\n"));
 		ads_msgfree(ads, res);
 		return ADS_ERROR(LDAP_NO_SUCH_OBJECT);
 	}
@@ -2028,44 +2027,24 @@ ADS_STATUS ads_add_service_principal_name(ADS_STRUCT *ads, const char *machine_n
 		return ADS_ERROR(LDAP_NO_MEMORY);
 	}
 
-	/* add short name spn */
-
-	if ( (psp1 = talloc_asprintf(ctx, "%s/%s", spn, machine_name)) == NULL ) {
-		talloc_destroy(ctx);
-		ads_msgfree(ads, res);
-		return ADS_ERROR(LDAP_NO_MEMORY);
-	}
-	if (!strlower_m(&psp1[strlen(spn) + 1])) {
-		ret = ADS_ERROR(LDAP_NO_MEMORY);
-		goto out;
-	}
-	servicePrincipalName[0] = psp1;
-
-	DEBUG(5,("ads_add_service_principal_name: INFO: Adding %s to host %s\n", 
-		psp1, machine_name));
-
-
-	/* add fully qualified spn */
+	DEBUG(5,("ads_add_service_principal_name: INFO: "
+		"Adding %s to host %s\n",
+		spns[0] ? "N/A" : spns[0], machine_name));
 
-	if ( (psp2 = talloc_asprintf(ctx, "%s/%s", spn, my_fqdn)) == NULL ) {
-		ret = ADS_ERROR(LDAP_NO_MEMORY);
-		goto out;
-	}
-	if (!strlower_m(&psp2[strlen(spn) + 1])) {
-		ret = ADS_ERROR(LDAP_NO_MEMORY);
-		goto out;
-	}
-	servicePrincipalName[1] = psp2;
 
-	DEBUG(5,("ads_add_service_principal_name: INFO: Adding %s to host %s\n", 
-		psp2, machine_name));
+	DEBUG(5,("ads_add_service_principal_name: INFO: "
+		"Adding %s to host %s\n",
+		spns[1] ? "N/A" : spns[1], machine_name));
 
 	if ( (mods = ads_init_mods(ctx)) == NULL ) {
 		ret = ADS_ERROR(LDAP_NO_MEMORY);
 		goto out;
 	}
 
-	ret = ads_add_strlist(ctx, &mods, "servicePrincipalName", servicePrincipalName);
+	ret = ads_add_strlist(ctx,
+			      &mods,
+			      "servicePrincipalName",
+			      servicePrincipalName);
 	if (!ADS_ERR_OK(ret)) {
 		DEBUG(1,("ads_add_service_principal_name: Error: Updating Service Principals in LDAP\n"));
 		goto out;