From e7fae5d4b0905eb77d40b32f653a994ba96fb12f Mon Sep 17 00:00:00 2001 From: Seb Aebischer Date: Mon, 29 Jan 2018 16:45:58 +0000 Subject: [PATCH 1/7] Read timeouts in fsmpico This commit is a transfer of GitLab branch 88-read-timeouts-in-fsm. --- src/fsmpico.c | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/src/fsmpico.c b/src/fsmpico.c index 667170c..a9bea76 100644 --- a/src/fsmpico.c +++ b/src/fsmpico.c @@ -69,6 +69,8 @@ // Defines +#define READ_TIMEOUT (5000) +#define CONTINUOUS_TIMEOUT_LEEWAY (1000) #define RECONNECT_DELAY (10000) // Structure definitions @@ -111,6 +113,8 @@ struct _FsmPico { FSMPICOSTATE state; AuthFsmComms * comms; void * user_data; + + int reauthDelay; }; // Function prototypes @@ -165,6 +169,8 @@ FsmPico * fsmpico_new() { fsmpico->comms->sessionEnded = FsmSessionEndedNull; fsmpico->comms->statusUpdate = FsmStatusUpdateNull; + fsmpico->reauthDelay = 0; + return fsmpico; } @@ -364,6 +370,8 @@ void fsmpico_read(FsmPico * fsmpico, char const * data, size_t length) { createMessagePicoAuth(fsmpico, message, fsmpico->extraData); fsmpico->comms->write(buffer_get_buffer(message), buffer_get_pos(message), fsmpico->user_data); stateTransition(fsmpico, FSMPICOSTATE_STATUS); + // set a timeout for waiting for the status message + fsmpico->comms->setTimeout(READ_TIMEOUT, fsmpico->user_data); } break; case FSMPICOSTATE_STATUS: @@ -390,6 +398,7 @@ void fsmpico_read(FsmPico * fsmpico, char const * data, size_t length) { result = readMessageServiceReauth(fsmpico, dataread, &timeout); if (result) { stateTransition(fsmpico, FSMPICOSTATE_PICOREAUTH); + fsmpico->reauthDelay = timeout; LOG(LOG_DEBUG, "Timeout set to: %d", timeout); // Wait for timeout fsmpico->comms->setTimeout(timeout, fsmpico->user_data); @@ -425,6 +434,8 @@ void fsmpico_connected(FsmPico * fsmpico) { createMessageStart(fsmpico, message); fsmpico->comms->write(buffer_get_buffer(message), buffer_get_pos(message), fsmpico->user_data); stateTransition(fsmpico, FSMPICOSTATE_SERVICEAUTH); + // set a timeout for waiting for the service auth message + fsmpico->comms->setTimeout(READ_TIMEOUT, fsmpico->user_data); break; case FSMPICOSTATE_CONTSTARTPICO: fsmpico->currentState = REAUTHSTATE_CONTINUE; @@ -489,7 +500,7 @@ void fsmpico_timeout(FsmPico * fsmpico) { message = buffer_new(0); switch (fsmpico->state) { - case FSMPICOSTATE_CONTSTARTPICO: + case FSMPICOSTATE_CONTSTARTPICO: LOG(LOG_DEBUG, "Reconnecting for continuous authentication"); fsmpico->comms->reconnect(fsmpico->user_data); break; @@ -497,10 +508,26 @@ void fsmpico_timeout(FsmPico * fsmpico) { createMessagePicoReauth(fsmpico, message, extraData); fsmpico->comms->write(buffer_get_buffer(message), buffer_get_pos(message), fsmpico->user_data); stateTransition(fsmpico, FSMPICOSTATE_SERVICEREAUTH); + // set a timeout for awaiting a response + fsmpico->comms->setTimeout(fsmpico->reauthDelay + CONTINUOUS_TIMEOUT_LEEWAY, fsmpico->user_data); break; - default: + case FSMPICOSTATE_SERVICEAUTH: + case FSMPICOSTATE_STATUS: + case FSMPICOSTATE_SERVICEREAUTH: + // gave up waiting for a reply to a sent message, enter error state + stateTransition(fsmpico, FSMPICOSTATE_ERROR); + fsmpico->comms->error(fsmpico->user_data); + break; + case FSMPICOSTATE_INVALID: + case FSMPICOSTATE_ERROR: + // in these states, the state machine is stopped, so ignore it (but log) LOG(LOG_DEBUG, "Timer fired during an invalid state"); break; + default: + // in any other state, we're not expecting a timeout and this is an error + stateTransition(fsmpico, FSMPICOSTATE_ERROR); + fsmpico->comms->error(fsmpico->user_data); + break; } buffer_delete(extraData); From bbe200a4808ed76a8d53659f8783a655abe55488 Mon Sep 17 00:00:00 2001 From: Claudio Dettoni Date: Wed, 31 Jan 2018 13:53:20 +0000 Subject: [PATCH 2/7] Fix test according to new timeouts --- tests/test_fsm.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/test_fsm.c b/tests/test_fsm.c index a0cabfe..a7207bf 100644 --- a/tests/test_fsm.c +++ b/tests/test_fsm.c @@ -461,10 +461,15 @@ START_TEST (fsm_pico_test) { } int expectedTimeouts[] = { + 5000, // Read timeout for ServiceAuthMessage + 5000, // Read timeout for StatusMessage 10000, //Reconnect delay 9000, // Continuous timeout + 11000, // Read timeout for ServiceReauthMessage 9000, // Continuous timeout + 11000, // Read timeout for ServiceReauthMessage 9000, // Continuous timeout + 11000, // Read timeout for ServiceReauthMessage 9000 // Continuous timeout }; From 6ed4f4ce69eda9092342e76fb4f061ab57db5e6b Mon Sep 17 00:00:00 2001 From: Claudio Dettoni Date: Thu, 1 Feb 2018 16:00:16 +0000 Subject: [PATCH 3/7] Adding extra data to service reauth message I added the functionality of having an extra data to the service reauth message in a similar fashion as the pico reauth message It was implemented in a way that previous messages still work, so it is not breaking compatibility --- include/pico/messageservicereauth.h | 2 + src/messageservicereauth.c | 29 ++++ tests/Makefile.am | 2 +- tests/Makefile.in | 24 +++- tests/test_messages.c | 199 ++++++++++++++++++++++++++++ 5 files changed, 251 insertions(+), 5 deletions(-) create mode 100644 tests/test_messages.c diff --git a/include/pico/messageservicereauth.h b/include/pico/messageservicereauth.h index 698140f..56b0dc2 100644 --- a/include/pico/messageservicereauth.h +++ b/include/pico/messageservicereauth.h @@ -68,6 +68,8 @@ typedef struct _MessageServiceReAuth MessageServiceReAuth; MessageServiceReAuth * messageservicereauth_new(); void messageservicereauth_delete(MessageServiceReAuth * messageservicereauth); void messageservicereauth_set(MessageServiceReAuth * messageservicereauth, Buffer * sharedKey, long int timeout, REAUTHSTATE reauthState, SequenceNumber const * sequenceNum); +void messageservicereauth_set_extra_data(MessageServiceReAuth * messageservicereauth, Buffer * extraData); +const Buffer * messageservicereauth_get_extra_data(MessageServiceReAuth * messageservicereauth); void messageservicereauth_serialize(MessageServiceReAuth * messageservicereauth, Buffer * buffer); bool messageservicereauth_deserialize(MessageServiceReAuth * messageservicereauth, Buffer const * buffer); int messageservicereauth_get_timeout(MessageServiceReAuth * messageservicereauth); diff --git a/src/messageservicereauth.c b/src/messageservicereauth.c index b12acb1..e51a4b3 100644 --- a/src/messageservicereauth.c +++ b/src/messageservicereauth.c @@ -83,6 +83,7 @@ struct _MessageServiceReAuth { Buffer * iv; Buffer * encryptedData; + Buffer * extraData; }; // Function prototypes @@ -108,6 +109,7 @@ MessageServiceReAuth * messageservicereauth_new() { messageservicereauth->iv = buffer_new(0); messageservicereauth->encryptedData = buffer_new(0); + messageservicereauth->extraData = buffer_new(0); return messageservicereauth; } @@ -134,6 +136,8 @@ void messageservicereauth_delete(MessageServiceReAuth * messageservicereauth) { buffer_delete(messageservicereauth->encryptedData); } + buffer_delete(messageservicereauth->extraData); + FREE(messageservicereauth); } } @@ -163,6 +167,15 @@ void messageservicereauth_set(MessageServiceReAuth * messageservicereauth, Buffe } } +void messageservicereauth_set_extra_data(MessageServiceReAuth * messageservicereauth, Buffer * extraData) { + buffer_clear(messageservicereauth->extraData); + buffer_append_buffer(messageservicereauth->extraData, extraData); +} + +const Buffer * messageservicereauth_get_extra_data(MessageServiceReAuth * messageservicereauth) { + return messageservicereauth->extraData; +} + /** * Serialize the Status data in JSON format. * @@ -196,6 +209,9 @@ void messageservicereauth_serialize(MessageServiceReAuth * messageservicereauth, // Sequence number buffer_append_lengthprepend(toEncrypt, sequencenumber_get_raw_bytes(messageservicereauth->sequenceNum), SEQUENCE_NUMBER_LENGTH); + + // Extra data + buffer_append_buffer_lengthprepend(toEncrypt, messageservicereauth->extraData); iv = buffer_new(CRYPTOSUPPORT_IV_SIZE); cryptosupport_generate_iv(iv); @@ -349,6 +365,19 @@ bool messageservicereauth_deserialize(MessageServiceReAuth * messageservicereaut LOG(LOG_ERR, "Error deserializing decrypted length-prepended challenge sequence number data\n"); result = false; } + + // length | char extraData[length] (optional) + buffer_clear(bytes); + next = buffer_copy_lengthprepend(cleartext, start, bytes); + length = buffer_get_pos(bytes); + if (next > start) { + buffer_clear(messageservicereauth->extraData); + buffer_append_buffer(messageservicereauth->extraData, bytes); + start = next; + } + else { + LOG(LOG_DEBUG, "Service reauth message without extra data\n"); + } } if (result) { diff --git a/tests/Makefile.am b/tests/Makefile.am index a50c036..810e3a6 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1,6 +1,6 @@ AUTOMAKE_OPTIONS = subdir-objects -TESTS = test_base64 test_buffer test_cryptosupport test_sigmaverifier test_channel test_json test_users test_auth test_displayqr test_continuous test_beacons test_fsm +TESTS = test_base64 test_buffer test_cryptosupport test_sigmaverifier test_channel test_json test_users test_auth test_displayqr test_continuous test_beacons test_fsm test_messages noinst_LTLIBRARIES = lib_mockbt.la lib_mockbt_la_SOURCES = \ diff --git a/tests/Makefile.in b/tests/Makefile.in index be791c9..8a084ca 100644 --- a/tests/Makefile.in +++ b/tests/Makefile.in @@ -93,7 +93,7 @@ TESTS = test_base64$(EXEEXT) test_buffer$(EXEEXT) \ test_channel$(EXEEXT) test_json$(EXEEXT) test_users$(EXEEXT) \ test_auth$(EXEEXT) test_displayqr$(EXEEXT) \ test_continuous$(EXEEXT) test_beacons$(EXEEXT) \ - test_fsm$(EXEEXT) + test_fsm$(EXEEXT) test_messages$(EXEEXT) check_PROGRAMS = $(am__EXEEXT_1) subdir = tests ACLOCAL_M4 = $(top_srcdir)/aclocal.m4 @@ -125,7 +125,7 @@ am__EXEEXT_1 = test_base64$(EXEEXT) test_buffer$(EXEEXT) \ test_channel$(EXEEXT) test_json$(EXEEXT) test_users$(EXEEXT) \ test_auth$(EXEEXT) test_displayqr$(EXEEXT) \ test_continuous$(EXEEXT) test_beacons$(EXEEXT) \ - test_fsm$(EXEEXT) + test_fsm$(EXEEXT) test_messages$(EXEEXT) test_auth_SOURCES = test_auth.c test_auth_OBJECTS = test_auth.$(OBJEXT) test_auth_LDADD = $(LDADD) @@ -166,6 +166,10 @@ test_json_SOURCES = test_json.c test_json_OBJECTS = test_json.$(OBJEXT) test_json_LDADD = $(LDADD) test_json_DEPENDENCIES = ../libpico.la .libs/lib_mockbt.la +test_messages_SOURCES = test_messages.c +test_messages_OBJECTS = test_messages.$(OBJEXT) +test_messages_LDADD = $(LDADD) +test_messages_DEPENDENCIES = ../libpico.la .libs/lib_mockbt.la test_sigmaverifier_SOURCES = test_sigmaverifier.c test_sigmaverifier_OBJECTS = test_sigmaverifier.$(OBJEXT) test_sigmaverifier_LDADD = $(LDADD) @@ -211,11 +215,11 @@ am__v_CCLD_1 = SOURCES = $(lib_mockbt_la_SOURCES) test_auth.c test_base64.c \ test_beacons.c test_buffer.c test_channel.c test_continuous.c \ test_cryptosupport.c test_displayqr.c test_fsm.c test_json.c \ - test_sigmaverifier.c test_users.c + test_messages.c test_sigmaverifier.c test_users.c DIST_SOURCES = $(lib_mockbt_la_SOURCES) test_auth.c test_base64.c \ test_beacons.c test_buffer.c test_channel.c test_continuous.c \ test_cryptosupport.c test_displayqr.c test_fsm.c test_json.c \ - test_sigmaverifier.c test_users.c + test_messages.c test_sigmaverifier.c test_users.c am__can_run_installinfo = \ case $$AM_UPDATE_INFO_DIR in \ n|no|NO) false;; \ @@ -686,6 +690,10 @@ test_json$(EXEEXT): $(test_json_OBJECTS) $(test_json_DEPENDENCIES) $(EXTRA_test_ @rm -f test_json$(EXEEXT) $(AM_V_CCLD)$(LINK) $(test_json_OBJECTS) $(test_json_LDADD) $(LIBS) +test_messages$(EXEEXT): $(test_messages_OBJECTS) $(test_messages_DEPENDENCIES) $(EXTRA_test_messages_DEPENDENCIES) + @rm -f test_messages$(EXEEXT) + $(AM_V_CCLD)$(LINK) $(test_messages_OBJECTS) $(test_messages_LDADD) $(LIBS) + test_sigmaverifier$(EXEEXT): $(test_sigmaverifier_OBJECTS) $(test_sigmaverifier_DEPENDENCIES) $(EXTRA_test_sigmaverifier_DEPENDENCIES) @rm -f test_sigmaverifier$(EXEEXT) $(AM_V_CCLD)$(LINK) $(test_sigmaverifier_OBJECTS) $(test_sigmaverifier_LDADD) $(LIBS) @@ -712,6 +720,7 @@ distclean-compile: @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/test_displayqr.Po@am__quote@ @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/test_fsm.Po@am__quote@ @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/test_json.Po@am__quote@ +@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/test_messages.Po@am__quote@ @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/test_sigmaverifier.Po@am__quote@ @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/test_users.Po@am__quote@ @AMDEP_TRUE@@am__include@ @am__quote@mockbt/$(DEPDIR)/lib_mockbt_la-mockbt.Plo@am__quote@ @@ -1031,6 +1040,13 @@ test_fsm.log: test_fsm$(EXEEXT) --log-file $$b.log --trs-file $$b.trs \ $(am__common_driver_flags) $(AM_LOG_DRIVER_FLAGS) $(LOG_DRIVER_FLAGS) -- $(LOG_COMPILE) \ "$$tst" $(AM_TESTS_FD_REDIRECT) +test_messages.log: test_messages$(EXEEXT) + @p='test_messages$(EXEEXT)'; \ + b='test_messages'; \ + $(am__check_pre) $(LOG_DRIVER) --test-name "$$f" \ + --log-file $$b.log --trs-file $$b.trs \ + $(am__common_driver_flags) $(AM_LOG_DRIVER_FLAGS) $(LOG_DRIVER_FLAGS) -- $(LOG_COMPILE) \ + "$$tst" $(AM_TESTS_FD_REDIRECT) .test.log: @p='$<'; \ $(am__set_b); \ diff --git a/tests/test_messages.c b/tests/test_messages.c new file mode 100644 index 0000000..f6ee917 --- /dev/null +++ b/tests/test_messages.c @@ -0,0 +1,199 @@ +/** + * @file + * @author Claudio Dettoni + * @version $(VERSION) + * + * @section LICENSE + * + * (C) Copyright Cambridge Authentication Ltd, 2018 + * + * This file is part of libpico. + * + * Libpico is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of + * the License, or (at your option) any later version. + * + * Libpico 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 Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public + * License along with libpico. If not, see + * . + * + * + * @brief Unit tests for the message + */ + +#include +#include "pico/messagepicoreauth.h" +#include "pico/messageservicereauth.h" + +// Defines + +// Structure definitions + +// Function prototypes + +// Function definitions + +START_TEST (test_messagepicoreauth) { + MessagePicoReAuth * msg; + Buffer * sharedKey; + SequenceNumber* sequenceNumber; + SequenceNumber* sequenceNumberCopy; + SequenceNumber* sequenceNumberRecovered; + Buffer * extraData; + Buffer * json; + + sharedKey = buffer_new(0); + buffer_append(sharedKey, "\x00\x01\x02\x03\x04\x05", 6); + sequenceNumber = sequencenumber_new(); + sequenceNumberCopy = sequencenumber_new(); + sequenceNumberRecovered = sequencenumber_new(); + sequencenumber_random(sequenceNumber); + sequencenumber_copy(sequenceNumberCopy, sequenceNumber); + extraData = buffer_new(0); + buffer_append(extraData, "Extra", 5); + json = buffer_new(0); + + msg = messagepicoreauth_new(); + messagepicoreauth_set(msg, sharedKey, sequenceNumber); + messagepicoreauth_set_reauthstate(msg, REAUTHSTATE_PAUSE); + messagepicoreauth_serialize(msg, extraData, json); + + ck_assert(buffer_get_pos(json) > 0); + messagepicoreauth_delete(msg); + + msg = messagepicoreauth_new(); + messagepicoreauth_set(msg, sharedKey, NULL); + messagepicoreauth_deserialize(msg, json); + + ck_assert_str_eq(buffer_get_buffer(messagepicoreauth_get_extra_data(msg)), "Extra"); + ck_assert(messagepicoreauth_get_reauthstate(msg) == REAUTHSTATE_PAUSE); + messagepicoreauth_get_sequencenum(msg, sequenceNumberRecovered); + ck_assert(sequencenumber_equals(sequenceNumberRecovered, sequenceNumberCopy)); + + buffer_delete(sharedKey); + sequencenumber_delete(sequenceNumber); + sequencenumber_delete(sequenceNumberCopy); + sequencenumber_delete(sequenceNumberRecovered); + messagepicoreauth_delete(msg); + buffer_delete(json); + buffer_delete(extraData); +} +END_TEST + +START_TEST (test_messageservicereauth) { + MessageServiceReAuth * msg; + Buffer * sharedKey; + SequenceNumber* sequenceNumber; + SequenceNumber* sequenceNumberCopy; + SequenceNumber* sequenceNumberRecovered; + Buffer * extraData; + Buffer * json; + + sharedKey = buffer_new(0); + buffer_append(sharedKey, "\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f", 16); + sequenceNumber = sequencenumber_new(); + sequenceNumberCopy = sequencenumber_new(); + sequenceNumberRecovered = sequencenumber_new(); + sequencenumber_random(sequenceNumber); + sequencenumber_copy(sequenceNumberCopy, sequenceNumber); + extraData = buffer_new(0); + buffer_append(extraData, "Extra", 5); + json = buffer_new(0); + + msg = messageservicereauth_new(); + messageservicereauth_set(msg, sharedKey, 123, REAUTHSTATE_STOP, sequenceNumber); + messageservicereauth_set_extra_data(msg, extraData); + messageservicereauth_serialize(msg, json); + + buffer_print(json); + + ck_assert(buffer_get_pos(json) > 0); + messageservicereauth_delete(msg); + + msg = messageservicereauth_new(); + messageservicereauth_set(msg, sharedKey, 0, REAUTHSTATE_INVALID, NULL); + messageservicereauth_deserialize(msg, json); + + ck_assert(messageservicereauth_get_reauthstate(msg) == REAUTHSTATE_STOP); + ck_assert(messageservicereauth_get_timeout(msg) == 123); + messageservicereauth_get_sequencenum(msg, sequenceNumberRecovered); + ck_assert(sequencenumber_equals(sequenceNumberRecovered, sequenceNumberCopy)); + ck_assert_str_eq(buffer_get_buffer(messageservicereauth_get_extra_data(msg)), "Extra"); + + buffer_delete(sharedKey); + sequencenumber_delete(sequenceNumber); + sequencenumber_delete(sequenceNumberRecovered); + sequencenumber_delete(sequenceNumberCopy); + messageservicereauth_delete(msg); + buffer_delete(json); + buffer_delete(extraData); +} +END_TEST + +START_TEST (test_deserialize_messageservicereauth_without_extra_data) { + MessageServiceReAuth * msg; + Buffer * sharedKey; + SequenceNumber* sequenceNumberExpected; + SequenceNumber* sequenceNumberRecovered; + Buffer * expectedNumberBuf; + Buffer * json; + + sequenceNumberRecovered = sequencenumber_new(); + sequenceNumberExpected = sequencenumber_new(); + sharedKey = buffer_new(0); + buffer_append(sharedKey, "\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f", 16); + json = buffer_new(0); + buffer_append_string(json, "{\"iv\":\"YlGac7bdxNNWcToeNw3Xsg==\",\"encryptedData\":\"uFmElztl5yBtd79Cp651v5OX9YmIQ3BQ7N8tjhf/ShYNCu0fuLolmdqhog+pH2swMTh/jxXaajV0\",\"sessionId\":0}"); + expectedNumberBuf = buffer_new(0); + buffer_append(expectedNumberBuf, "\x3c\x3e\xbe\x5b\xf7\xd9\x08\xa1\xde\x02\x77\x51\xf4\xdf\x86\x46\x6c\x4a\xde\x6e\x48\x67\x83\xa6\x64\xaf\xda\x56\x0d\x9e\x69\xde", 32); + sequencenumber_transfer_from_buffer(sequenceNumberExpected, expectedNumberBuf); + + msg = messageservicereauth_new(); + messageservicereauth_set(msg, sharedKey, 0, REAUTHSTATE_INVALID, NULL); + ck_assert(messageservicereauth_deserialize(msg, json)); + + ck_assert(messageservicereauth_get_reauthstate(msg) == REAUTHSTATE_STOP); + ck_assert(messageservicereauth_get_timeout(msg) == 123); + messageservicereauth_get_sequencenum(msg, sequenceNumberRecovered); + sequencenumber_print(sequenceNumberRecovered); + sequencenumber_print(sequenceNumberExpected); + ck_assert(sequencenumber_equals(sequenceNumberRecovered, sequenceNumberExpected)); + + buffer_delete(sharedKey); + buffer_delete(expectedNumberBuf); + sequencenumber_delete(sequenceNumberRecovered); + sequencenumber_delete(sequenceNumberExpected); + messageservicereauth_delete(msg); + buffer_delete(json); +} +END_TEST + +int main (void) { + int number_failed; + Suite * s; + SRunner *sr; + TCase * tc; + + s = suite_create("Libpico"); + + // Base64 test case + tc = tcase_create("Messages"); + tcase_add_test(tc, test_messagepicoreauth); + tcase_add_test(tc, test_messageservicereauth); + tcase_add_test(tc, test_deserialize_messageservicereauth_without_extra_data); + suite_add_tcase(s, tc); + sr = srunner_create(s); + + srunner_run_all(sr, CK_NORMAL); + number_failed = srunner_ntests_failed(sr); + srunner_free(sr); + + return (number_failed == 0) ? 0 : -1; +} + From a5c4a3c770aa19b837db576487eff8f31d62edb0 Mon Sep 17 00:00:00 2001 From: Claudio Dettoni Date: Thu, 1 Feb 2018 17:43:46 +0000 Subject: [PATCH 4/7] Adding extraData to fsmpico This was in a similar way to what already existed on fsmservice --- include/pico/fsmpico.h | 2 + include/pico/messageservicereauth.h | 2 +- src/fsmpico.c | 81 +++++++++++++++++++++++------ src/fsmservice.c | 14 +++-- src/messageservicereauth.c | 2 +- 5 files changed, 80 insertions(+), 21 deletions(-) diff --git a/include/pico/fsmpico.h b/include/pico/fsmpico.h index 96d7f0e..f8d7026 100644 --- a/include/pico/fsmpico.h +++ b/include/pico/fsmpico.h @@ -103,6 +103,8 @@ FsmPico * fsmpico_new(); void fsmpico_delete(FsmPico * fsmpico); void fsmpico_set_functions(FsmPico * fsmpico, FsmWrite write, FsmSetTimeout setTimeout, FsmError error, FsmReconnect reconnect, FsmDisconnect disconnect, FsmAuthenticated authenticated, FsmSessionEnded sessionEnded, FsmStatusUpdate statusUpdate); void fsmpico_set_userdata(FsmPico * fsmpico, void * user_data); +Buffer const * fsmpico_get_received_extra_data(FsmPico * fsmpico); +void fsmpico_set_outbound_extra_data(FsmPico * fsmpico, Buffer const * extraData); // Use these functions to control the authentication process void fsmpico_start(FsmPico * fsmpico, Buffer const * extraData, EC_KEY * serviceIdPubKey, EC_KEY * clientIdPubKey, EVP_PKEY * clientIdPrivKey); diff --git a/include/pico/messageservicereauth.h b/include/pico/messageservicereauth.h index 56b0dc2..bb32131 100644 --- a/include/pico/messageservicereauth.h +++ b/include/pico/messageservicereauth.h @@ -68,7 +68,7 @@ typedef struct _MessageServiceReAuth MessageServiceReAuth; MessageServiceReAuth * messageservicereauth_new(); void messageservicereauth_delete(MessageServiceReAuth * messageservicereauth); void messageservicereauth_set(MessageServiceReAuth * messageservicereauth, Buffer * sharedKey, long int timeout, REAUTHSTATE reauthState, SequenceNumber const * sequenceNum); -void messageservicereauth_set_extra_data(MessageServiceReAuth * messageservicereauth, Buffer * extraData); +void messageservicereauth_set_extra_data(MessageServiceReAuth * messageservicereauth, const Buffer * extraData); const Buffer * messageservicereauth_get_extra_data(MessageServiceReAuth * messageservicereauth); void messageservicereauth_serialize(MessageServiceReAuth * messageservicereauth, Buffer * buffer); bool messageservicereauth_deserialize(MessageServiceReAuth * messageservicereauth, Buffer const * buffer); diff --git a/src/fsmpico.c b/src/fsmpico.c index fac4ae7..1ae071c 100644 --- a/src/fsmpico.c +++ b/src/fsmpico.c @@ -111,6 +111,7 @@ struct _FsmPico { Buffer * sharedKey; Shared * shared; Buffer * extraData; + Buffer * returnedExtraData; FSMPICOSTATE state; AuthFsmComms * comms; @@ -126,7 +127,7 @@ static bool readMessageServiceAuth(FsmPico * fsmpico, Buffer const * message); static void createMessagePicoAuth(FsmPico * fsmpico, Buffer * message, Buffer const * sendExtraData); static bool readMessageStatus(FsmPico * fsmpico, Buffer const * message, Buffer * returnedExtraData, char *status); static void createMessagePicoReauth(FsmPico * fsmpico, Buffer * message, Buffer const * sendExtraData); -static bool readMessageServiceReauth(FsmPico * fsmpico, Buffer const * message, int * timeout); +static bool readMessageServiceReauth(FsmPico * fsmpico, Buffer const * message, int * timeout, Buffer * returnedExtraData); static void stateTransition(FsmPico* fsmpico, FSMPICOSTATE newState); static void FsmWriteNull(char const * data, size_t length, void * user_data); @@ -152,6 +153,7 @@ FsmPico * fsmpico_new() { fsmpico->shared = shared_new(); fsmpico->extraData = buffer_new(0); + fsmpico->returnedExtraData = buffer_new(0); fsmpico->currentState = REAUTHSTATE_INVALID; fsmpico->picoSeqNumber = sequencenumber_new(); @@ -202,6 +204,9 @@ void fsmpico_delete(FsmPico * fsmpico) { buffer_delete(fsmpico->sharedKey); fsmpico->sharedKey = NULL; } + + buffer_delete(fsmpico->extraData); + buffer_delete(fsmpico->returnedExtraData); FREE(fsmpico->comms); @@ -352,13 +357,11 @@ void fsmpico_read(FsmPico * fsmpico, char const * data, size_t length) { LOG(LOG_DEBUG, "Read"); bool result; - Buffer * receivedExtraData; int timeout; Buffer * message; Buffer * dataread; char status; - receivedExtraData = buffer_new(0); message = buffer_new(0); dataread = buffer_new(length); buffer_append(dataread, data, length); @@ -377,7 +380,7 @@ void fsmpico_read(FsmPico * fsmpico, char const * data, size_t length) { } break; case FSMPICOSTATE_STATUS: - result = readMessageStatus(fsmpico, dataread, receivedExtraData, &status); + result = readMessageStatus(fsmpico, dataread, fsmpico->returnedExtraData, &status); if (result) { fsmpico->comms->authenticated((int) status, fsmpico->user_data); fsmpico->comms->disconnect(fsmpico->user_data); @@ -397,7 +400,7 @@ void fsmpico_read(FsmPico * fsmpico, char const * data, size_t length) { break; case FSMPICOSTATE_CONTSTARTSERVICE: case FSMPICOSTATE_SERVICEREAUTH: - result = readMessageServiceReauth(fsmpico, dataread, &timeout); + result = readMessageServiceReauth(fsmpico, dataread, &timeout, fsmpico->returnedExtraData); if (result) { stateTransition(fsmpico, FSMPICOSTATE_PICOREAUTH); fsmpico->reauthDelay = timeout; @@ -412,7 +415,6 @@ void fsmpico_read(FsmPico * fsmpico, char const * data, size_t length) { break; } - buffer_delete(receivedExtraData); buffer_delete(message); buffer_delete(dataread); } @@ -423,12 +425,10 @@ void fsmpico_read(FsmPico * fsmpico, char const * data, size_t length) { * @param fsmpico The object to apply to. */ void fsmpico_connected(FsmPico * fsmpico) { - Buffer * extraData; Buffer * message; LOG(LOG_DEBUG, "Connected"); - extraData = buffer_new(0); message = buffer_new(0); switch (fsmpico->state) { @@ -444,7 +444,7 @@ void fsmpico_connected(FsmPico * fsmpico) { buffer_clear(fsmpico->sharedKey); buffer_append_buffer(fsmpico->sharedKey, shared_get_shared_key(fsmpico->shared)); sequencenumber_random(fsmpico->picoSeqNumber); - createMessagePicoReauth(fsmpico, message, extraData); + createMessagePicoReauth(fsmpico, message, fsmpico->extraData); fsmpico->comms->write(buffer_get_buffer(message), buffer_get_pos(message), fsmpico->user_data); stateTransition(fsmpico, FSMPICOSTATE_CONTSTARTSERVICE); break; @@ -454,7 +454,6 @@ void fsmpico_connected(FsmPico * fsmpico) { break; } - buffer_delete(extraData); buffer_delete(message); } @@ -493,12 +492,10 @@ void fsmpico_disconnected(FsmPico * fsmpico) { * @param fsmpico The object to apply to. */ void fsmpico_timeout(FsmPico * fsmpico) { - Buffer * extraData; Buffer * message; LOG(LOG_DEBUG, "Timeout"); - extraData = buffer_new(0); message = buffer_new(0); switch (fsmpico->state) { @@ -507,7 +504,7 @@ void fsmpico_timeout(FsmPico * fsmpico) { fsmpico->comms->reconnect(fsmpico->user_data); break; case FSMPICOSTATE_PICOREAUTH: - createMessagePicoReauth(fsmpico, message, extraData); + createMessagePicoReauth(fsmpico, message, fsmpico->extraData); fsmpico->comms->write(buffer_get_buffer(message), buffer_get_pos(message), fsmpico->user_data); stateTransition(fsmpico, FSMPICOSTATE_SERVICEREAUTH); // set a timeout for awaiting a response @@ -532,10 +529,57 @@ void fsmpico_timeout(FsmPico * fsmpico) { break; } - buffer_delete(extraData); buffer_delete(message); } +/** + * Get the latest extra data that was sent by the Service. This is updated when + * the service sends either a ServiceAuth or PicoReauth message. The value is reset + * for each of these messages, and so the previous value will be wiped when + * either of these messages is received (even if the Pico doesn't send any + * extra data). + * + * To be alerted to any fresh data that arrives, the simplest approach is to + * set up an FsmStatusUpdate callback and check for any data recevied on either + * of the following two events: + * + * 1. FSMPICOSTATE_STATUS + * 2. FSMPICOSTATE_SERVICEREAUTH + * + * Then make a call using this function to check whether any new data + * has arrived (in which case, the returned buffer will be non-empty). + * + * @param fsmpico The object to get the extra data from. + * @return The latest extra data received from the Service, or an empty buffer. + */ +Buffer const * fsmpico_get_received_extra_data(FsmPico * fsmpico) { + return fsmpico->returnedExtraData; +} + +/** + * + * Set the extra data that will be sent to the Service. The data is sent in the + * Status and ServiceReauth messages. As such, to ensure it's set prior to each + * of these messages being sent, it's safe to set the new extra data when an + * update notifcation is triggered for either of the following events: + * + * 1. FSMPICOSTATE_STATUS + * 2. FSMPICOSTATE_SERVICEREAUTH + * + * These two events are those that immediately proceed the arrival of a + * Status or ServiceReauth message. + * + * @param fsmpico The object to set the extra data. + * @return The next extra data to be sent to the Service. + */ +void fsmpico_set_outbound_extra_data(FsmPico * fsmpico, Buffer const * extraData) { + // Record the extra data + buffer_clear(fsmpico->extraData); + if (extraData != NULL) { + buffer_append_buffer(fsmpico->extraData, extraData); + } +} + void stateTransition(FsmPico* fsmpico, FSMPICOSTATE newState) { fsmpico->state = newState; @@ -666,8 +710,10 @@ static void createMessagePicoReauth(FsmPico * fsmpico, Buffer * message, Buffer * @param fsmpico The object to apply to. * @param message The message data to interpret. * @param timeout A pointer to an integer to return the timeout value in. + * @param returnedExtraData A buffer to store any extra data that was extracted + * from the message. */ -static bool readMessageServiceReauth(FsmPico * fsmpico, Buffer const * message, int * timeout) { +static bool readMessageServiceReauth(FsmPico * fsmpico, Buffer const * message, int * timeout, Buffer * returnedExtraData) { MessageServiceReAuth * messageservicereauth; bool result; bool sequencenumber_match; @@ -700,6 +746,11 @@ static bool readMessageServiceReauth(FsmPico * fsmpico, Buffer const * message, } } + if (result && returnedExtraData != NULL) { + buffer_clear(returnedExtraData); + buffer_append_buffer(returnedExtraData, messageservicereauth_get_extra_data(messageservicereauth)); + } + if (result && sequencenumber_match) { sequencenumber_increment(fsmpico->serviceSeqNumber); } diff --git a/src/fsmservice.c b/src/fsmservice.c index 757559a..689248f 100644 --- a/src/fsmservice.c +++ b/src/fsmservice.c @@ -131,7 +131,7 @@ static void createMessageServiceAuth(FsmService * fsmservice, Buffer * message); static bool readMessagePicoAuth(FsmService * fsmservice, Buffer /* const */ * message, Buffer * returnedExtraData); static void createMessageStatus(FsmService * fsmservice, Buffer * message, Buffer const * sendExtraData, signed char status); static bool readMessagePicoReauth(FsmService * fsmservice, Buffer /* const */ * message, Buffer * returnedExtraData); -static void createMessageServiceReauth(FsmService * fsmservice, Buffer * message, int timeout); +static void createMessageServiceReauth(FsmService * fsmservice, Buffer * message, int timeout, const Buffer * extraData); static bool fsmservice_check_user(FsmService * fsmservice, Buffer * user, Buffer * symmetrickey); static void FsmWriteNull(char const * data, size_t length, void * user_data); @@ -562,7 +562,7 @@ void fsmservice_timeout(FsmService * fsmservice) { fsmservice->comms->sessionEnded(fsmservice->user_data); break; case FSMSERVICESTATE_SERVICEREAUTH: - createMessageServiceReauth(fsmservice, message, fsmservice->currentTimeout); + createMessageServiceReauth(fsmservice, message, fsmservice->currentTimeout, fsmservice->extraData); fsmservice->comms->write(buffer_get_buffer(message), buffer_get_pos(message), fsmservice->user_data); fsmservice->state = FSMSERVICESTATE_PICOREAUTH; fsmservice->comms->statusUpdate(fsmservice->state, fsmservice->user_data); @@ -746,14 +746,18 @@ static bool readMessagePicoReauth(FsmService * fsmservice, Buffer * message, Buf * @param fsmservice The object to apply to. * @param message A buffer to store the resulting message in. * @param timeout The timeout in milliseconds to send to the Pico. + * @param extraData [optional] application data to be included in the message */ -static void createMessageServiceReauth(FsmService * fsmservice, Buffer * message, int timeout) { +static void createMessageServiceReauth(FsmService * fsmservice, Buffer * message, int timeout, const Buffer * extraData) { MessageServiceReAuth * messageservicereauth; LOG(LOG_DEBUG, "Send MessageServiceReauth"); messageservicereauth = messageservicereauth_new(); messageservicereauth_set(messageservicereauth, fsmservice->sharedKey, timeout, fsmservice->currentState, fsmservice->serviceSeqNumber); + if (extraData != NULL) { + messageservicereauth_set_extra_data(messageservicereauth, extraData); + } messageservicereauth_serialize(messageservicereauth, message); // Increment the sequence number ready for the next message @@ -1004,7 +1008,9 @@ Buffer const * fsmservice_get_received_extra_data(FsmService * fsmservice) { void fsmservice_set_outbound_extra_data(FsmService * fsmservice, Buffer const * extraData) { // Record the extra data buffer_clear(fsmservice->extraData); - buffer_append_buffer(fsmservice->extraData, extraData); + if (extraData != NULL) { + buffer_append_buffer(fsmservice->extraData, extraData); + } } /** @} addtogroup Protocol */ diff --git a/src/messageservicereauth.c b/src/messageservicereauth.c index e51a4b3..c1bfa09 100644 --- a/src/messageservicereauth.c +++ b/src/messageservicereauth.c @@ -167,7 +167,7 @@ void messageservicereauth_set(MessageServiceReAuth * messageservicereauth, Buffe } } -void messageservicereauth_set_extra_data(MessageServiceReAuth * messageservicereauth, Buffer * extraData) { +void messageservicereauth_set_extra_data(MessageServiceReAuth * messageservicereauth, const Buffer * extraData) { buffer_clear(messageservicereauth->extraData); buffer_append_buffer(messageservicereauth->extraData, extraData); } From 11e443b5800ca69302fab2ceb5a3863f739c7970 Mon Sep 17 00:00:00 2001 From: Claudio Dettoni Date: Thu, 1 Feb 2018 17:45:25 +0000 Subject: [PATCH 5/7] Adding ability send send the reauth message at any time --- include/pico/fsmpico.h | 1 + include/pico/fsmservice.h | 1 + src/fsmpico.c | 50 ++++++++++++++++++++++++++++++++++++++ src/fsmservice.c | 51 +++++++++++++++++++++++++++++++++++++++ tests/test_fsm.c | 50 +++++++++++++++++++++++++++++++++++++- 5 files changed, 152 insertions(+), 1 deletion(-) diff --git a/include/pico/fsmpico.h b/include/pico/fsmpico.h index f8d7026..096c058 100644 --- a/include/pico/fsmpico.h +++ b/include/pico/fsmpico.h @@ -110,6 +110,7 @@ void fsmpico_set_outbound_extra_data(FsmPico * fsmpico, Buffer const * extraData void fsmpico_start(FsmPico * fsmpico, Buffer const * extraData, EC_KEY * serviceIdPubKey, EC_KEY * clientIdPubKey, EVP_PKEY * clientIdPrivKey); void fsmpico_stop(FsmPico * fsmpico); FSMPICOSTATE fsmpico_get_state(FsmPico * fsmpico); +void fsmpico_send_extra_data(FsmPico * fsmpico); // Call these functions when an event occurs void fsmpico_read(FsmPico * fsmpico, char const * data, size_t length); diff --git a/include/pico/fsmservice.h b/include/pico/fsmservice.h index 7b3e96f..9369494 100644 --- a/include/pico/fsmservice.h +++ b/include/pico/fsmservice.h @@ -120,6 +120,7 @@ DLL_PUBLIC void fsmservice_set_outbound_extra_data(FsmService * fsmservice, Buff DLL_PUBLIC void fsmservice_start(FsmService * fsmservice, Shared * shared, Users const * users, Buffer const * extraData); DLL_PUBLIC void fsmservice_stop(FsmService * fsmservice); DLL_PUBLIC FSMSERVICESTATE fsmservice_get_state(FsmService * fsmservice); +DLL_PUBLIC void fsmservice_send_extra_data(FsmService * fsmpico); // Call these functions when an event occurs DLL_PUBLIC void fsmservice_read(FsmService * fsmservice, char const * data, size_t length); diff --git a/src/fsmpico.c b/src/fsmpico.c index 1ae071c..bddf67e 100644 --- a/src/fsmpico.c +++ b/src/fsmpico.c @@ -409,6 +409,16 @@ void fsmpico_read(FsmPico * fsmpico, char const * data, size_t length) { fsmpico->comms->setTimeout(MAX((timeout - CONTAUTH_LEEWAY), 0), fsmpico->user_data); } break; + case FSMPICOSTATE_PICOREAUTH: + result = readMessageServiceReauth(fsmpico, dataread, &timeout, fsmpico->returnedExtraData); + if (result) { + stateTransition(fsmpico, FSMPICOSTATE_PICOREAUTH); + fsmpico->reauthDelay = timeout; + LOG(LOG_DEBUG, "Timeout set to: %d", timeout); + // Update reauthDelay but keep the previous timeout + // TODO assert we are waiting for a timeout? + } + break; default: stateTransition(fsmpico, FSMPICOSTATE_ERROR); fsmpico->comms->error(fsmpico->user_data); @@ -580,6 +590,46 @@ void fsmpico_set_outbound_extra_data(FsmPico * fsmpico, Buffer const * extraData } } +/** + * This functions should be called after fsmpico_set_outbound_extra_data + * if the application using FsmPico wants that data to be sent immediately. + * This functions only works after continuous authentication has been + * established. + * + * @param fsmpico The object to containing the state + * @param buffer Data to be sent + * + */ +void fsmpico_send_extra_data(FsmPico* fsmpico) { + Buffer * message; + + LOG(LOG_DEBUG, "fsmpico_send_extra_data"); + + message = buffer_new(0); + + switch (fsmpico->state) { + case FSMPICOSTATE_PICOREAUTH: + createMessagePicoReauth(fsmpico, message, fsmpico->extraData); + fsmpico->comms->write(buffer_get_buffer(message), buffer_get_pos(message), fsmpico->user_data); + stateTransition(fsmpico, FSMPICOSTATE_SERVICEREAUTH); + // set a timeout for awaiting a response + fsmpico->comms->setTimeout(fsmpico->reauthDelay + CONTINUOUS_TIMEOUT_LEEWAY, fsmpico->user_data); + break; + case FSMPICOSTATE_SERVICEREAUTH: + createMessagePicoReauth(fsmpico, message, fsmpico->extraData); + fsmpico->comms->write(buffer_get_buffer(message), buffer_get_pos(message), fsmpico->user_data); + stateTransition(fsmpico, FSMPICOSTATE_SERVICEREAUTH); + // Keep the previous timeout waiting for service + // TODO assert we are waiting for a timeout? + break; + default: + LOG(LOG_DEBUG, "Sending data during an invalid state"); + break; + } + + buffer_delete(message); + +} void stateTransition(FsmPico* fsmpico, FSMPICOSTATE newState) { fsmpico->state = newState; diff --git a/src/fsmservice.c b/src/fsmservice.c index 689248f..e1dd9f3 100644 --- a/src/fsmservice.c +++ b/src/fsmservice.c @@ -453,6 +453,14 @@ void fsmservice_read(FsmService * fsmservice, char const * data, size_t length) fsmservice->comms->error(fsmservice->user_data); } break; + case FSMSERVICESTATE_SERVICEREAUTH: + result = readMessagePicoReauth(fsmservice, dataread, fsmservice->returnedExtraData); + if (result) { + // Keep same state + fsmservice->comms->statusUpdate(fsmservice->state, fsmservice->user_data); + // TODO assert we are waiting for a timeout? + } + break; default: fsmservice->state = FSMSERVICESTATE_ERROR; fsmservice->comms->error(fsmservice->user_data); @@ -1013,5 +1021,48 @@ void fsmservice_set_outbound_extra_data(FsmService * fsmservice, Buffer const * } } +/** + * This functions should be called after fsmservice_set_outbound_extra_data when the application + * want to send the data immediately + * This functions only works after continuous authentication has been + * established. + * + * @param fsmservice The object containing the state + * + */ +void fsmservice_send_extra_data(FsmService* fsmservice) { + Buffer * message; + + LOG(LOG_DEBUG, "fsmservice_send_extra_data"); + + message = buffer_new(0); + + switch (fsmservice->state) { + case FSMSERVICESTATE_PICOREAUTH: + createMessageServiceReauth(fsmservice, message, fsmservice->currentTimeout, fsmservice->extraData); + fsmservice->comms->write(buffer_get_buffer(message), buffer_get_pos(message), fsmservice->user_data); + fsmservice->state = FSMSERVICESTATE_PICOREAUTH; + fsmservice->comms->statusUpdate(fsmservice->state, fsmservice->user_data); + // Keep the previous timeout waiting for service + // TODO assert we are waiting for a timeout? + break; + case FSMSERVICESTATE_SERVICEREAUTH: + createMessageServiceReauth(fsmservice, message, fsmservice->currentTimeout, fsmservice->extraData); + fsmservice->comms->write(buffer_get_buffer(message), buffer_get_pos(message), fsmservice->user_data); + fsmservice->state = FSMSERVICESTATE_PICOREAUTH; + fsmservice->comms->statusUpdate(fsmservice->state, fsmservice->user_data); + // set a timeout for awaiting a response + fsmservice->comms->setTimeout(fsmservice->currentTimeout, fsmservice->user_data); + break; + default: + LOG(LOG_DEBUG, "Sending data during an invalid state"); + break; + } + + buffer_delete(message); + +} + + /** @} addtogroup Protocol */ diff --git a/tests/test_fsm.c b/tests/test_fsm.c index a7207bf..dbe1392 100644 --- a/tests/test_fsm.c +++ b/tests/test_fsm.c @@ -239,6 +239,7 @@ START_TEST (fsm_fsm_test) { Node* queue = NULL; currentTime = 0; int cycles = 0; + int cyclesService = 0; Shared * picoShared = shared_new(); shared_load_or_generate_pico_keys(picoShared, "testpicokey.pub", "testpicokey.priv"); @@ -291,12 +292,57 @@ START_TEST (fsm_fsm_test) { } void picoStatusUpdate(FSMPICOSTATE state, void * user_data) { + Buffer * extraData; + extraData = buffer_new(0); + // Using this variable, otherwise send_extra_data will call status_update + // and we will have a stack overflow + static bool sendingReply = false; + if (state == FSMPICOSTATE_PICOREAUTH) { cycles++; if (cycles > 3) { queue = push_stop(queue, pico, NULL, currentTime); } } + if (state == FSMPICOSTATE_SERVICEREAUTH && !sendingReply) { + if (cycles == 2) { + sendingReply = true; + ck_assert_str_eq(buffer_get_buffer(fsmpico_get_received_extra_data(pico)), "EXTRA!!"); + buffer_append_string(extraData, "Extra reply"); + fsmpico_set_outbound_extra_data(pico, extraData); + fsmpico_send_extra_data(pico); + fsmpico_set_outbound_extra_data(pico, NULL); + } else { + ck_assert_str_eq(buffer_get_buffer(fsmpico_get_received_extra_data(pico)), ""); + } + } + + buffer_delete(extraData); + } + + void serviceStatusUpdate(FSMSERVICESTATE state, void * user_data) { + Buffer * extraData; + extraData = buffer_new(0); + + if (state == FSMSERVICESTATE_SERVICEREAUTH) { + cyclesService++; + if (cyclesService == 2) { + buffer_append_string(extraData, "EXTRA!!"); + fsmservice_set_outbound_extra_data(serv, extraData); + fsmservice_send_extra_data(serv); + fsmservice_set_outbound_extra_data(serv, NULL); + } + } + + if (state == FSMSERVICESTATE_SERVICEREAUTH) { + if (cyclesService == 4) { + ck_assert_str_eq(buffer_get_buffer(fsmservice_get_received_extra_data(serv)), "Extra reply"); + } else { + ck_assert_str_eq(buffer_get_buffer(fsmservice_get_received_extra_data(serv)), ""); + } + } + + buffer_delete(extraData); } void serviceAuthenticated(int status, void * user_data) { @@ -309,9 +355,11 @@ START_TEST (fsm_fsm_test) { ck_assert(buffer_equals(receivedSymKey, symmetricKey)); ck_assert(!strcmp(buffer_get_buffer(user), "Donald")); ck_assert(!strcmp(buffer_get_buffer(extraData), "p@ssword")); + + fsmpico_set_outbound_extra_data(pico, NULL); } - fsmservice_set_functions(serv, serviceWrite, serviceSetTimeout, NULL, NULL, serviceDisconnect, serviceAuthenticated, NULL, NULL); + fsmservice_set_functions(serv, serviceWrite, serviceSetTimeout, NULL, NULL, serviceDisconnect, serviceAuthenticated, NULL, serviceStatusUpdate); fsmservice_set_continuous(serv, true); fsmpico_set_functions(pico, picoWrite, picoSetTimeout, NULL, picoReconnect, picoDisconnect, NULL, NULL, picoStatusUpdate); From 3d290abcabdd715c54aa12bbbdd0601f3988a847 Mon Sep 17 00:00:00 2001 From: Seb Aebischer Date: Fri, 2 Feb 2018 17:35:42 +0000 Subject: [PATCH 6/7] Reply to ServiceReauth immediately Remove the timeout between reading a ServiceReauth message and sending the PicoReauth message. --- src/fsmpico.c | 28 +++++++++------------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/src/fsmpico.c b/src/fsmpico.c index a9bea76..8ebf85e 100644 --- a/src/fsmpico.c +++ b/src/fsmpico.c @@ -354,12 +354,14 @@ void fsmpico_read(FsmPico * fsmpico, char const * data, size_t length) { int timeout; Buffer * message; Buffer * dataread; + Buffer * extraDataToSend; char status; receivedExtraData = buffer_new(0); message = buffer_new(0); dataread = buffer_new(length); buffer_append(dataread, data, length); + extraDataToSend = buffer_new(0); // TODO: If the reads fail, should move to an error state switch (fsmpico->state) { @@ -399,9 +401,12 @@ void fsmpico_read(FsmPico * fsmpico, char const * data, size_t length) { if (result) { stateTransition(fsmpico, FSMPICOSTATE_PICOREAUTH); fsmpico->reauthDelay = timeout; - LOG(LOG_DEBUG, "Timeout set to: %d", timeout); - // Wait for timeout - fsmpico->comms->setTimeout(timeout, fsmpico->user_data); + // reply with the PicoReauth message + createMessagePicoReauth(fsmpico, message, extraDataToSend); + fsmpico->comms->write(buffer_get_buffer(message), buffer_get_pos(message), fsmpico->user_data); + stateTransition(fsmpico, FSMPICOSTATE_SERVICEREAUTH); + // set a timeout for awaiting a response + fsmpico->comms->setTimeout(fsmpico->reauthDelay + CONTINUOUS_TIMEOUT_LEEWAY, fsmpico->user_data); } break; default: @@ -413,6 +418,7 @@ void fsmpico_read(FsmPico * fsmpico, char const * data, size_t length) { buffer_delete(receivedExtraData); buffer_delete(message); buffer_delete(dataread); + buffer_delete(extraDataToSend); } /** @@ -491,26 +497,13 @@ void fsmpico_disconnected(FsmPico * fsmpico) { * @param fsmpico The object to apply to. */ void fsmpico_timeout(FsmPico * fsmpico) { - Buffer * extraData; - Buffer * message; - LOG(LOG_DEBUG, "Timeout"); - extraData = buffer_new(0); - message = buffer_new(0); - switch (fsmpico->state) { case FSMPICOSTATE_CONTSTARTPICO: LOG(LOG_DEBUG, "Reconnecting for continuous authentication"); fsmpico->comms->reconnect(fsmpico->user_data); break; - case FSMPICOSTATE_PICOREAUTH: - createMessagePicoReauth(fsmpico, message, extraData); - fsmpico->comms->write(buffer_get_buffer(message), buffer_get_pos(message), fsmpico->user_data); - stateTransition(fsmpico, FSMPICOSTATE_SERVICEREAUTH); - // set a timeout for awaiting a response - fsmpico->comms->setTimeout(fsmpico->reauthDelay + CONTINUOUS_TIMEOUT_LEEWAY, fsmpico->user_data); - break; case FSMPICOSTATE_SERVICEAUTH: case FSMPICOSTATE_STATUS: case FSMPICOSTATE_SERVICEREAUTH: @@ -529,9 +522,6 @@ void fsmpico_timeout(FsmPico * fsmpico) { fsmpico->comms->error(fsmpico->user_data); break; } - - buffer_delete(extraData); - buffer_delete(message); } From b5ce68f0d3c92789acbfaddda77a6d93a4f9d03e Mon Sep 17 00:00:00 2001 From: Claudio Dettoni Date: Mon, 12 Feb 2018 15:40:12 +0000 Subject: [PATCH 7/7] Improve messageservicereauth destructor --- src/messageservicereauth.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/messageservicereauth.c b/src/messageservicereauth.c index 09a7090..5597a29 100644 --- a/src/messageservicereauth.c +++ b/src/messageservicereauth.c @@ -123,20 +123,28 @@ void messageservicereauth_delete(MessageServiceReAuth * messageservicereauth) { if (messageservicereauth) { if (messageservicereauth->sharedKey) { buffer_delete(messageservicereauth->sharedKey); + messageservicereauth->sharedKey = NULL; } if (messageservicereauth->sequenceNum) { sequencenumber_delete(messageservicereauth->sequenceNum); + messageservicereauth->sequenceNum = NULL; } if (messageservicereauth->iv) { buffer_delete(messageservicereauth->iv); + messageservicereauth->iv = NULL; } + if (messageservicereauth->encryptedData) { buffer_delete(messageservicereauth->encryptedData); + messageservicereauth->encryptedData = NULL; + } + + if (messageservicereauth->extraData) { + buffer_delete(messageservicereauth->extraData); + messageservicereauth->extraData = NULL; } - - buffer_delete(messageservicereauth->extraData); FREE(messageservicereauth); }