[PATCH 2/2] dns: Support larger queries when asking forwarder

[Matthieu Patou]

On 05/25/2013 04:21 AM, Kai Blin wrote:
> This should fix bug #9632
>
> Signed-off-by: Kai Blin <kai at samba.org>
> ---
>   source4/dns_server/dns_query.c  | 13 +++++++++++++
>   source4/dns_server/dns_server.c |  2 ++
>   source4/dns_server/dns_server.h |  4 ++++
>   source4/dns_server/dns_utils.c  | 21 +++++++++++++++++++++
>   4 files changed, 40 insertions(+)
>
> diff --git a/source4/dns_server/dns_query.c b/source4/dns_server/dns_query.c
> index 4ad14b9..5414e1d 100644
> --- a/source4/dns_server/dns_query.c
> +++ b/source4/dns_server/dns_query.c
> @@ -138,14 +138,17 @@ struct ask_forwarder_state {
>   static void ask_forwarder_done(struct tevent_req *subreq);
>   
>   static struct tevent_req *ask_forwarder_send(
> +	struct dns_server *dns,
>   	TALLOC_CTX *mem_ctx, struct tevent_context *ev,
>   	const char *forwarder, struct dns_name_question *question)
>   {
>   	struct tevent_req *req, *subreq;
>   	struct ask_forwarder_state *state;
> +	struct dns_res_rec *options;
>   	struct dns_name_packet out_packet = { 0, };
>   	DATA_BLOB out_blob;
>   	enum ndr_err_code ndr_err;
> +	WERROR werr;
>   
>   	req = tevent_req_create(mem_ctx, &state, struct ask_forwarder_state);
>   	if (req == NULL) {
> @@ -166,6 +169,15 @@ static struct tevent_req *ask_forwarder_send(
>   	out_packet.qdcount = 1;
>   	out_packet.questions = question;
>   
> +	werr = dns_generate_options(dns, state, &options);
> +	if (!W_ERROR_IS_OK(werr)) {
> +		tevent_req_werror(req, werr);
> +		return tevent_req_post(req, ev);
> +	}
> +
> +	out_packet.arcount = 1;
> +	out_packet.additional = options;
> +
>   	ndr_err = ndr_push_struct_blob(
>   		&out_blob, state, &out_packet,
>   		(ndr_push_flags_fn_t)ndr_push_dns_name_packet);
> @@ -615,6 +627,7 @@ struct tevent_req *dns_server_process_query_send(
>   			  in->questions[0].name));
>   
>   		subreq = ask_forwarder_send(
> +			dns,
>   			state, ev, lpcfg_dns_forwarder(dns->task->lp_ctx),
>   			&in->questions[0]);
>   		if (tevent_req_nomem(subreq, req)) {
> diff --git a/source4/dns_server/dns_server.c b/source4/dns_server/dns_server.c
> index bb4605f..7ad4aca 100644
> --- a/source4/dns_server/dns_server.c
> +++ b/source4/dns_server/dns_server.c
> @@ -791,6 +791,8 @@ static void dns_task_init(struct task_server *task)
>   	}
>   
>   	dns->task = task;
> +	/*FIXME: Make this a configurable option */
> +	dns->max_payload = 4096;
>   
>   	dns->server_credentials = cli_credentials_init(dns);
>   	if (!dns->server_credentials) {
> diff --git a/source4/dns_server/dns_server.h b/source4/dns_server/dns_server.h
> index ef85730..efe4db8 100644
> --- a/source4/dns_server/dns_server.h
> +++ b/source4/dns_server/dns_server.h
> @@ -56,6 +56,7 @@ struct dns_server {
>   	struct dns_server_zone *zones;
>   	struct dns_server_tkey_store *tkeys;
>   	struct cli_credentials *server_credentials;
> +	uint16_t max_payload;
>   };
>   
>   struct dns_request_state {
> @@ -107,6 +108,9 @@ WERROR dns_name2dn(struct dns_server *dns,
>   		   TALLOC_CTX *mem_ctx,
>   		   const char *name,
>   		   struct ldb_dn **_dn);
> +WERROR dns_generate_options(struct dns_server *dns,
> +			    TALLOC_CTX *mem_ctx,
> +			    struct dns_res_rec **options);
>   struct dns_server_tkey *dns_find_tkey(struct dns_server_tkey_store *store,
>   				      const char *name);
>   WERROR dns_verify_tsig(struct dns_server *dns,
> diff --git a/source4/dns_server/dns_utils.c b/source4/dns_server/dns_utils.c
> index e03a409..21c7f5a 100644
> --- a/source4/dns_server/dns_utils.c
> +++ b/source4/dns_server/dns_utils.c
> @@ -378,3 +378,24 @@ WERROR dns_name2dn(struct dns_server *dns,
>   	*_dn = dn;
>   	return WERR_OK;
>   }
> +
> +WERROR dns_generate_options(struct dns_server *dns,
> +			    TALLOC_CTX *mem_ctx,
> +			    struct dns_res_rec **options)
> +{
> +	struct dns_res_rec *o;
> +
> +	o = talloc_zero(mem_ctx, struct dns_res_rec);
> +	if (o == NULL) {
> +		return WERR_NOMEM;
> +	}
> +	o->name = '\0';
> +	o->rr_type = DNS_QTYPE_OPT;
> +	/* This is ugly, but RFC2671 wants the payload size in this field */
> +	o->rr_class = (enum dns_qclass) dns->max_payload;
> +	o->ttl = 0;
> +	o->length = 0;
> +
> +	*options = o;
> +	return WERR_OK;
> +}
I'm not a huge fan of the short 'o' here, opt is almost as short.

If you can fix this consider that you have my review here as well.

Thanks.
Matthieu

-- 
Matthieu Patou
Samba Team
http://samba.org