SoC/2017

From SambaWiki
Jump to: navigation, search

Initial task description

Improve libcli/dns

Samba comes with its own asynchronous DNS parser framework developed for the internal DNS server. Basic calls have been implemented for a client-side library as well, but a more fleshed out implementation would be needed. The goal of this project is to implement more high-level calls handling DNS requests, such as UDP/TCP switchover and client-side GSS-TSIG cryptography. A test suite excercising all the functions is required and can be used to cross-check and complement the existing DNS server tests already shipped by Samba. This testsuite should use cmocka.

  • Difficulty: Medium
  • Language(s): C
  • Mentors: Kai Blin, David Disseldorp
  • Student: Dimitris Gravanis


Project Summary

Project Information


Client-side DNS call handling with GSS-TSIG

Unix SMB/CIFS implementation

Dimitrios Gravanis © 2017

Based on the existing work by Samba Team ©


About

For the Samba AD DC, libcli/dns is a library that allows the handling of DNS calls (send/receive requests) and uses GSS-TSIG encryption protocol for signed packets, to accommodate encrypted client-server communication.

The project goal was to enhance client-server communication features, by implementing TCP request send/receive handling in addition to the existing UDP asynchronous request capabilites, and sign client-side packets with GSS-TSIG signatures, to provide security.

It consists of its respective function and structure libraries, that provide definitions for client-side functionality.

For more information on the project goals, read the GSoC proposal here.

The project timeline and development journal is documented in its dedicated blogspot.


Repositories
  • Individual project "mirror" repository (requires Samba source code for integration - NOT STANDALONE): link
  • Personal samba-team/samba fork with integrated changes in libcli/dns: link
  • Public Gist: link
  • Samba GitHub repository: link


Commits
  • dimgrav/Samba-GSOC2017: link
  • dimgrav/samba (fork): link


The libcli/dns library


To integrate the functionality described in the project goals, the entire libcli/dns structure had to be reorganized, since the vast majority of the current code in libcli/dns was created during the GSoC project duration, with the old code being integrated or submitted to minor changes, such as renames (for reasons of semantics and integration of the new code) and small additions, mainly to incorporate all the new code into the Samba building scripts.

The project code is currently under review by Samba.


libcli/dns contents

The project generated code and the difference in libcli/dns structure is demonstrated as follows:


Initial libcli/dns structure: (before)

  • dns.c
  • dns.h
  • libdns.h
  • wscript_build

Project libcli/dns structure: (after)

  • cli-fn/
    • README.md
    • client_crypto.c
    • dns_tcp.c
    • dns_udp.c
  • cmocka-tests/
    • test-fn/
      • cli_crypto_test.c
      • dns_tcp_test.c
      • dns_udp_test.c
      • wscript
    • README.md
    • cli_tests.c
    • wscript_build
  • README.md
  • cli_dns.c (replaces dns.c)
  • dns.h
  • libtcp.h
  • libudp.h (renamed from libdns.h)
  • libtsig.h
  • libwrap.h
  • wrap_cli.c
  • wscript_build


The README files document features and provide useful information on building with Waf.


Code examples

Changes in code exceed 2000 lines, therefore including all the new code in this page would be inefficient. Instead, some code parts that represent key features of libcli/dns are used as examples.

All the new code can be found in the repositories mentioned in the respective section of this page, or at the dedicated patch Gist.


1. Excerpt from cli_dns.c: send asynchronous DNS request to the server using TCP

This is the initiation of the send/receive transaction between client and server:

/* tcp request to send */
struct tevent_req *dns_tcp_req_send(TALLOC_CTX *mem_ctx,
					struct tevent_context *ev,
					const char *server_addr_string,
					struct iovec *vector,
					size_t count)
{
	struct tevent_req *req, *subreq, *socreq;
	struct dns_tcp_request_state *state;
	struct tsocket_address *local_address, *remote_address;
	struct tstream_context *stream;
	int req_ret, soc_ret, err;

	req = tevent_req_create(mem_ctx, &state, struct dns_tcp_request_state);
	if (req == NULL) {
		return NULL;
	}

	state->ev = ev;

	/* check for connected sockets and use if any */
	req_ret = tsocket_address_inet_from_strings(state, "ip", NULL, 0,
						&local_address);
	if (req_ret != 0) {
		tevent_req_error(req, errno);
		return tevent_req_post(req, ev);
	}

	req_ret = tsocket_address_inet_from_strings(state, "ip", server_addr_string,
						DNS_SERVICE_PORT, &remote_address);
	if (req_ret != 0) {
		tevent_req_error(req, errno);
		return tevent_req_post(req, ev);
	}

	/* must be reviewed! */
	soc_ret = tstream_inet_tcp_connect_recv(socreq, err, mem_ctx, stream, NULL);
	TALLOC_FREE(socreq);
	if (soc_ret == -1 && err != 0) {
		tevent_req_error(socreq, err);
		return tevent_req_post(req, ev);
	}

	socreq = tstream_inet_tcp_connect_send(mem_ctx, ev, local_address, remote_address);
	if (tevent_req_nomem(socreq, req)) {
		tevent_req_error(req, errno);
		return tevent_req_post(req, ev);
	}
	tevent_req_set_callback(socreq, dns_tcp_req_send, req);

	state->tstream = stream;
	state->v_count = count;

	subreq = tstream_writev_send(mem_ctx, ev, stream, vector, count);
	if (tevent_req_nomem(subreq, req)) {
		return tevent_req_post(req, ev);
	}

	if (!tevent_req_set_endtime(req, ev,
		timeval_current_ofs(DNS_REQUEST_TIMEOUT, 0))) {
		tevent_req_oom(req);
		return tevent_req_post(req, ev);
	}

	/* associate callback */
	tevent_req_set_callback(subreq, dns_tcp_req_recv_reply, req);
	
	return req;
}


2. Excerpt from cli_dns.c: generate GSS-TSIG and sign DNS packet.

This provides a means to verify credentials between client and server:

/* generate signature and rebuild packet with TSIG */
WERROR dns_cli_generate_tsig(struct dns_client *dns,
		       		TALLOC_CTX *mem_ctx,
		       		struct dns_request_cli_state *state,
		   		struct dns_name_packet *packet,
	      			DATA_BLOB *in)
{
	int tsig_flag = 0;
	struct dns_client_tkey *tkey = NULL;
	uint16_t i, arcount = 0;
	DATA_BLOB tsig_blob, fake_tsig_blob;
	uint8_t *buffer = NULL;
	size_t buffer_len = 0, packet_len = 0;
	
	NTSTATUS gen_sig;
	DATA_BLOB sig = (DATA_BLOB) {.data = NULL, .length = 0};
	struct dns_res_rec *tsig = NULL;
	time_t current_time = time(NULL);

	/* find TSIG record in inbound packet */
	for (i=0; i < packet->arcount; i++) {
		if (packet->additional[i].rr_type == DNS_QTYPE_TSIG) {
			tsig_flag = 1;
			break;
		}
	}
	if (tsig_flag != 1) {
		return WERR_OK;
	}

	/* check TSIG record format consistency */
	if (tsig_flag == 1 && i + 1 != packet->arcount) {
		DEBUG(1, ("TSIG format inconsistent!\n"));
		return DNS_ERR(FORMAT_ERROR);
	}

	/* save the keyname from the TSIG request to add MAC later */
	tkey = dns_find_cli_tkey(dns->tkeys, state->tsig->name);
	if (tkey == NULL) {
		state->key_name = talloc_strdup(state->mem_ctx,
						state->tsig->name);
		if (state->key_name == NULL) {
			return WERR_NOT_ENOUGH_MEMORY;
		}
		state->tsig_error = DNS_RCODE_BADKEY;
		return DNS_ERR(NOTAUTH);
	}
	state->key_name = talloc_strdup(state->mem_ctx, tkey->name);
	if (state->key_name == NULL) {
		return WERR_NOT_ENOUGH_MEMORY;
	}

	/* 
	 * preserve input packet but remove TSIG record bytes
	 * then count down the arcount field in the packet 
	 */
	packet_len = in->length - tsig_blob.length;
	packet->arcount--;
	arcount = RSVAL(buffer, 10);
	RSSVAL(buffer, 10, arcount-1);

	/* append fake_tsig_blob to buffer */
	buffer_len = packet_len + fake_tsig_blob.length;
	buffer = talloc_zero_array(mem_ctx, uint8_t, buffer_len);
	if (buffer == NULL) {
		return WERR_NOT_ENOUGH_MEMORY;
	}
	
	memcpy(buffer, in->data, packet_len);
	memcpy(buffer + packet_len, fake_tsig_blob.data, fake_tsig_blob.length);

	/* generate signature */
	gen_sig = gensec_sign_packet(tkey->gensec, mem_ctx, buffer, buffer_len,
				    buffer, buffer_len, &sig);

	/* get MAC size and save MAC to sig*/
	sig.length = state->tsig->rdata.tsig_record.mac_size;
	sig.data = talloc_memdup(mem_ctx, state->tsig->rdata.tsig_record.mac, sig.length);
	if (sig.data == NULL) {
		return WERR_NOT_ENOUGH_MEMORY;
	}

	/* rebuild packet with MAC from gensec_sign_packet() */
	tsig = talloc_zero(mem_ctx, struct dns_res_rec);

	tsig->name = talloc_strdup(tsig, state->key_name);
	if (tsig->name == NULL) {
		return WERR_NOT_ENOUGH_MEMORY;
	}
	tsig->rr_class = DNS_QCLASS_ANY;
	tsig->rr_type = DNS_QTYPE_TSIG;
	tsig->ttl = 0;
	tsig->length = UINT16_MAX;
	tsig->rdata.tsig_record.algorithm_name = talloc_strdup(tsig, "gss-tsig");
	tsig->rdata.tsig_record.time_prefix = 0;
	tsig->rdata.tsig_record.time = current_time;
	tsig->rdata.tsig_record.fudge = 300;
	tsig->rdata.tsig_record.error = state->tsig_error;
	tsig->rdata.tsig_record.original_id = packet->id;
	tsig->rdata.tsig_record.other_size = 0;
	tsig->rdata.tsig_record.other_data = NULL;
	if (sig.length > 0) {
		tsig->rdata.tsig_record.mac_size = sig.length;
		tsig->rdata.tsig_record.mac = talloc_memdup(tsig, sig.data, sig.length);
	}
	
	packet->additional = talloc_realloc(mem_ctx, packet->additional,
					    struct dns_res_rec,
					    packet->arcount + 1);
	if (packet->additional == NULL) {
		return WERR_NOT_ENOUGH_MEMORY;
	}
	packet->arcount++;
	
	return WERR_OK;
}


3. Excerpt from cmocka-tests/cli_tests.c: test for the dns_find_cli_tkey() function

dns_find_cli_tkey locates transaction key name in a DNS packet:

static struct dns_client_tkey *test_tkey_name(void) {
	
	struct dns_client_tkey *test_tkey = NULL;
	test_tkey->name = "TEST_TKEY";

	return test_tkey;
};

/* calls fail() if assertions are false */
static void tkey_test(void **state)
{
	struct dns_client_tkey_store *test_store;
	const char *test_name = "TEST_TKEY";
	
	struct dns_client_tkey *testing;
	struct dns_client_tkey *verifier;

	testing = test_tkey_name();
	verifier  = dns_find_cli_tkey(test_store, test_name);

	assert_non_null(testing);
	assert_non_null(verifier);
	assert_string_equal(testing->name, verifier->name);
	
	TALLOC_FREE(testing);
	TALLOC_FREE(verifier);
	return;
}


Other changes

In Samba/source4/dns_server/dns_query.c:

@@ -30,7 +30,7 @@
 #include "dsdb/samdb/samdb.h"
 #include "dsdb/common/util.h"
 #include "dns_server/dns_server.h"
-#include "libcli/dns/libdns.h"
+#include "libcli/dns/libudp.h"
 #include "lib/util/dlinklist.h"
 #include "lib/util/util_net.h"
 #include "lib/util/tevent_werror.h"

In Samba/libcli/dns/wscript_build:

@@ -1,5 +1,7 @@
#!/usr/bin/env python

+# builds a library for DNS TCP/UDP calls that utilizes GSS-TSIG encryption
 bld.SAMBA_SUBSYSTEM('clidns',
-         source='dns.c',
-         public_deps='LIBTSOCKET tevent-util')
+	  source='cli_dns.c',
+	  public_deps='LIBTSOCKET tevent-util',
+	  deps='gensec auth samba_server_gensec dnsserver_common')

In Samba/wscript_build:

@@ -120,12 +120,14 @@ bld.RECURSE('libcli/lsarpc')
 bld.RECURSE('libcli/drsuapi')
 bld.RECURSE('libcli/echo')
 bld.RECURSE('libcli/dns')
+bld.RECURSE('libcli/dns/cmocka-tests')
 bld.RECURSE('libcli/samsync')
 bld.RECURSE('libcli/registry')
 bld.RECURSE('source4/lib/policy')
 bld.RECURSE('libcli/named_pipe_auth')
 if bld.CONFIG_GET('ENABLE_SELFTEST'):
     bld.RECURSE('testsuite/unittests')
+    bld.RECURSE('libcli/dns/cmocka-tests/test-fn')
 
 if bld.CONFIG_GET('KRB5_VENDOR') in (None, 'heimdal'):
     if bld.CONFIG_GET("HEIMDAL_KRB5_CONFIG") and bld.CONFIG_GET("USING_SYSTEM_KRB5"):


DNS client and features


This section provides information on cli_dns functionality.


TCP/UDP requests

The client may use either TCP or UDP protocols to send a DNS name request to the server, then handle the reception of the appropriate server response.

Features:

  • UDP async request send/receive
  • TCP async request send/receive
  • GSS-TSIG generation
  • DNS name packet parsing and signing

The library consists of cli_dns.c, that includes functions, and dns.h, libtcp.h, libtsig.h, libudp.h, that provide definitions and structures.


Wrapping

wrap_cli.c provides multiple wrapping of the above functionality, to hide buffer creation, DNS packet parsing and signature generation. Definitions of the wrapped functions are provided in libwrap.h.


Test suite

In cmocka-tests, cli_tests.c provides a test suite for the complete client-side functionality, as defined by the functions in libcli/dns/cli_dns.c. The API used for unit testing is Cmocka.

In cmocka-tests/test-fn, there are individual unit tests for every feature library in libcli/dns. All of these tests are incorporated in cmocka-tests/cli_tests.c These tests can be built by using waf-samba and the intended configuration in cmocka-tests/test-fn/wscript. The purpose of these test suites is to facilitate future additions and features in Samba client-side code, without the necessity to integrate them directly to cli_dns.c, thus making changes easier to test and encourage future contributions.


To-do list


Not all initially set goals where achieved in the expected time-frame. Potential work and improvements include:


Client

Though Samba builds successfully using the newly created patches, it would be sensible to revise the patch versions and try to make the code in them optimal. Callback functions in request transactions and pointer casts should be reviewed and feedback from the test suites should provide the necessary guidelines for changes and fixes.


Tests

I was unable to run the individual test suites myself in the given time. The code however, was integrated and built with Samba without problems, though it is essential to work on the individual tests in libcli/dns/cmocka-tests/test-fn and ensure that they are functional, adequate and serve their purpose.

Specifically:

$ waf configure --enable-selftest && make test

builds only for libcli/dns/cmocka-tests/cli_tests.c, which means modifying top-level wscript_build as follows:

@@ -127,7 +127,7 @@ bld.RECURSE('source4/lib/policy')
 bld.RECURSE('libcli/named_pipe_auth')
 if bld.CONFIG_GET('ENABLE_SELFTEST'):
     bld.RECURSE('testsuite/unittests')
-    bld.RECURSE('libcli/dns/cmocka-tests/test-fn')
+#    bld.RECURSE('libcli/dns/cmocka-tests/test-fn')

Then, running:

$ make test TESTS=client_tests

returns ALL OK, but does not appear to enter cli_tests.c.

These issues must be addressed.


In terms of code quality, all included tests have room for refinement, improvement and more thorough function checks. A few specific points of interest would be:

  1. cmocka-tests/test-fn: wscript needs to be properly configured to enable standalone test builds for the feature libraries.
  2. cmocka-tests/cli_tests.c: TCP/UDP callbacks may be additionally tested for internal error output in their respective test functions.


It is my absolute intention to personally complete all needed fixes and perform optimizations in due time.


Epilogue

GSoC 2017 has been a massively influential experience for me. I will always remember how overwhelmingly excited I was when I received the acceptance e-mail, during class. Being actively involved in a large scale programming project, being part of the Open Source community, for me is its own reward and something that fulfilled a goal (dare I say dream) that I set in my early teens.


During the first period of the programme, I was neck-deep in classes and MSc exams, but I tried to go through every bit of documentation I could come across, study RFCs for networking protocols, read articles on active directories in a network, search for tutorials, I would literally consume every tiny bit of credible information I could get my hands on. This quest for knowledge was also expanded in better understanding Linux/UNIX systems. Having set my milestones in a way that could support the need for better comprehension of the fundamentals, as well as other information that would come up during the project, helped significantly.


When the coding period started, I quickly came to realize that my programming skills in C and Python had to be majorly extended to include all the Samba APIs, and this should also happen fast! By far the hardest part during the coding process, was learning to properly read Samba code and learn how to use the APIs to add the features required for my project. And this was NOT easy! I learned a great deal about how C handles a number of conventions, how the Samba APIs fit the job that they're used for (Metze wrote on an e-mail, that once I got around talloc, I'd never want to use anything else - and he was right!), I learned how to integrate new code into the existing one and make it work together. Honestly, I could write so much about the technical knowledge I acquired during working on Samba for GSoC, it would take over this entire section!


I must however mention what are probably the most important pieces of knowledge I received:

  • the importance of standardized mailing list conventions in a project's work flow
  • the awesomeness of Git for version control


Overall, the last three months were really tough, spending more than 10-11 hours/day working on the project. There were many frustrating moments, so much fatigue from the whole year, summer temptations that made it hard not to lose focus. And if I had to choose again between spending this summer coding in 40 °C, or laying on a beach at a Greek island, I'd choose the first in a blink of an eye!


I want to thank Jeremy, Stefan (Metze), Andreas, Ralph (Slow), Simo and everyone else from Samba Team that took the time to answer to my, on occasion, preposterous questions! :)


My dearest thanks go to my mentors, Kai and David, who allowed me to take part in GSoC and helped me troubleshoot my way through crashes, bugs, miscomprehensions. Especially David, whose replies to my e-mails during those late hours at the final stage of the project, provided much needed answers.


If there is a small piece of advice that I can share with future participants, is to give yourselves 110% to your projects and remember that they are meant to be educative and fun. The rewards for your efforts will be far more meaningful than you might initially think.


Cheers, Dimitris