PATCHv2: adjust 'net ads add keytab' for windows SPN(s) & add new 'net ads setspn' subcommand

Jeremy Allison jra at samba.org
Wed Feb 28 00:13:33 UTC 2018


On Tue, Feb 27, 2018 at 11:18:19AM +0000, Noel Power via samba-technical wrote:
> Thanks Andreas,
> 

OK, still needs some work.

Fistly - change all occurrences of 'principle' -> 'principal'

The rest I'll cover inline:

> From 0a7f1723284d2100039b4f86fb5055e242fa48a2 Mon Sep 17 00:00:00 2001
> From: Noel Power <noel.power at suse.com>
> Date: Fri, 12 Jan 2018 14:22:34 +0000
> Subject: [PATCH 02/19] s3/libads: Clean up code a little rename
>  'ads_get_samaccountname()'
> 
> Function 'ads_get_samaccountname()' basically returns the machine_name passed
> as an input param (appended with '$') if it exists on the ad. The function
> really is testing for the existence of the samaccountname and is not really
> 'getting' it. This is also the way it is used. Renaming this function to
> 'ads_has_samaccountname()' better reflects what it is actually doing and how
> clients calling the code use it. It also makes the client code using calling
> this function less confusing.
> 
> Signed-off-by: Noel Power <noel.power at suse.com>
> ---
>  source3/libads/ads_proto.h       |  2 +-
>  source3/libads/kerberos_keytab.c | 23 +++++++++++++++--------
>  source3/libads/ldap.c            | 11 ++++++++---
>  3 files changed, 24 insertions(+), 12 deletions(-)
> 
> diff --git a/source3/libads/ads_proto.h b/source3/libads/ads_proto.h
> index b6d9d9b31fc..8e9a7690a0f 100644
> --- a/source3/libads/ads_proto.h
> +++ b/source3/libads/ads_proto.h
> @@ -121,7 +121,7 @@ ADS_STATUS ads_get_sid_from_extended_dn(TALLOC_CTX *mem_ctx,
>  					struct dom_sid *sid);
>  char* ads_get_dnshostname( ADS_STRUCT *ads, TALLOC_CTX *ctx, const char *machine_name );
>  char* ads_get_upn( ADS_STRUCT *ads, TALLOC_CTX *ctx, const char *machine_name );
> -char* ads_get_samaccountname( ADS_STRUCT *ads, TALLOC_CTX *ctx, const char *machine_name );
> +bool ads_has_samaccountname( ADS_STRUCT *ads, TALLOC_CTX *ctx, const char *machine_name );
>  ADS_STATUS ads_join_realm(ADS_STRUCT *ads, const char *machine_name,
>  			uint32_t account_type, const char *org_unit);
>  ADS_STATUS ads_leave_realm(ADS_STRUCT *ads, const char *hostname);
> diff --git a/source3/libads/kerberos_keytab.c b/source3/libads/kerberos_keytab.c
> index 37ac7ba985e..df9ed03a1ad 100644
> --- a/source3/libads/kerberos_keytab.c
> +++ b/source3/libads/kerberos_keytab.c
> @@ -114,7 +114,6 @@ int ads_keytab_add_entry(ADS_STRUCT *ads, const char *srvPrinc)
>  	char *password_s = NULL;
>  	char *my_fqdn;
>  	TALLOC_CTX *tmpctx = NULL;
> -	char *machine_name;
>  	ADS_STATUS aderr;
>  	int i;
>  
> @@ -163,15 +162,13 @@ int ads_keytab_add_entry(ADS_STRUCT *ads, const char *srvPrinc)
>  		goto out;
>  	}
>  
> -	machine_name = ads_get_samaccountname(ads, tmpctx, lp_netbios_name());
> -	if (!machine_name) {
> +	/* make sure we have a single instance of a the computer account */
> +	if (!ads_has_samaccountname(ads, tmpctx, lp_netbios_name())) {
>  		DEBUG(0, (__location__ ": unable to determine machine "
>  			  "account's short name in AD!\n"));
>  		ret = -1;
>  		goto out;
>  	}
> -	/*strip the trailing '$' */
> -	machine_name[strlen(machine_name)-1] = '\0';
>  
>  	/* Construct our principal */
>  	if (strchr_m(srvPrinc, '@')) {
> @@ -201,7 +198,7 @@ int ads_keytab_add_entry(ADS_STRUCT *ads, const char *srvPrinc)
>  			goto out;
>  		}
>  		short_princ_s = talloc_asprintf(tmpctx, "%s/%s@%s",
> -						srvPrinc, machine_name,
> +						srvPrinc, lp_netbios_name(),
>  						lp_realm());
>  		if (short_princ_s == NULL) {
>  			ret = -1;
> @@ -375,6 +372,7 @@ int ads_keytab_create_default(ADS_STRUCT *ads)
>  	char **spn_array;
>  	size_t num_spns;
>  	size_t i;
> +	bool ok = false;
>  	ADS_STATUS status;
>  
>  	ZERO_STRUCT(kt_entry);
> @@ -451,14 +449,23 @@ int ads_keytab_create_default(ADS_STRUCT *ads)
>  	}
>  
>  	/* now add the userPrincipalName and sAMAccountName entries */
> -	sam_account_name = ads_get_samaccountname(ads, frame, machine_name);
> -	if (!sam_account_name) {
> +	ok = ads_has_samaccountname(ads, frame, machine_name);
> +	if (!ok) {
>  		DEBUG(0, (__location__ ": unable to determine machine "
>  			  "account's name in AD!\n"));
>  		ret = -1;
>  		goto done;
>  	}
>  
> +	/*
> +	 * append '$' to netbios name so 'ads_keytab_add_entry' recognises
> +	 * it as a machine account rather than a service or Windows SPN.
> +	 */
> +	sam_account_name = talloc_asprintf(frame, "%s$",machine_name);
> +	if (sam_account_name == NULL) {
> +		ret = -1;
> +		goto done;
> +	}
>  	/* upper case the sAMAccountName to make it easier for apps to
>  	   know what case to use in the keytab file */
>  	if (!strupper_m(sam_account_name)) {
> diff --git a/source3/libads/ldap.c b/source3/libads/ldap.c
> index 4f238ef83c2..739cfb477af 100644
> --- a/source3/libads/ldap.c
> +++ b/source3/libads/ldap.c
> @@ -3463,12 +3463,13 @@ out:
>  /********************************************************************
>  ********************************************************************/
>  
> -char* ads_get_samaccountname( ADS_STRUCT *ads, TALLOC_CTX *ctx, const char *machine_name )
> +bool ads_has_samaccountname( ADS_STRUCT *ads, TALLOC_CTX *ctx, const char *machine_name )
>  {
>  	LDAPMessage *res = NULL;
>  	ADS_STATUS status;
>  	int count = 0;
>  	char *name = NULL;
> +	bool ok = false;
>  
>  	status = ads_find_machine_acct(ads, &res, machine_name);
>  	if (!ADS_ERR_OK(status)) {
> @@ -3488,8 +3489,12 @@ char* ads_get_samaccountname( ADS_STRUCT *ads, TALLOC_CTX *ctx, const char *mach
>  
>  out:
>  	ads_msgfree(ads, res);
> -
> -	return name;
> +	if (name != NULL) {
> +		ok = (strlen(name) > 0);

The else clause is redundant - remove it.

> +	} else {
> +		ok = false;
> +	}
> +	return ok;
>  }
>  
>  #if 0
> -- 
> 2.13.6
> 
> 
> From 6e83513b288ddd6e598e578650200ed2a9773e3d Mon Sep 17 00:00:00 2001
> From: Noel Power <noel.power at suse.com>
> Date: Mon, 29 Jan 2018 17:51:15 +0000
> Subject: [PATCH 03/19] s3/libads: Add a basic Windows SPN parser.
> 
> (see https://social.technet.microsoft.com/wiki/contents/articles/717.service-principal-names-spns-setspn-syntax-setspn-exe.aspx)
> 
> Signed-off-by: Noel Power <noel.power at suse.com>
> ---
>  source3/libads/ads_proto.h | 10 +++++
>  source3/libads/util.c      | 91 ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 101 insertions(+)
> 
> diff --git a/source3/libads/ads_proto.h b/source3/libads/ads_proto.h
> index 8e9a7690a0f..a35f211c7d3 100644
> --- a/source3/libads/ads_proto.h
> +++ b/source3/libads/ads_proto.h
> @@ -193,4 +193,14 @@ void ndr_print_ads_saslwrap_struct(struct ndr_print *ndr,
>  
>  ADS_STATUS ads_change_trust_account_password(ADS_STRUCT *ads, char *host_principal);
>  
> +struct spn_struct {
> +	const char *serviceclass;
> +	const char *servicename;
> +	const char *host;
> +	int32_t port;
> +};
> +
> +/* parse a windows style SPN, returns NULL if parsing fails */
> +struct spn_struct *parse_spn(TALLOC_CTX *ctx, const char *srvprinc);
> +
>  #endif /* _LIBADS_ADS_PROTO_H_ */
> diff --git a/source3/libads/util.c b/source3/libads/util.c
> index 14dbf86b21a..9ddb8d6b687 100644
> --- a/source3/libads/util.c
> +++ b/source3/libads/util.c
> @@ -133,3 +133,94 @@ ADS_STATUS ads_change_trust_account_password(ADS_STRUCT *ads, char *host_princip
>  	return ADS_SUCCESS;
>  }
>  #endif
> +
> +/**
> +* @brief Parses windows style SPN service/host:port/servicename
> +*      serviceclass - A string that identifies the general class of service
> +*            e.g. 'http'
> +*      host - A netbios name or fully-qualified DNS name
> +*      port - An optional TCP or UDP port number
> +*      servicename - An optional distinguished name, GUID, DNS name or
> +*                    DNS name of an SRV or MX record. (not needed for host
> +*                    based services)
> +*
> +* @param[in]  ctx 	- Talloc context.
> +* @param[in]  srvprinc  - The service principle
> +*
> +* @return 		- struct spn_struct containing the fields parsed or NULL
> +*			  if srvprinc could not be parsed.
> +*/
> +struct spn_struct *parse_spn(TALLOC_CTX *ctx, const char *srvprinc)
> +{
> +	struct spn_struct * result = talloc_zero(ctx, struct spn_struct);

talloc must check for NULL.

> +	char *tmp = NULL;
> +	char *port_str = NULL;
> +	char *host_str = NULL;
> +	result->port = -1;
> +	result->serviceclass = talloc_strdup(result, srvprinc);

talloc must check for NULL.
> +
> +	tmp = strchr_m(result->serviceclass, '/');
> +	if (tmp == NULL) {
> +		/* illegal */
> +		DBG_ERR("failed to parse spn %s,"
> +			" no host definition\n", srvprinc);
> +		TALLOC_FREE(result);
> +		goto out;
> +	}
> +
> +	*tmp = '\0'; /* terminate service principle*/
> +	tmp++;
> +	host_str = tmp;
> +
> +	tmp = strchr_m(host_str, ':');
> +	if (tmp != NULL) {
> +		*tmp  = '\0';
> +		tmp++;
> +		port_str = tmp;
> +	} else {
> +		tmp = host_str;
> +	}
> +
> +	tmp = strchr_m(tmp, '/');
> +	if (tmp != NULL) {
> +		*tmp  = '\0';
> +		tmp++;
> +		result->servicename = tmp;
> +	}
> +
> +	if (strlen(host_str) == 0) {
> +		/* illegal */
> +		DBG_ERR("failed to parse spn %s,"
> +			" illegal host definition\n", srvprinc);
> +		TALLOC_FREE(result);
> +		goto out;
> +	}
> +	result->host = host_str;
> +
> +	if (result->servicename != NULL && (strlen(result->servicename) == 0)) {
> +		DBG_ERR("failed to parse spn %s,"
> +			" empty servicename definition\n", srvprinc);
> +		TALLOC_FREE(result);
> +		goto out;
> +	}
> +	if (port_str != NULL) {
> +		if (strlen(port_str) == 0) {
> +			DBG_ERR("failed to parse spn %s,"
> +				" empty port definition\n", srvprinc);
> +			TALLOC_FREE(result);
> +			goto out;
> +		}
> +		result->port = (int32_t)strtol(port_str, NULL, 10);
> +		if (result->port <= 0
> +		    || result->port > 65535
> +		    || errno == ERANGE) {
> +			DBG_ERR("failed to parse spn %s,"
> +				" port number convertion failed\n", srvprinc);
> +			errno = 0;
> +			TALLOC_FREE(result);
> +			goto out;
> +		}
> +	}
> +out:
> +	return result;
> +}
> -- 
> 2.13.6
> 
> 
> From 22388e0f37b939b6d42f67da60f026a11937f0e6 Mon Sep 17 00:00:00 2001
> From: Noel Power <noel.power at suse.com>
> Date: Fri, 16 Feb 2018 16:52:01 +0000
> Subject: [PATCH 04/19] 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 at suse.com>
> ---
>  source3/libads/ads_proto.h       |  4 +-
>  source3/libads/kerberos_keytab.c | 81 +++++++++++++++++++++++++++++++++++-----
>  source3/libads/ldap.c            | 55 +++++++++------------------
>  3 files changed, 91 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..00c3a89950e 100644
> --- a/source3/libads/kerberos_keytab.c
> +++ b/source3/libads/kerberos_keytab.c
> @@ -85,6 +85,73 @@ 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,

Return ADS_STATUS here, otherwise you're losing a
error code that might be useful higher up.

> +					 ADS_STRUCT *ads,
> +					 const char *service_or_spn,
> +					 const char *my_fqdn)
> +{
> +	const char **spn_names = NULL;
> +	ADS_STATUS aderr;
> +	bool ok = false;
> +
> +	DEBUG(3, (__location__ ": 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)) {
> +		DEBUG(1, (__location__ ": failed to "
> +			"ads_add_service_principal_name.\n"));
> +		return false;
> +	}
> +	return true;
> +}
> +
>  /**********************************************************************
>   Adds a single service principal, i.e. 'host' to the system keytab
>  ***********************************************************************/
> @@ -114,7 +181,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 +278,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 739cfb477af..369b20a6d3c 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;
> -- 
> 2.13.6
> 
> 
> From dcba6de01909137113fbf9c1555632b46824882b Mon Sep 17 00:00:00 2001
> From: Noel Power <noel.power at suse.com>
> Date: Wed, 24 Jan 2018 14:26:03 +0000
> Subject: [PATCH 05/19] s3/utils: add new 'net ads setspn list' subcommand
> 
> This patch adds basic functionality not unlike the setspn.exe
> command that is provided by windows for adminsistering SPN on
> the AD. (see https://docs.microsoft.com/en-us/previous-versions/windows/it-pro/windows-server-2012-R2-and-2012/cc731241(v=ws.11)
> 
> Only the basic list operation (that corresponds to the -l
>     switch for setspn.exe is implemented)
> 
> Usage:
> 
>      net ads setspn list <computer>
> 
> Note: <computer> is optional, if not specified the computer account
> associated with value returned by lp_netbios_name() is used instead.
> 
> Signed-off-by: Noel Power <noel.power at suse.com>
> ---
>  source3/libads/ads_proto.h      |  3 +++
>  source3/libads/net_ads_setspn.c | 57 ++++++++++++++++++++++++++++++++++++++++
>  source3/utils/net_ads.c         | 58 +++++++++++++++++++++++++++++++++++++++++
>  source3/utils/net_proto.h       |  1 +
>  source3/wscript_build           |  1 +
>  5 files changed, 120 insertions(+)
>  create mode 100644 source3/libads/net_ads_setspn.c
> 
> diff --git a/source3/libads/ads_proto.h b/source3/libads/ads_proto.h
> index 258162d7d2a..414e677009b 100644
> --- a/source3/libads/ads_proto.h
> +++ b/source3/libads/ads_proto.h
> @@ -54,6 +54,9 @@ int ads_keytab_flush(ADS_STRUCT *ads);
>  int ads_keytab_create_default(ADS_STRUCT *ads);
>  int ads_keytab_list(const char *keytab_name);
>  
> +/* The following definitions come from libads/net_ads_setspn.c  */
> +int ads_setspn_list(ADS_STRUCT *ads, const char *machine);
> +
>  /* The following definitions come from libads/krb5_errs.c  */
>  
>  /* The following definitions come from libads/kerberos_util.c  */
> diff --git a/source3/libads/net_ads_setspn.c b/source3/libads/net_ads_setspn.c
> new file mode 100644
> index 00000000000..3ffc789b16e
> --- /dev/null
> +++ b/source3/libads/net_ads_setspn.c
> @@ -0,0 +1,57 @@
> +/*
> +   Unix SMB/CIFS implementation.
> +   net ads setspn routines
> +   Copyright (C) Noel Power 2018
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +*/
> +
> +#include "includes.h"
> +#include "ads.h"
> +
> +#ifdef HAVE_ADS
> +int ads_setspn_list(ADS_STRUCT *ads, const char *machine_name)
> +{
> +	int ret = -1;
> +	int i = 0;
> +	TALLOC_CTX *frame = NULL;
> +	char **spn_array = NULL;
> +	size_t num_spns = 0;
> +	ADS_STATUS status;
> +
> +	frame = talloc_stackframe();

Actually, talloc_stackframe() is the *one* talloc case
where you can ignore an error, as it'll abort if it
fails :-).

> +	if (frame == NULL) {
> +		ret = -1;
> +		goto done;
> +	}
> +	status = ads_get_service_principal_names(frame,
> +						 ads,
> +						 machine_name,
> +						 &spn_array,
> +						 &num_spns);
> +	if (!ADS_ERR_OK(status)) {
> +		ret = -1;
> +		goto done;
> +	}
> +	d_printf("Registered SPNs for %s\n", machine_name);
> +	for (i = 0; i < num_spns; i++) {
> +		d_printf("\t%s\n", spn_array[i]);
> +	}
> +	ret = 0;
> +done:
> +	TALLOC_FREE(frame);
> +	return ret;
> +}
> +
> +#endif /* HAVE_ADS */
> diff --git a/source3/utils/net_ads.c b/source3/utils/net_ads.c
> index c83aced9f81..1e3663f7bcb 100644
> --- a/source3/utils/net_ads.c
> +++ b/source3/utils/net_ads.c
> @@ -3005,6 +3005,51 @@ int net_ads_kerberos(struct net_context *c, int argc, const char **argv)
>  	return net_run_function(c, argc, argv, "net ads kerberos", func);
>  }
>  
> +static int net_ads_setspn_list(struct net_context *c, int argc, const char **argv)
> +{
> +	int ret = 0;
> +	ADS_STRUCT *ads = NULL;
> +	if (c->display_usage) {
> +		d_printf("%s\n%s",
> +			 _("Usage:"),
> +			 _("net ads setspn list <machinename>\n"));
> +		ret = 0;
> +		goto done;
> +	}
> +	if (!ADS_ERR_OK(ads_startup(c, true, &ads))) {
> +		ret = -1;
> +		goto done;
> +	}
> +	if (argc) {
> +		ret |= ads_setspn_list(ads, argv[0]);
> +	} else {
> +		ret |= ads_setspn_list(ads, lp_netbios_name());
> +	}
> +
> +done:
> +	if (ads) {
> +		ads_destroy(&ads);
> +	}
> +	return ret;
> +}
> +
> +int net_ads_setspn(struct net_context *c, int argc, const char **argv)
> +{
> +	struct functable func[] = {
> +		{
> +			"list",
> +			net_ads_setspn_list,
> +			NET_TRANSPORT_ADS,
> +			N_("List Service Principal Names (SPN)"),
> +			N_("net ads setspn list machine\n"
> +			   "    List Service Principal Names (SPN)")
> +		},
> +		{NULL, NULL, 0, NULL, NULL}
> +	};
> +
> +	return net_run_function(c, argc, argv, "net ads setspn", func);
> +}
> +
>  static int net_ads_enctype_lookup_account(struct net_context *c,
>  					  ADS_STRUCT *ads,
>  					  const char *account,
> @@ -3444,6 +3489,14 @@ int net_ads(struct net_context *c, int argc, const char **argv)
>  			   "    Manage local keytab file")
>  		},
>  		{
> +			"setspn",
> +			net_ads_setspn,
> +			NET_TRANSPORT_ADS,
> +			N_("Manage Service Principal Names (SPN)s"),
> +			N_("net ads spnset\n"
> +			   "    Manage Service Principal Names (SPN)s")
> +		},
> +		{
>  			"gpo",
>  			net_ads_gpo,
>  			NET_TRANSPORT_ADS,
> @@ -3491,6 +3544,11 @@ int net_ads_kerberos(struct net_context *c, int argc, const char **argv)
>  	return net_ads_noads();
>  }
>  
> +int net_ads_setspn(struct net_context *c, int argc, const char **argv)
> +{
> +	return net_ads_noads();
> +}
> +
>  int net_ads_changetrustpw(struct net_context *c, int argc, const char **argv)
>  {
>  	return net_ads_noads();
> diff --git a/source3/utils/net_proto.h b/source3/utils/net_proto.h
> index 293685c18d4..22fe39e0f1c 100644
> --- a/source3/utils/net_proto.h
> +++ b/source3/utils/net_proto.h
> @@ -43,6 +43,7 @@ int net_ads_printer_usage(struct net_context *c, int argc, const char **argv);
>  int net_ads_changetrustpw(struct net_context *c, int argc, const char **argv);
>  int net_ads_keytab(struct net_context *c, int argc, const char **argv);
>  int net_ads_kerberos(struct net_context *c, int argc, const char **argv);
> +int net_ads_setspn(struct net_context *c, int argc, const char **argv);
>  int net_ads(struct net_context *c, int argc, const char **argv);
>  
>  /* The following definitions come from utils/net_ads_gpo.c  */
> diff --git a/source3/wscript_build b/source3/wscript_build
> index d5ac7a280cb..123f122a22d 100644
> --- a/source3/wscript_build
> +++ b/source3/wscript_build
> @@ -522,6 +522,7 @@ bld.SAMBA3_LIBRARY('ads',
>                            libads/ldap_schema.c
>                            libads/util.c
>                            libads/ndr.c
> +                          libads/net_ads_setspn.c
>                            ''',
>                     deps='''
>                          cli-ldap-common
> -- 
> 2.13.6
> 
> 
> From 7c83e834943a2541e15d3c3cd9c9a9d606afc6fb Mon Sep 17 00:00:00 2001
> From: Noel Power <noel.power at suse.com>
> Date: Wed, 24 Jan 2018 14:41:06 +0000
> Subject: [PATCH 06/19] s3/utils: add new 'net ads setspn add' subcommand
> 
> This patch adds 'add' to the 'net ads setspn' subcommand
> 
> (see https://docs.microsoft.com/en-us/previous-versions/windows/it-pro/windows-server-2012-R2-and-2012/cc731241(v=ws.11)
> 
> Usage:
> 
>      net ads setspn add <computer> <SPN>
> 
> Note: <computer> is optional, if not specified the computer account
> associated with value returned by lp_netbios_name() is used instead.
> 
> Signed-off-by: Noel Power <noel.power at suse.com>
> ---
>  source3/libads/ads_proto.h      |  3 +-
>  source3/libads/net_ads_setspn.c | 83 +++++++++++++++++++++++++++++++++++++++++
>  source3/utils/net_ads.c         | 36 ++++++++++++++++++
>  3 files changed, 121 insertions(+), 1 deletion(-)
> 
> diff --git a/source3/libads/ads_proto.h b/source3/libads/ads_proto.h
> index 414e677009b..8e714c8a34d 100644
> --- a/source3/libads/ads_proto.h
> +++ b/source3/libads/ads_proto.h
> @@ -56,7 +56,8 @@ int ads_keytab_list(const char *keytab_name);
>  
>  /* The following definitions come from libads/net_ads_setspn.c  */
>  int ads_setspn_list(ADS_STRUCT *ads, const char *machine);
> -
> +int ads_setspn_add(ADS_STRUCT *ads, const char *machine_name,
> +		   const char * spn);
>  /* The following definitions come from libads/krb5_errs.c  */
>  
>  /* The following definitions come from libads/kerberos_util.c  */
> diff --git a/source3/libads/net_ads_setspn.c b/source3/libads/net_ads_setspn.c
> index 3ffc789b16e..e3af3f7066a 100644
> --- a/source3/libads/net_ads_setspn.c
> +++ b/source3/libads/net_ads_setspn.c
> @@ -54,4 +54,87 @@ done:
>  	return ret;
>  }
>  
> +/* returns true if spn exists in spn_array (match is NOT case-sensitive) */
> +static bool find_spn_in_spnlist(TALLOC_CTX *ctx,
> +				const char *spn,
> +				char **spn_array,
> +				size_t num_spns)
> +{
> +	int i = 0;
> +	char *lc_spn = strlower_talloc(ctx, spn);
> +	if (lc_spn == NULL) {
> +		DBG_ERR("Out of memory, lowercasing %s.\n",
> +			spn);
> +		return false;
> +	}
> +	for (i = 0; i < num_spns; i++) {
> +		char *lc_spn_attr = strlower_talloc(ctx, spn_array[i]);
> +		if (lc_spn_attr == NULL) {
> +			DBG_ERR("Out of memory, lowercasing %s.\n",
> +				spn_array[i]);
> +			return false;
> +		}
> +
> +		if (strequal(lc_spn, lc_spn_attr)) {
> +			return true;
> +		}
> +	}
> +	return false;
> +}
> +
> +int ads_setspn_add(ADS_STRUCT *ads, const char *machine_name, const char * spn)
> +{
> +	int ret = -1;
> +	TALLOC_CTX *frame = NULL;
> +	ADS_STATUS status;
> +	struct spn_struct *spn_struct = NULL;
> +	const char *spns[2] = {NULL, NULL};
> +	char **existing_spns = NULL;
> +	size_t num_spns = 0;
> +	bool found = false;
> +
> +	frame = talloc_stackframe();

Actually, talloc_stackframe() is the *one* talloc case
where you can ignore an error, as it'll abort if it
fails :-).

> +	DEBUG(0,("setspn add"));
> +	if (frame == NULL) {
> +		ret = -1;
> +		goto done;
> +	}
> +	spns[0] = spn;
> +	spn_struct = parse_spn(frame, spn);
> +	if (spn_struct == NULL) {
> +		ret = -1;
> +		goto done;
> +	}
> +
> +	status = ads_get_service_principal_names(frame,
> +						 ads,
> +						 machine_name,
> +						 &existing_spns,
> +						 &num_spns);
> +
> +	if (!ADS_ERR_OK(status)) {
> +		ret = -1;
> +		goto done;
> +	}
> +
> +	found = find_spn_in_spnlist(frame, spn, existing_spns, num_spns);
> +	if (found) {
> +		d_printf("Duplicate SPN found, aborting operation.\n");
> +		ret = -1;
> +		goto done;
> +	}
> +
> +	d_printf("Registering SPN %s for object %s\n", spn, machine_name);
> +	status = ads_add_service_principal_names(ads, machine_name, spns);
> +	if (!ADS_ERR_OK(status)) {
> +		ret = -1;
> +		goto done;
> +	}
> +	ret = 0;
> +	d_printf("Updated object\n");
> +done:
> +	TALLOC_FREE(frame);
> +	return ret;
> +}
> +
>  #endif /* HAVE_ADS */
> diff --git a/source3/utils/net_ads.c b/source3/utils/net_ads.c
> index 1e3663f7bcb..6098f5faeee 100644
> --- a/source3/utils/net_ads.c
> +++ b/source3/utils/net_ads.c
> @@ -3033,6 +3033,34 @@ done:
>  	return ret;
>  }
>  
> +static int net_ads_setspn_add(struct net_context *c, int argc, const char **argv)
> +{
> +	int ret = 0;
> +	ADS_STRUCT *ads = NULL;
> +	if (c->display_usage || argc < 1) {
> +		d_printf("%s\n%s",
> +			 _("Usage:"),
> +			 _("net ads setspn add <machinename> SPN\n"));
> +		ret = 0;
> +		goto done;
> +	}
> +	if (!ADS_ERR_OK(ads_startup(c, true, &ads))) {
> +		ret = -1;
> +		goto done;
> +	}
> +	if (argc > 1) {

ads_setspn_add returns either 0 or -1. Why isn't this
a bool ? I'm not 100% convinced on the |= idiom for
a return value (I know the ldb code uses it a lot).

Only a nit-pick here really...

> +		ret |= ads_setspn_add(ads, argv[0], argv[1]);
> +	} else {
> +		ret |= ads_setspn_add(ads, lp_netbios_name(), argv[0]);
> +	}
> +
> +done:
> +	if (ads) {
> +		ads_destroy(&ads);
> +	}
> +	return ret;
> +}
> +
>  int net_ads_setspn(struct net_context *c, int argc, const char **argv)
>  {
>  	struct functable func[] = {
> @@ -3044,6 +3072,14 @@ int net_ads_setspn(struct net_context *c, int argc, const char **argv)
>  			N_("net ads setspn list machine\n"
>  			   "    List Service Principal Names (SPN)")
>  		},
> +		{
> +			"add",
> +			net_ads_setspn_add,
> +			NET_TRANSPORT_ADS,
> +			N_("Add Service Principal Names (SPN)"),
> +			N_("net ads setspn add machine spn\n"
> +			   "    Add Service Principal Names (SPN)")
> +		},
>  		{NULL, NULL, 0, NULL, NULL}
>  	};
>  
> -- 
> 2.13.6
> 
> 
> From 9138b4e8e894bf1dbc17bc2e2db462d1417412f2 Mon Sep 17 00:00:00 2001
> From: Noel Power <noel.power at suse.com>
> Date: Wed, 24 Jan 2018 14:51:03 +0000
> Subject: [PATCH 07/19] s3/utils: add new 'net ads setspn delete' subcommand
> 
> This patch adds 'delete' to the 'net ads setspn' subcommand
> 
> (see https://docs.microsoft.com/en-us/previous-versions/windows/it-pro/windows-server-2012-R2-and-2012/cc731241(v=ws.11)
> 
> Usage:
> 
>     net ads setspn delete <computer> <SPN>
> 
> Note: <computer> is optional, if not specified the computer account
> associated with value returned by lp_netbios_name() is used instead.
> 
> Signed-off-by: Noel Power <noel.power at suse.com>
> ---
>  source3/libads/ads_proto.h      |   3 ++
>  source3/libads/net_ads_setspn.c | 112 ++++++++++++++++++++++++++++++++++++++++
>  source3/utils/net_ads.c         |  36 +++++++++++++
>  3 files changed, 151 insertions(+)
> 
> diff --git a/source3/libads/ads_proto.h b/source3/libads/ads_proto.h
> index 8e714c8a34d..47e250a23df 100644
> --- a/source3/libads/ads_proto.h
> +++ b/source3/libads/ads_proto.h
> @@ -58,6 +58,9 @@ int ads_keytab_list(const char *keytab_name);
>  int ads_setspn_list(ADS_STRUCT *ads, const char *machine);
>  int ads_setspn_add(ADS_STRUCT *ads, const char *machine_name,
>  		   const char * spn);
> +int ads_setspn_delete(ADS_STRUCT *ads, const char *machine_name,
> +		      const char * spn);
> +
>  /* The following definitions come from libads/krb5_errs.c  */
>  
>  /* The following definitions come from libads/kerberos_util.c  */
> diff --git a/source3/libads/net_ads_setspn.c b/source3/libads/net_ads_setspn.c
> index e3af3f7066a..f8b3684caf2 100644
> --- a/source3/libads/net_ads_setspn.c
> +++ b/source3/libads/net_ads_setspn.c
> @@ -137,4 +137,116 @@ done:
>  	return ret;
>  }
>  
> +int ads_setspn_delete(ADS_STRUCT *ads,
> +		const char *machine_name,
> +		const char * spn)
> +{
> +	int ret = -1;
> +	int i = 0, j = 0;
> +	TALLOC_CTX *frame = NULL;
> +	char **spn_array = NULL;
> +	const char **new_spn_array = NULL;
> +	char *lc_spn = NULL;
> +	size_t num_spns = 0;
> +	ADS_STATUS status;
> +	ADS_MODLIST mods;
> +	LDAPMessage *res = NULL;
> +
> +	frame = talloc_stackframe();

Actually, talloc_stackframe() is the *one* talloc case
where you can ignore an error, as it'll abort if it
fails :-).

> +	if (frame == NULL) {
> +		DBG_ERR("Out of memory, failed to allocate stackframe.\n");
> +		ret = -1;
> +		goto done;
> +	}
> +
> +	lc_spn = strlower_talloc(frame, spn);
> +	if (lc_spn == NULL) {
> +		DBG_ERR("Out of memory, lowercasing %s.\n", spn);
> +		ret = -1;
> +		goto done;
> +	}
> +
> +	status = ads_find_machine_acct(ads,
> +				       &res,
> +				       machine_name);
> +
> +	if (!ADS_ERR_OK(status)) {
> +		ret = -1;
> +		goto done;
> +	}
> +
> +	status = ads_get_service_principal_names(frame,
> +						 ads,
> +						 machine_name,
> +						 &spn_array,
> +						 &num_spns);
> +	if (!ADS_ERR_OK(status)) {
> +		ret = -1;
> +		goto done;
> +	}
> +
> +	new_spn_array = talloc_zero_array(frame, const char*, num_spns + 1);
> +
> +	if (!new_spn_array) {
> +		DBG_ERR("Out of memory, failed to allocate array.\n");
> +		ret = -1;
> +		goto done;
> +	}
> +
> +	/*
> +	 * create new spn list to write to object (excluding the spn to
> +	 * be deleted).
> +	 */
> +	for (i = 0, j = 0; i < num_spns; i++) {
> +		/*
> +		 * windows setspn.exe deletes matching spn in a case
> +		 * insensitive way.
> +		 */
> +		char *lc_spn_attr = strlower_talloc(frame, spn_array[i]);
> +		if (lc_spn_attr == NULL) {
> +			DBG_ERR("Out of memory, lowercasing %s.\n",
> +				spn_array[i]);
> +			ret = -1;
> +			goto done;
> +		}
> +
> +		if (!strequal(lc_spn, lc_spn_attr)) {
> +			new_spn_array[j++] = spn_array[i];
> +		}
> +	}
> +
> +	/* found and removed spn */
> +	if (j < num_spns) {
> +		char *dn = NULL;
> +		mods = ads_init_mods(frame);
> +		if (mods == NULL) {
> +			ret = -1;
> +			goto done;
> +		}
> +		d_printf("Unregistering SPN %s for %s\n", spn, machine_name);
> +		status = ads_mod_strlist(frame, &mods, "servicePrincipalName", new_spn_array);
> +		if (!ADS_ERR_OK(status)) {
> +			ret = -1;
> +			goto done;
> +		}
> +
> +		dn = ads_get_dn(ads, frame, res);
> +		if (dn == NULL ) {
> +			ret = -1;
> +			goto done;
> +		}
> +
> +		status = ads_gen_mod(ads, dn, mods);
> +		if (!ADS_ERR_OK(status)) {
> +			ret = -1;
> +			goto done;
> +		}
> +	}
> +	d_printf("Updated object\n");
> +	ret = 0;
> +done:
> +	TALLOC_FREE(frame);
> +	return ret;
> +}
> +
>  #endif /* HAVE_ADS */
> diff --git a/source3/utils/net_ads.c b/source3/utils/net_ads.c
> index 6098f5faeee..169a6fe99db 100644
> --- a/source3/utils/net_ads.c
> +++ b/source3/utils/net_ads.c
> @@ -3061,6 +3061,34 @@ done:
>  	return ret;
>  }
>  
> +static int net_ads_setspn_delete(struct net_context *c, int argc, const char **argv)
> +{
> +	int ret = 0;
> +	ADS_STRUCT *ads = NULL;
> +	if (c->display_usage || argc < 1) {
> +		d_printf("%s\n%s",
> +			 _("Usage:"),
> +			 _("net ads setspn delete <machinename> SPN\n"));
> +		ret = 0;
> +		goto done;
> +	}
> +	if (!ADS_ERR_OK(ads_startup(c, true, &ads))) {
> +		ret = -1;
> +		goto done;
> +	}
> +	if (argc > 1) {
> +		ret |= ads_setspn_delete(ads, argv[0], argv[1]);
> +	} else {
> +		ret |= ads_setspn_delete(ads, lp_netbios_name(), argv[0]);
> +	}
> +
> +done:
> +	if (ads) {
> +		ads_destroy(&ads);
> +	}
> +	return ret;
> +}
> +
>  int net_ads_setspn(struct net_context *c, int argc, const char **argv)
>  {
>  	struct functable func[] = {
> @@ -3080,6 +3108,14 @@ int net_ads_setspn(struct net_context *c, int argc, const char **argv)
>  			N_("net ads setspn add machine spn\n"
>  			   "    Add Service Principal Names (SPN)")
>  		},
> +		{
> +			"delete",
> +			net_ads_setspn_delete,
> +			NET_TRANSPORT_ADS,
> +			N_("Delete Service Principal Names (SPN)"),
> +			N_("net ads setspn delete machine spn\n"
> +			   "    Delete Service Principal Names (SPN)")
> +		},
>  		{NULL, NULL, 0, NULL, NULL}
>  	};
>  
> -- 
> 2.13.6
> 
> 
> From af5caf5d5e527dd546ec0d14285d9a22669da454 Mon Sep 17 00:00:00 2001
> From: Noel Power <noel.power at suse.com>
> Date: Thu, 18 Jan 2018 11:30:50 +0000
> Subject: [PATCH 08/19] testprocs/blackbox: Add tests for net ads setspn
>  (add|delete|list)
> 
> Signed-off-by: Noel Power <noel.power at suse.com>
> ---
>  testprogs/blackbox/test_net_ads.sh | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/testprogs/blackbox/test_net_ads.sh b/testprogs/blackbox/test_net_ads.sh
> index 16f77f5e672..680367b6387 100755
> --- a/testprogs/blackbox/test_net_ads.sh
> +++ b/testprogs/blackbox/test_net_ads.sh
> @@ -33,6 +33,26 @@ testit "join" $VALGRIND $net_tool ads join -U$DC_USERNAME%$DC_PASSWORD || failed
>  
>  testit "testjoin" $VALGRIND $net_tool ads testjoin -kP || failed=`expr $failed + 1`
>  
> +netbios=$(grep "netbios name" $BASEDIR/$WORKDIR/client.conf | cut -f2 -d= | awk '{$1=$1};1')
> +
> +testit "test setspn list $netbios" $VALGRIND $net_tool ads setspn list $netbios -U$DC_USERNAME%$DC_PASSWORD || failed=`expr $failed + 1`
> +spn="foo"
> +testit_expect_failure "test setspn add illegal windows spn ($spn)" $VALGRIND $net_tool ads setspn add $spn -U$DC_USERNAME%$DC_PASSWORD || failed=`expr $failed + 1`
> +
> +spn="foo/somehost.domain.com"
> +testit "test setspn add ($spn)" $VALGRIND $net_tool ads setspn add $spn -U$DC_USERNAME%$DC_PASSWORD || failed=`expr $failed + 1`
> +
> +found=$($net_tool ads setspn list -U$DC_USERNAME%$DC_PASSWORD | grep $spn | wc -l)
> +testit "test setspn list shows the newly added spn ($spn)" test $found -eq 1 || failed=`expr $failed + 1`
> +
> +up_spn=$(echo $spn | tr '[:lower:]' '[:upper:]')
> +testit_expect_failure "test setspn add existing (case-insensitive) spn ($spn)" $VALGRIND $net_tool ads setspn add $up_spn -U$DC_USERNAME%$DC_PASSWORD || failed=`expr $failed + 1`
> +
> +testit "test setspn delete existing (case-insensitive) ($spn)" $VALGRIND $net_tool ads setspn delete $spn  -U$DC_USERNAME%$DC_PASSWORD || failed=`expr $failed + 1`
> +
> +found=$($net_tool ads setspn list  -U$DC_USERNAME%$DC_PASSWORD | grep $spn | wc -l)
> +testit "test setspn list shows the newly deleted spn ($spn) is gone" test $found -eq 0 || failed=`expr $failed + 1`
> +
>  testit "changetrustpw" $VALGRIND $net_tool ads changetrustpw || failed=`expr $failed + 1`
>  
>  testit "leave" $VALGRIND $net_tool ads leave -U$DC_USERNAME%$DC_PASSWORD || failed=`expr $failed + 1`
> -- 
> 2.13.6
> 
> 
> From ce53a80990be387bd7cd6f84eda6fff022eceaf4 Mon Sep 17 00:00:00 2001
> From: Noel Power <noel.power at suse.com>
> Date: Mon, 29 Jan 2018 18:30:33 +0000
> Subject: [PATCH 09/19] s3/libads: Allow 'net ads keytab add' handle Windows
>  SPN(s) part 1
> 
> This patch addresses how the windows SPN is converted into a kerberos
> priniciple to be written to the keytab file. A followup patch will
> deal with writing Window SPN(s) to the AD.
> 
> Before this change 'net ads keytab add' handled three scenarios
> 
> a) 'net ads keytab add param' is passed a fully qualified kerberos principle
>    (identified by the presence of '@' in param) In this scenario the keytab
>    file alone is updated with the principle contained in 'param'.
> b) 'net ads keytab add param'; is passed a machine name (identified by
>    the paramater ending with '$'). In this case the machine name
>    is converted to a kerberos principle with according to the recipe
>    'param at realm' where realm is determined by lp_realm().
> c) 'net ads keytab add param' is passed a service (e.g. nfs, http etc.)
>    In this scenario the param containing the service is first converted to
>    into 2 kerberos principles (long and short forms) according to the
>    following recipe
>       i) long form:  'param/fully_qualified_dns at realm'
>      ii) short form: 'param/netbios_name at realm'
>      where 'fully_qualified_dns is retrieved from 'dNSHostName' attribute of
>      'this' machines computer account on the AD.
>      The principles are written to the keytab file
>    Secondly 2 windows SPNs are generated from 'param' as follows
>       i) long form 'param/full_qualified_dns'
>      ii) short form 'param/netbios_name'
>    These SPNs are written to the AD computer account object
> 
> After this change a) & b) & c) will retain legacy behaviour except
> in the case of c) where if the 'param' passed to c) is a Windows SPN
> (e.g. conforming to format 'serviceclass/host:port'
>   i) 'param' will get converted to a kerberos principle (just a single one)
>      with the following recipe: 'serviceclass/host at realm' which will
>      be written to the keytab file. The SPN written to the AD is created
>      as before and the legacy behaviour is preserved.
> 
> Signed-off-by: Noel Power <noel.power at suse.com>
> ---
>  source3/libads/kerberos_keytab.c | 68 ++++++++++++++++++++++++++++++++++------
>  1 file changed, 58 insertions(+), 10 deletions(-)
> 
> diff --git a/source3/libads/kerberos_keytab.c b/source3/libads/kerberos_keytab.c
> index 00c3a89950e..c76228dc463 100644
> --- a/source3/libads/kerberos_keytab.c
> +++ b/source3/libads/kerberos_keytab.c
> @@ -152,6 +152,59 @@ static bool ads_set_machine_account_spns(TALLOC_CTX *ctx,
>  	return true;
>  }
>  
> +/*
> + * Create kerberos principle(s) from SPN or service name.
> + */
> +static bool service_or_spn_to_kerberos_princ(TALLOC_CTX *ctx,
> +					     const char *service_or_spn,
> +					     const char *my_fqdn,
> +					     char **p_princ_s,
> +					     char **p_short_princ_s)
> +{
> +	char *princ_s = NULL;
> +	char *short_princ_s = NULL;
> +	const char *service = service_or_spn;
> +	const char *host = my_fqdn;
> +	struct spn_struct* spn_struct = NULL;
> +	char *tmp = NULL;
> +	bool ok = true;
> +
> +	/* SPN should have '/' */
> +	tmp = strchr_m(service_or_spn, '/');
> +	if (tmp != NULL) {
> +		spn_struct = parse_spn(ctx, service_or_spn);
> +		if (spn_struct == NULL) {
> +			ok = false;
> +			goto out;
> +		}
> +	}
> +	if (spn_struct != NULL) {
> +		service = spn_struct->serviceclass;
> +		host = spn_struct->host;
> +	}
> +	princ_s = talloc_asprintf(ctx, "%s/%s@%s",
> +				  service,
> +				  host, lp_realm());
> +	if (princ_s == NULL) {
> +		ok = false;
> +		goto out;
> +	}
> +
> +	if (spn_struct == NULL) {
> +		short_princ_s = talloc_asprintf(ctx, "%s/%s@%s",
> +					service, lp_netbios_name(),
> +					lp_realm());
> +		if (short_princ_s == NULL) {
> +			ok = false;
> +			goto out;
> +		}
> +	}
> +	*p_princ_s = princ_s;
> +	*p_short_princ_s = short_princ_s;
> +out:
> +	return ok;
> +}
> +
>  /**********************************************************************
>   Adds a single service principal, i.e. 'host' to the system keytab
>  ***********************************************************************/
> @@ -257,16 +310,11 @@ int ads_keytab_add_entry(ADS_STRUCT *ads, const char *srvPrinc)
>  		 * can obtain credentials for it and double-check the salt value
>  		 * used to generate the service's keys. */
>  
> -		princ_s = talloc_asprintf(tmpctx, "%s/%s@%s",
> -					  srvPrinc, my_fqdn, lp_realm());
> -		if (!princ_s) {
> -			ret = -1;
> -			goto out;
> -		}
> -		short_princ_s = talloc_asprintf(tmpctx, "%s/%s@%s",
> -						srvPrinc, lp_netbios_name(),
> -						lp_realm());
> -		if (short_princ_s == NULL) {
> +		if (!service_or_spn_to_kerberos_princ(tmpctx,
> +						      srvPrinc,
> +						      my_fqdn,
> +						      &princ_s,
> +						      &short_princ_s)) {
>  			ret = -1;
>  			goto out;
>  		}
> -- 
> 2.13.6
> 
> 
> From 11f0c2559e7d16de4f9d786b04665bef23cac458 Mon Sep 17 00:00:00 2001
> From: Noel Power <noel.power at suse.com>
> Date: Mon, 29 Jan 2018 18:38:05 +0000
> Subject: [PATCH 10/19] s3/libads: Allow 'net ads keytab add' handle Windows
>  SPN(s) part 2
> 
> This patch addresses how the windows SPN is written to the AD.
> 
> If a legacy service (e.g. cifs, http etc.) is passed as param to
> 'net ads keytab add param' then windows SPNs are generated from
> 'param' as follows
>           i) long form 'param/full_qualified_dns'
>          ii) short form 'param/netbios_name'
> 
> If the SPN is a is a Windows SPN (e.g. conforming to format
> 'serviceclass/host:port') then this is the SPN that is passed to
> the AD.
> 
> Signed-off-by: Noel Power <noel.power at suse.com>
> ---
>  source3/libads/kerberos_keytab.c | 33 ++++++++++++++++++++++++---------
>  1 file changed, 24 insertions(+), 9 deletions(-)
> 
> diff --git a/source3/libads/kerberos_keytab.c b/source3/libads/kerberos_keytab.c
> index c76228dc463..aa7f193ee74 100644
> --- a/source3/libads/kerberos_keytab.c
> +++ b/source3/libads/kerberos_keytab.c
> @@ -128,19 +128,34 @@ static bool ads_set_machine_account_spns(TALLOC_CTX *ctx,
>  {
>  	const char **spn_names = NULL;
>  	ADS_STATUS aderr;
> -	bool ok = false;
> +	struct spn_struct* spn_struct = NULL;
> +	char *tmp = NULL;
> +
> +	/* SPN should have '/' */
> +	tmp = strchr_m(service_or_spn, '/');
> +	if (tmp != NULL) {
> +		spn_struct = parse_spn(ctx, service_or_spn);
> +		if (spn_struct == NULL) {
> +			return false;
> +		}
> +	}
>  
>  	DEBUG(3, (__location__ ": 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;
> +	if (spn_struct != NULL) {
> +		spn_names = talloc_zero_array(ctx, const char*, 2);
> +		spn_names[0] = service_or_spn;
> +	} else {
> +		bool ok = false;
> +		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);
> -- 
> 2.13.6
> 
> 
> From 0c57804da8d9eacdac8dc201c67136e6e005279b Mon Sep 17 00:00:00 2001
> From: Noel Power <noel.power at suse.com>
> Date: Wed, 17 Jan 2018 17:18:15 +0000
> Subject: [PATCH 11/19] testprogs/blackbox: add tests for net ads keytab add
> 
> Signed-off-by: Noel Power <noel.power at suse.com>
> ---
>  testprogs/blackbox/test_net_ads.sh | 78 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 78 insertions(+)
> 
> diff --git a/testprogs/blackbox/test_net_ads.sh b/testprogs/blackbox/test_net_ads.sh
> index 680367b6387..7e3a596fe5e 100755
> --- a/testprogs/blackbox/test_net_ads.sh
> +++ b/testprogs/blackbox/test_net_ads.sh
> @@ -63,6 +63,84 @@ testit "join (dedicated keytab)" $VALGRIND $net_tool ads join -U$DC_USERNAME%$DC
>  
>  testit "testjoin (dedicated keytab)" $VALGRIND $net_tool ads testjoin -kP || failed=`expr $failed + 1`
>  
> +netbios=$(grep "netbios name" $BASEDIR/$WORKDIR/client.conf | cut -f2 -d= | awk '{$1=$1};1')
> +uc_netbios=$(echo $netbios | tr '[:lower:]' '[:upper:]')
> +lc_realm=$(echo $REALM | tr '[:upper:]' '[:lower:]')
> +fqdns="$netbios.$lc_realm"
> +
> +krb_princ="primary/instance@$REALM"
> +testit "test (dedicated keytab) add a fully qualified krb5 principle" $VALGRIND $net_tool ads keytab add $krb_princ -U$DC_USERNAME%$DC_PASSWORD --option="kerberosmethod=dedicatedkeytab" --option="dedicatedkeytabfile=$dedicated_keytab_file" || failed=`expr $failed + 1`
> +
> +found=`$net_tool ads keytab list -U$DC_USERNAME%$DC_PASSWORD --option="kerberosmethod=dedicatedkeytab" --option="dedicatedkeytabfile=$dedicated_keytab_file" | grep $krb_princ | wc -l`
> +
> +testit "test (dedicated keytab) at least one fully qualified krb5 principle that was added is present in keytab" test $found -gt 1 || failed=`expr $failed + 1`
> +
> +machinename="machine123"
> +testit "test (dedicated keytab) add a kerberos prinicple created from machinename to keytab" $VALGRIND $net_tool ads keytab add $machinename'$' -U$DC_USERNAME%$DC_PASSWORD --option="kerberosmethod=dedicatedkeytab" --option="dedicatedkeytabfile=$dedicated_keytab_file" || failed=`expr $failed + 1`
> +search_str="$machinename\$@$REALM"
> +found=`$net_tool ads keytab list -U$DC_USERNAME%$DC_PASSWORD --option="kerberosmethod=dedicatedkeytab" --option="dedicatedkeytabfile=$dedicated_keytab_file" | grep $search_str | wc -l`
> +testit "test (dedicated keytab) at least one krb5 principle created from $machinename added is present in keytab" test $found -gt 1 || failed=`expr $failed + 1`
> +
> +service="nfs"
> +testit "test (dedicated keytab) add a $service service to keytab" $VALGRIND $net_tool ads keytab add $service -U$DC_USERNAME%$DC_PASSWORD --option="kerberosmethod=dedicatedkeytab" --option="dedicatedkeytabfile=$dedicated_keytab_file" || failed=`expr $failed + 1`
> +
> +search_str="$service/$fqdns@$REALM"
> +found=`$net_tool ads keytab list -U$DC_USERNAME%$DC_PASSWORD --option="kerberosmethod=dedicatedkeytab" --option="dedicatedkeytabfile=$dedicated_keytab_file" | grep $search_str | wc -l`
> +testit "test (dedicated keytab) at least one (long form) krb5 principle created from service added is present in keytab" test $found -gt 1 || failed=`expr $failed + 1`
> +
> +search_str="$service/$uc_netbios@$REALM"
> +found=`$net_tool ads keytab list -U$DC_USERNAME%$DC_PASSWORD --option="kerberosmethod=dedicatedkeytab" --option="dedicatedkeytabfile=$dedicated_keytab_file" | grep $search_str | wc -l`
> +testit "test (dedicated keytab) at least one (shorter form) krb5 principle created from service added is present in keytab" test $found -gt 1 || failed=`expr $failed + 1`
> +
> +spn_service="random_srv"
> +spn_host="somehost.subdomain.domain"
> +spn_port="12345"
> +
> +windows_spn="$spn_service/$spn_host"
> +testit "test (dedicated keytab) add a $windows_spn windows style SPN to keytab" $VALGRIND $net_tool ads keytab add $windows_spn -U$DC_USERNAME%$DC_PASSWORD --option="kerberosmethod=dedicatedkeytab" --option="dedicatedkeytabfile=$dedicated_keytab_file" || failed=`expr $failed + 1`
> +
> +search_str="$spn_service/$spn_host@$REALM"
> +found=`$net_tool ads keytab list -U$DC_USERNAME%$DC_PASSWORD --option="kerberosmethod=dedicatedkeytab" --option="dedicatedkeytabfile=$dedicated_keytab_file" | grep $search_str | wc -l`
> +testit "test (dedicated keytab) at least one krb5 principle created from windown SPN added is present in keytab" test $found -gt 1 || failed=`expr $failed + 1`
> +
> +windows_spn="$spn_service/$spn_host:$spn_port"
> +testit "test (dedicated keytab) add a $windows_spn windows style SPN to keytab" $VALGRIND $net_tool ads keytab add $windows_spn -U$DC_USERNAME%$DC_PASSWORD --option="kerberosmethod=dedicatedkeytab" --option="dedicatedkeytabfile=$dedicated_keytab_file" || failed=`expr $failed + 1`
> +
> +search_str="$spn_service/$spn_host@$REALM"
> +found=`$net_tool ads keytab list -U$DC_USERNAME%$DC_PASSWORD --option="kerberosmethod=dedicatedkeytab" --option="dedicatedkeytabfile=$dedicated_keytab_file" | grep $search_str | wc -l`
> +testit "test (dedicated keytab) at least one krb5 principle created from windown SPN (with port) added is present in keytab" test $found -gt 1 || failed=`expr $failed + 1`
> +
> +# keytab add shouldn't have written spn to AD
> +found=$($net_tool ads setspn list -U$DC_USERNAME%$DC_PASSWORD | grep $service | wc -l)
> +testit_expect_failure "test (dedicated keytab) spn is not written to AD (using keytab add)" test $found -eq 0 || failed=`expr $failed + 1`
> +
> +ad_service="writetoad"
> +testit_expected_failure "test (dedicated keytab) add a $ad_service service to keytab (using add_update_ads" $VALGRIND $net_tool ads keytab add_update_ads $ad_service -U$DC_USERNAME%$DC_PASSWORD --option="kerberosmethod=dedicatedkeytab" --option="dedicatedkeytabfile=$dedicated_keytab_file" || failed=`expr $failed + 1`
> +
> +found=$($net_tool ads setspn list -U$DC_USERNAME%$DC_PASSWORD | grep $ad_service | wc -l)
> +testit_expect_failure "test (dedicated keytab) spn is written to AD (using keytab add_update_ads)" test $found -eq 2 || failed=`expr $failed + 1`
> +
> +
> +# test existence in keytab of service (previously added) pulled from SPN post
> +# 'keytab create' is now present in keytab file
> +testit "test (dedicated keytab) keytab created succeeds" $VALGRIND $net_tool ads keytab create -U$DC_USERNAME%$DC_PASSWORD --option="kerberosmethod=dedicatedkeytab" --option="dedicatedkeytabfile=$dedicated_keytab_file" || failed=`expr $failed + 1`
> +found=$($net_tool ads keytab list -U$DC_USERNAME%$DC_PASSWORD --option="kerberosmethod=dedicatedkeytab" --option="dedicatedkeytabfile=$dedicated_keytab_file" | grep $ad_service | wc -l)
> +testit_expect_failure "test (dedicated keytab) spn service that exists in AD (created via add_update_ads) is added to keytab file" test $found -gt 1 || failed=`expr $failed + 1`
> +
> +found_ad=$($net_tool ads setspn list -U$DC_USERNAME%$DC_PASSWORD | grep $service | wc -l)
> +found_keytab=$($net_tool ads keytab list -U$DC_USERNAME%$DC_PASSWORD  --option="kerberosmethod=dedicatedkeytab" --option="dedicatedkeytabfile=$dedicated_keytab_file" | grep $service | wc -l)
> +# test after create that a spn that exists in the keytab but shouldn't
> +# be written to the AD.
> +testit_expected_failure "test spn service doensn't exist in AD but is present in keytab file after keytab create" test $found_ad -eq 0 -a $found_keytab -gt 1 || failed=`expr $failed + 1`
> +
> +# SPN parser is very basic but does detect some illegal combination
> +
> +windows_spn="$spn_service/$spn_host:"
> +testit_expect_failure "test (dedicated keytab) fail to parse windows spn with missing port" $VALGRIND $net_tool ads keytab add $windows_spn -U$DC_USERNAME%$DC_PASSWORD --option="kerberosmethod=dedicatedkeytab" --option="dedicatedkeytabfile=$dedicated_keytab_file" || failed=`expr $failed + 1
> +
> +windows_spn="$spn_service/$spn_host/"
> +testit_expect_failure "test (dedicated keytab) fail to parse windows spn with missing servicename" $VALGRIND $net_tool ads keytab add $windows_spn -U$DC_USERNAME%$DC_PASSWORD --option="kerberosmethod=dedicatedkeytab" --option="dedicatedkeytabfile=$dedicated_keytab_file" || failed=`expr $failed + 1
> +
>  testit "changetrustpw (dedicated keytab)" $VALGRIND $net_tool ads changetrustpw || failed=`expr $failed + 1`
>  
>  testit "leave (dedicated keytab)" $VALGRIND $net_tool ads leave -U$DC_USERNAME%$DC_PASSWORD || failed=`expr $failed + 1`
> -- 
> 2.13.6
> 
> 
> From 2e08a720006316faa2aa8314b904b9320b06397a Mon Sep 17 00:00:00 2001
> From: Noel Power <noel.power at suse.com>
> Date: Thu, 8 Feb 2018 17:33:08 +0000
> Subject: [PATCH 12/19] s3/libads: add param to prevent writing spn(s) to ads
> 
> 'net ads keytab add' currently in addition to adding to the
> keytab file this command also can update AD computer objects
> via ldap. This behaviour isn't very intuitive or expected given
> the command name. By default we shouldn't write to the ADS.
> 
> Prepare to change the default behaviour by modifying the function
> 'ads_keytab_add_entry' to take a paramater to modify the existing
> behaviour to optionally update the AD (or not).
> Signed-off-by: Noel Power <noel.power at suse.com>
> ---
>  source3/libads/ads_proto.h       |  3 ++-
>  source3/libads/kerberos_keytab.c | 14 +++++++-------
>  source3/utils/net_ads.c          |  2 +-
>  3 files changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/source3/libads/ads_proto.h b/source3/libads/ads_proto.h
> index 47e250a23df..43134bd9a0e 100644
> --- a/source3/libads/ads_proto.h
> +++ b/source3/libads/ads_proto.h
> @@ -49,7 +49,8 @@ void ads_disp_sd(ADS_STRUCT *ads, TALLOC_CTX *mem_ctx, struct security_descripto
>  
>  /* The following definitions come from libads/kerberos_keytab.c  */
>  
> -int ads_keytab_add_entry(ADS_STRUCT *ads, const char *srvPrinc);
> +int ads_keytab_add_entry(ADS_STRUCT *ads, const char *srvPrinc,
> +			 bool update_ads);
>  int ads_keytab_flush(ADS_STRUCT *ads);
>  int ads_keytab_create_default(ADS_STRUCT *ads);
>  int ads_keytab_list(const char *keytab_name);
> diff --git a/source3/libads/kerberos_keytab.c b/source3/libads/kerberos_keytab.c
> index aa7f193ee74..893fdfa2499 100644
> --- a/source3/libads/kerberos_keytab.c
> +++ b/source3/libads/kerberos_keytab.c
> @@ -224,7 +224,7 @@ out:
>   Adds a single service principal, i.e. 'host' to the system keytab
>  ***********************************************************************/
>  
> -int ads_keytab_add_entry(ADS_STRUCT *ads, const char *srvPrinc)
> +int ads_keytab_add_entry(ADS_STRUCT *ads, const char *srvPrinc, bool update_ads)
>  {
>  	krb5_error_code ret = 0;
>  	krb5_context context = NULL;
> @@ -339,7 +339,7 @@ int ads_keytab_add_entry(ADS_STRUCT *ads, const char *srvPrinc)
>  		   host/... principal in the AD account.
>  		   So only create these in the keytab, not in AD.  --jerry */
>  
> -		if (!strequal(srvPrinc, "cifs") &&
> +		if (update_ads && !strequal(srvPrinc, "cifs") &&
>  		    !strequal(srvPrinc, "host")) {
>  			if (!ads_set_machine_account_spns(tmpctx,
>  							  ads,
> @@ -537,7 +537,7 @@ int ads_keytab_create_default(ADS_STRUCT *ads)
>  		p[0] = '\0';
>  
>  		/* Add the SPNs found on the DC */
> -		ret = ads_keytab_add_entry(ads, srv_princ);
> +		ret = ads_keytab_add_entry(ads, srv_princ, true);
>  		if (ret != 0) {
>  			DEBUG(1, ("ads_keytab_add_entry failed while "
>  				  "adding '%s' principal.\n",
> @@ -550,7 +550,7 @@ int ads_keytab_create_default(ADS_STRUCT *ads)
>  	   really needs them and we will fall back to verifying against
>  	   secrets.tdb */
>  
> -	ret = ads_keytab_add_entry(ads, "cifs"));
> +	ret = ads_keytab_add_entry(ads, "cifs", true));
>  	if (ret != 0 ) {
>  		DEBUG(1, (__location__ ": ads_keytab_add_entry failed while "
>  			  "adding 'cifs'.\n"));
> @@ -599,7 +599,7 @@ int ads_keytab_create_default(ADS_STRUCT *ads)
>  		goto done;
>  	}
>  
> -	ret = ads_keytab_add_entry(ads, sam_account_name);
> +	ret = ads_keytab_add_entry(ads, sam_account_name, true);
>  	if (ret != 0) {
>  		DEBUG(1, (__location__ ": ads_keytab_add_entry() failed "
>  			  "while adding sAMAccountName (%s)\n",
> @@ -610,7 +610,7 @@ int ads_keytab_create_default(ADS_STRUCT *ads)
>  	/* remember that not every machine account will have a upn */
>  	upn = ads_get_upn(ads, frame, machine_name);
>  	if (upn) {
> -		ret = ads_keytab_add_entry(ads, upn);
> +		ret = ads_keytab_add_entry(ads, upn, true);
>  		if (ret != 0) {
>  			DEBUG(1, (__location__ ": ads_keytab_add_entry() "
>  				  "failed while adding UPN (%s)\n", upn));
> @@ -724,7 +724,7 @@ int ads_keytab_create_default(ADS_STRUCT *ads)
>  
>  	ret = 0;
>  	for (i = 0; oldEntries[i]; i++) {
> -		ret |= ads_keytab_add_entry(ads, oldEntries[i]);
> +		ret |= ads_keytab_add_entry(ads, oldEntries[i], true);
>  		TALLOC_FREE(oldEntries[i]);
>  	}
>  
> diff --git a/source3/utils/net_ads.c b/source3/utils/net_ads.c
> index 169a6fe99db..41f2e6802aa 100644
> --- a/source3/utils/net_ads.c
> +++ b/source3/utils/net_ads.c
> @@ -2626,7 +2626,7 @@ static int net_ads_keytab_add(struct net_context *c, int argc, const char **argv
>  		return -1;
>  	}
>  	for (i = 0; i < argc; i++) {
> -		ret |= ads_keytab_add_entry(ads, argv[i]);
> +		ret |= ads_keytab_add_entry(ads, argv[i], false);
>  	}
>  	ads_destroy(&ads);
>  	return ret;
> -- 
> 2.13.6
> 
> 
> From 6eb67f825ee2a9079754ac2212adbf5256ccd4a8 Mon Sep 17 00:00:00 2001
> From: Noel Power <noel.power at suse.com>
> Date: Fri, 9 Feb 2018 14:03:33 +0000
> Subject: [PATCH 13/19] s3/utils: Modify default behaviour of 'net ads keytab
>  add'
> 
> This change modifies the behaviour of 'net ads keytab add' such
> that only the keytab file is modified.
> 
> A new command 'net ads keytab add_update_ads' has been added that
> preserves the legacy behaviour which can update the AD computer
> object with Winows SPN(s) as appropriate. Alternatively the new
> command 'net ads setspn add' can be used to manually add the
> windows SPN(s) that previously would have been added.
> 
> Signed-off-by: Noel Power <noel.power at suse.com>
> ---
>  source3/utils/net_ads.c | 33 +++++++++++++++++++++++++++++----
>  1 file changed, 29 insertions(+), 4 deletions(-)
> 
> diff --git a/source3/utils/net_ads.c b/source3/utils/net_ads.c
> index 41f2e6802aa..95c368329e0 100644
> --- a/source3/utils/net_ads.c
> +++ b/source3/utils/net_ads.c
> @@ -2605,7 +2605,10 @@ static int net_ads_keytab_flush(struct net_context *c, int argc, const char **ar
>  	return ret;
>  }
>  
> -static int net_ads_keytab_add(struct net_context *c, int argc, const char **argv)
> +static int net_ads_keytab_add(struct net_context *c,
> +			      int argc,
> +			      const char **argv,
> +			      bool update_ads)
>  {
>  	int i;
>  	int ret = 0;
> @@ -2626,12 +2629,26 @@ static int net_ads_keytab_add(struct net_context *c, int argc, const char **argv
>  		return -1;
>  	}
>  	for (i = 0; i < argc; i++) {
> -		ret |= ads_keytab_add_entry(ads, argv[i], false);
> +		ret |= ads_keytab_add_entry(ads, argv[i], update_ads);
>  	}
>  	ads_destroy(&ads);
>  	return ret;
>  }
>  
> +static int net_ads_keytab_add_default(struct net_context *c,
> +				      int argc,
> +				      const char **argv)
> +{
> +	return net_ads_keytab_add(c, argc, argv, false);
> +}
> +
> +static int net_ads_keytab_add_update_ads(struct net_context *c,
> +					 int argc,
> +					 const char **argv)
> +{
> +	return net_ads_keytab_add(c, argc, argv, true);
> +}
> +
>  static int net_ads_keytab_create(struct net_context *c, int argc, const char **argv)
>  {
>  	ADS_STRUCT *ads;
> @@ -2680,11 +2697,19 @@ int net_ads_keytab(struct net_context *c, int argc, const char **argv)
>  	struct functable func[] = {
>  		{
>  			"add",
> -			net_ads_keytab_add,
> +			net_ads_keytab_add_default,
>  			NET_TRANSPORT_ADS,
>  			N_("Add a service principal"),
>  			N_("net ads keytab add\n"
> -			   "    Add a service principal")
> +			   "    Add a service principal, updates keytab file only.")
> +		},
> +		{
> +			"add_update_ads",
> +			net_ads_keytab_add_update_ads,
> +			NET_TRANSPORT_ADS,
> +			N_("Add a service principal"),
> +			N_("net ads keytab add_update_ads\n"
> +			   "    Add a service principal, depending on the param passed may update ADS computer object in addition to the keytab file.")
>  		},
>  		{
>  			"create",
> -- 
> 2.13.6
> 
> 
> From ed2330326f6107fc568534b5e5a378275f9028db Mon Sep 17 00:00:00 2001
> From: Noel Power <noel.power at suse.com>
> Date: Fri, 16 Feb 2018 15:50:03 +0000
> Subject: [PATCH 14/19] testprogs/blackbox: switch expected failure tests to
>  expected pass
> 
> Following the commit to change the behaviour of 'net ads keytab add' and
> new 'keytab add_update_ads' some tests previously failing should now
> pass.
> 
> Signed-off-by: Noel Power <noel.power at suse.com>
> ---
>  testprogs/blackbox/test_net_ads.sh | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/testprogs/blackbox/test_net_ads.sh b/testprogs/blackbox/test_net_ads.sh
> index 7e3a596fe5e..56a0c45555e 100755
> --- a/testprogs/blackbox/test_net_ads.sh
> +++ b/testprogs/blackbox/test_net_ads.sh
> @@ -112,20 +112,20 @@ testit "test (dedicated keytab) at least one krb5 principle created from windown
>  
>  # keytab add shouldn't have written spn to AD
>  found=$($net_tool ads setspn list -U$DC_USERNAME%$DC_PASSWORD | grep $service | wc -l)
> -testit_expect_failure "test (dedicated keytab) spn is not written to AD (using keytab add)" test $found -eq 0 || failed=`expr $failed + 1`
> +testit "test (dedicated keytab) spn is not written to AD (using keytab add)" test $found -eq 0 || failed=`expr $failed + 1`
>  
>  ad_service="writetoad"
> -testit_expected_failure "test (dedicated keytab) add a $ad_service service to keytab (using add_update_ads" $VALGRIND $net_tool ads keytab add_update_ads $ad_service -U$DC_USERNAME%$DC_PASSWORD --option="kerberosmethod=dedicatedkeytab" --option="dedicatedkeytabfile=$dedicated_keytab_file" || failed=`expr $failed + 1`
> +testit "test (dedicated keytab) add a $ad_service service to keytab (using add_update_ads" $VALGRIND $net_tool ads keytab add_update_ads $ad_service -U$DC_USERNAME%$DC_PASSWORD --option="kerberosmethod=dedicatedkeytab" --option="dedicatedkeytabfile=$dedicated_keytab_file" || failed=`expr $failed + 1`
>  
>  found=$($net_tool ads setspn list -U$DC_USERNAME%$DC_PASSWORD | grep $ad_service | wc -l)
> -testit_expect_failure "test (dedicated keytab) spn is written to AD (using keytab add_update_ads)" test $found -eq 2 || failed=`expr $failed + 1`
> +testit "test (dedicated keytab) spn is written to AD (using keytab add_update_ads)" test $found -eq 2 || failed=`expr $failed + 1`
>  
>  
>  # test existence in keytab of service (previously added) pulled from SPN post
>  # 'keytab create' is now present in keytab file
>  testit "test (dedicated keytab) keytab created succeeds" $VALGRIND $net_tool ads keytab create -U$DC_USERNAME%$DC_PASSWORD --option="kerberosmethod=dedicatedkeytab" --option="dedicatedkeytabfile=$dedicated_keytab_file" || failed=`expr $failed + 1`
>  found=$($net_tool ads keytab list -U$DC_USERNAME%$DC_PASSWORD --option="kerberosmethod=dedicatedkeytab" --option="dedicatedkeytabfile=$dedicated_keytab_file" | grep $ad_service | wc -l)
> -testit_expect_failure "test (dedicated keytab) spn service that exists in AD (created via add_update_ads) is added to keytab file" test $found -gt 1 || failed=`expr $failed + 1`
> +testit "test (dedicated keytab) spn service that exists in AD (created via add_update_ads) is added to keytab file" test $found -gt 1 || failed=`expr $failed + 1`
>  
>  found_ad=$($net_tool ads setspn list -U$DC_USERNAME%$DC_PASSWORD | grep $service | wc -l)
>  found_keytab=$($net_tool ads keytab list -U$DC_USERNAME%$DC_PASSWORD  --option="kerberosmethod=dedicatedkeytab" --option="dedicatedkeytabfile=$dedicated_keytab_file" | grep $service | wc -l)
> -- 
> 2.13.6
> 
> 
> From 04b5022a559a6782342ae25ca7c517fd851e688c Mon Sep 17 00:00:00 2001
> From: Noel Power <noel.power at suse.com>
> Date: Fri, 9 Feb 2018 14:07:27 +0000
> Subject: [PATCH 15/19] s3/libads: net ads keytab create shouldn't write SPN(s)
> 
> Modify default behaviour of 'net ads keytab create'
> 
> The change modifies the behaviour of 'net ads keytab create' such
> that only the keytab file is modified. The current behaviour doesn't
> make sense, existing SPN(s) pulled from the computer AD object have
> the format 'serviceclass/host:port/servicename'.
> 'ads_keytab_create_default' calls ads_keytab_add_entry passing
> 'serviceclass' for each SPN retrieved from the AD. For each
> serviceclass passed in a new pair of SPN(s) is generated as follows
>     i) long form 'param/full_qualified_dns'
>    ii) short form 'param/netbios_name'
> 
> This doesn't make sense as we are creating a new SPN(s) from an existing
> one probably replacing the existing host with the 'client' machine.
> 
> If the keytab file exists then additionally each kerberos principle in the
> keytab file is parsed to strip out the primary, then 'ads_keytab_add_entry'
> is called which then tries by default to generate a SPN from any primary
> that doesn't end in '$'. By default those SPNs are then added to the AD
> computer account for the client running the command.
> 
> Signed-off-by: Noel Power <noel.power at suse.com>
> ---
>  source3/libads/kerberos_keytab.c | 10 +++++-----
>  source3/utils/net_ads.c          |  2 +-
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/source3/libads/kerberos_keytab.c b/source3/libads/kerberos_keytab.c
> index 893fdfa2499..50a28c8cfe3 100644
> --- a/source3/libads/kerberos_keytab.c
> +++ b/source3/libads/kerberos_keytab.c
> @@ -537,7 +537,7 @@ int ads_keytab_create_default(ADS_STRUCT *ads)
>  		p[0] = '\0';
>  
>  		/* Add the SPNs found on the DC */
> -		ret = ads_keytab_add_entry(ads, srv_princ, true);
> +		ret = ads_keytab_add_entry(ads, srv_princ, false);
>  		if (ret != 0) {
>  			DEBUG(1, ("ads_keytab_add_entry failed while "
>  				  "adding '%s' principal.\n",
> @@ -550,7 +550,7 @@ int ads_keytab_create_default(ADS_STRUCT *ads)
>  	   really needs them and we will fall back to verifying against
>  	   secrets.tdb */
>  
> -	ret = ads_keytab_add_entry(ads, "cifs", true));
> +	ret = ads_keytab_add_entry(ads, "cifs", false));
>  	if (ret != 0 ) {
>  		DEBUG(1, (__location__ ": ads_keytab_add_entry failed while "
>  			  "adding 'cifs'.\n"));
> @@ -599,7 +599,7 @@ int ads_keytab_create_default(ADS_STRUCT *ads)
>  		goto done;
>  	}
>  
> -	ret = ads_keytab_add_entry(ads, sam_account_name, true);
> +	ret = ads_keytab_add_entry(ads, sam_account_name, false);
>  	if (ret != 0) {
>  		DEBUG(1, (__location__ ": ads_keytab_add_entry() failed "
>  			  "while adding sAMAccountName (%s)\n",
> @@ -610,7 +610,7 @@ int ads_keytab_create_default(ADS_STRUCT *ads)
>  	/* remember that not every machine account will have a upn */
>  	upn = ads_get_upn(ads, frame, machine_name);
>  	if (upn) {
> -		ret = ads_keytab_add_entry(ads, upn, true);
> +		ret = ads_keytab_add_entry(ads, upn, false);
>  		if (ret != 0) {
>  			DEBUG(1, (__location__ ": ads_keytab_add_entry() "
>  				  "failed while adding UPN (%s)\n", upn));
> @@ -724,7 +724,7 @@ int ads_keytab_create_default(ADS_STRUCT *ads)
>  
>  	ret = 0;
>  	for (i = 0; oldEntries[i]; i++) {
> -		ret |= ads_keytab_add_entry(ads, oldEntries[i], true);
> +		ret |= ads_keytab_add_entry(ads, oldEntries[i], false);
>  		TALLOC_FREE(oldEntries[i]);
>  	}
>  
> diff --git a/source3/utils/net_ads.c b/source3/utils/net_ads.c
> index 95c368329e0..bf93abe688b 100644
> --- a/source3/utils/net_ads.c
> +++ b/source3/utils/net_ads.c
> @@ -2717,7 +2717,7 @@ int net_ads_keytab(struct net_context *c, int argc, const char **argv)
>  			NET_TRANSPORT_ADS,
>  			N_("Create a fresh keytab"),
>  			N_("net ads keytab create\n"
> -			   "    Create a fresh keytab")
> +			   "    Create a fresh keytab or update exising one.")
>  		},
>  		{
>  			"flush",
> -- 
> 2.13.6
> 
> 
> From 5d954429ad0a296baa529564db5c79c1e35a62bf Mon Sep 17 00:00:00 2001
> From: Noel Power <noel.power at suse.com>
> Date: Fri, 16 Feb 2018 15:53:16 +0000
> Subject: [PATCH 16/19] testprogs/blackbox: 'keytab create' expected failures
>  should now pass
> 
> Following the commit to change the behaviour of 'net ads keytab create'
> some tests previously failing should now pass.
> 
> Signed-off-by: Noel Power <noel.power at suse.com>
> ---
>  testprogs/blackbox/test_net_ads.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/testprogs/blackbox/test_net_ads.sh b/testprogs/blackbox/test_net_ads.sh
> index 56a0c45555e..ecfb1bfcd5c 100755
> --- a/testprogs/blackbox/test_net_ads.sh
> +++ b/testprogs/blackbox/test_net_ads.sh
> @@ -131,7 +131,7 @@ found_ad=$($net_tool ads setspn list -U$DC_USERNAME%$DC_PASSWORD | grep $service
>  found_keytab=$($net_tool ads keytab list -U$DC_USERNAME%$DC_PASSWORD  --option="kerberosmethod=dedicatedkeytab" --option="dedicatedkeytabfile=$dedicated_keytab_file" | grep $service | wc -l)
>  # test after create that a spn that exists in the keytab but shouldn't
>  # be written to the AD.
> -testit_expected_failure "test spn service doensn't exist in AD but is present in keytab file after keytab create" test $found_ad -eq 0 -a $found_keytab -gt 1 || failed=`expr $failed + 1`
> +testit "test spn service doensn't exist in AD but is present in keytab file after keytab create" test $found_ad -eq 0 -a $found_keytab -gt 1 || failed=`expr $failed + 1`
>  
>  # SPN parser is very basic but does detect some illegal combination
>  
> -- 
> 2.13.6
> 
> 
> From a0092e63f70e8fee55a12035949976e56f7af14e Mon Sep 17 00:00:00 2001
> From: Noel Power <noel.power at suse.com>
> Date: Mon, 12 Feb 2018 12:13:02 +0000
> Subject: [PATCH 17/19] docs: Add manpage for 'net ads keytab' subcommand
> 
> Signed-off-by: Noel Power <noel.power at suse.com>
> ---
>  docs-xml/manpages/net.8.xml | 83 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 83 insertions(+)
> 
> diff --git a/docs-xml/manpages/net.8.xml b/docs-xml/manpages/net.8.xml
> index 542dac53be4..d170d770b4c 100644
> --- a/docs-xml/manpages/net.8.xml
> +++ b/docs-xml/manpages/net.8.xml
> @@ -1339,6 +1339,89 @@ to show in the result.
>  </refsect2>
>  
>  <refsect2>
> +<title>ADS KEYTAB <replaceable>CREATE</replaceable></title>
> +
> +<para>
> +Creates a new keytab file if one doesn't exist with default entries. Default
> +entries are kerberos principles created from the machinename of the
> +client, the UPN (if it exists) and any Windows SPN(s) associated with the
> +computer AD account for the client. If a keytab file already exists then only
> +missing kerberos principles from the default entries are added. No changes
> +are made to the computer AD account.
> +</para>
> +</refsect2>
> +
> +<refsect2>
> +<title>ADS KEYTAB <replaceable>ADD</replaceable> <replaceable>(principle | machine | serviceclass | windows SPN</replaceable></title>
> +
> +<para>
> +Adds a new keytab entry, the entry can be either;
> +  <variablelist>
> +    <varlistentry><term>kerberos principle</term>
> +    <listitem><para>
> +      A kerberos principle (identified by the presence of '@') is just
> +      added to the keytab file.
> +    </para></listitem>
> +    </varlistentry>
> +    <varlistentry><term>machinename</term>
> +    <listitem><para>
> +      A machinename (identified by the trailing '$') is used to create a
> +      a kerberos principle 'machinename at realm' which is added to the
> +      keytab file.
> +    </para></listitem>
> +    </varlistentry>
> +    <varlistentry><term>serviceclass</term>
> +    <listitem><para>
> +    A serviceclass (such as 'cifs', 'html' etc.) is used to create a pair
> +    of kerberos principles 'serviceclass/fully_qualified_dns_name at realm' &
> +    'serviceclass/netbios_name at realm' which are added to the keytab file.
> +    </para></listitem>
> +    </varlistentry>
> +    <varlistentry><term>Windows SPN</term>
> +    <listitem><para>
> +    A Windows SPN is of the format 'serviceclass/host:port', it is used to
> +    create a kerberos principle 'serviceclass/host at realm' which will
> +    be written to the keytab file.
> +    </para></listitem>
> +    </varlistentry>
> +  </variablelist>
> +</para>
> +<para>
> +Unlike old versions no computer AD objects are modified by this command. To
> +preserve the bevhaviour of older clients 'net ads keytab ad_update_ads' is
> +available.
> +</para>
> +</refsect2>
> +
> +<refsect2>
> +<title>ADS KEYTAB <replaceable>ADD_UPDATE_ADS</replaceable> <replaceable>(principle | machine | serviceclass | windows SPN</replaceable></title>
> +
> +<para>
> +Adds a new keytab entry (see section for net ads keytab add). In addition to
> +adding entries to the keytab file corrosponding Windows SPNs are created
> +from the entry passed to this command. These SPN(s) added to the AD computer
> +account object associated with the client machine running this command for
> +the following entry types;
> +  <variablelist>
> +    <varlistentry><term>serviceclass</term>
> +    <listitem><para>
> +    A serviceclass (such as 'cifs', 'html' etc.) is used to create a
> +    pair of Windows SPN(s) 'param/full_qualified_dns' &
> +    'param/netbios_name' which are added to the AD computer account object
> +   for this client.
> +    </para></listitem>
> +    </varlistentry>
> +    <varlistentry><term>Windows SPN</term>
> +    <listitem><para>
> +    A Windows SPN is of the format 'serviceclass/host:port', it is
> +    added as passed to the AD computer account object for this client.
> +    </para></listitem>
> +    </varlistentry>
> +  </variablelist>
> +</para>
> +</refsect2>
> +
> +<refsect2>
>  <title>ADS WORKGROUP</title>
>  
>  <para>Print out workgroup name for specified kerberos realm.</para>
> -- 
> 2.13.6
> 
> 
> From 652b22a45b927cd4542c0afd4116f21232a5c7f7 Mon Sep 17 00:00:00 2001
> From: Noel Power <noel.power at suse.com>
> Date: Mon, 12 Feb 2018 13:53:19 +0000
> Subject: [PATCH 18/19] docs: Add manpage for new 'net ads setspn' subcommand
> 
> Signed-off-by: Noel Power <noel.power at suse.com>
> ---
>  docs-xml/manpages/net.8.xml | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/docs-xml/manpages/net.8.xml b/docs-xml/manpages/net.8.xml
> index d170d770b4c..7eaf65a2978 100644
> --- a/docs-xml/manpages/net.8.xml
> +++ b/docs-xml/manpages/net.8.xml
> @@ -1422,6 +1422,39 @@ the following entry types;
>  </refsect2>
>  
>  <refsect2>
> +<title>ADS setspn <replaceable>SETSPN LIST [machine]</replaceable></title>
> +
> +<para>
> +   Lists the Windows SPNs stored in the 'machine' Windows AD Computer object.
> +   If 'machine' is not specified then computer account for this client is used
> +   instead.
> +</para>
> +</refsect2>
> +
> +<refsect2>
> +<title>ADS setspn <replaceable>SETSPN ADD SPN [machine]</replaceable></title>
> +
> +<para>
> +   Adds the specified Windows SPN to the 'machine' Windows AD Computer object.
> +   If 'machine' is not specified then computer account for this client is used
> +   instead.
> +</para>
> +</refsect2>
> +
> +
> +<refsect2>
> +<title>ADS setspn <replaceable>SETSPN DELETE SPN [machine]</replaceable></title>
> +
> +<para>
> +   DELETE the specified Window SPN from the 'machine' Windows AD Computer
> +   object. If 'machine' is not specified then computer account for this
> +   client is used
> +   instead.
> +</para>
> +
> +</refsect2>
> +
> +<refsect2>
>  <title>ADS WORKGROUP</title>
>  
>  <para>Print out workgroup name for specified kerberos realm.</para>
> -- 
> 2.13.6
> 
> 
> From cede582990d165c080cda627f55771fd01ba541b Mon Sep 17 00:00:00 2001
> From: Noel Power <noel.power at suse.com>
> Date: Mon, 12 Feb 2018 16:38:56 +0000
> Subject: [PATCH 19/19] docs: Add info for 'net ads keytab' changes and new
>  'net ads setspn' cmd
> 
> Signed-off-by: Noel Power <noel.power at suse.com>
> ---
>  WHATSNEW.txt | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/WHATSNEW.txt b/WHATSNEW.txt
> index ad045e336ff..7bd37927160 100644
> --- a/WHATSNEW.txt
> +++ b/WHATSNEW.txt
> @@ -17,6 +17,42 @@ NEW FEATURES/CHANGES
>  ====================
>  
>  
> +net ads setspn
> +---------------
> +
> +There is a new 'net ads setspn' sub command for managing Windows SPN(s)
> +on the AD. This command aims to give the basic functionaility that is
> +provided on windows by 'setspn.exe' e.g. ability to add, delete and list
> +Windows SPN(s) stored in a Windows AD Computer object.
> +
> +The format of the command is:
> +
> +net ads setspn list [machine]
> +net ads setspn [add | delete ] SPN [machine]
> +
> +'machine' is the name of the computer account on the AD that is to be managed.
> +If 'machine' is not specified the name of the 'client' running the command
> +is used instead.
> +
> +The format of a Windows SPN is
> +  'serviceclass/host:port/servicename' (servicename and port are optional)
> +
> +serviceclass/host is generally sufficient to specify a host based service.
> +
> +net ads keytab changes
> +----------------------
> +net ads keytab add no longer attempts to convert the passed serviceclass
> +(e.g. nfs, html etc.) into a Windows SPN which is added to the Windows AD
> +computer object. By default just the keytab file is modified.
> +
> +A new keytab subcommand 'add_update_ads' has been added to preserve the
> +legacy behaviour. However the new 'net ads setspn add' subcommand should
> +really be used instead.
> +
> +net ads keytab create no longer tries to generate SPN(s) from existing
> +entries in a keytab file. If it is required to add Windows SPN(s) then
> +'net ads setspn add' should be used instead.
> +
>  REMOVED FEATURES
>  ================
>  
> -- 
> 2.13.6
> 




More information about the samba-technical mailing list