Skip to content

Cellular: clean sensitive information from trace #9660

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 15, 2019
Merged

Cellular: clean sensitive information from trace #9660

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions UNITTESTS/features/cellular/framework/AT/at_cellularcontext/at_cellularcontexttest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,29 @@ TEST_F(TestAT_CellularContext, connect_disconnect_sync)
ASSERT_EQ(ctx1.disconnect(), NSAPI_ERROR_OK);
EXPECT_TRUE(ctx1.is_connected() == false);

ATHandler_stub::get_debug_clear();
EXPECT_FALSE(ATHandler_stub::is_get_debug_run());
ATHandler_stub::debug_call_count_clear();
at.set_debug(true);
ASSERT_EQ(ctx1.connect("1234", "internet", "usern", "pwd"), NSAPI_ERROR_OK);
EXPECT_TRUE(ATHandler_stub::is_get_debug_run());
EXPECT_TRUE(ATHandler_stub::set_debug_call_count_get() == 3);
EXPECT_TRUE(at.get_debug());
EXPECT_TRUE(ctx1.is_connected() == true);
ASSERT_EQ(ctx1.disconnect(), NSAPI_ERROR_OK);
EXPECT_TRUE(ctx1.is_connected() == false);

ATHandler_stub::get_debug_clear();
EXPECT_FALSE(ATHandler_stub::is_get_debug_run());
ATHandler_stub::debug_call_count_clear();
at.set_debug(false);
ASSERT_EQ(ctx1.connect("1234", "internet", "usern", "pwd"), NSAPI_ERROR_OK);
EXPECT_TRUE(ATHandler_stub::is_get_debug_run());
EXPECT_TRUE(ATHandler_stub::set_debug_call_count_get() == 3);
EXPECT_FALSE(at.get_debug());
EXPECT_TRUE(ctx1.is_connected() == true);
ASSERT_EQ(ctx1.disconnect(), NSAPI_ERROR_OK);
EXPECT_TRUE(ctx1.is_connected() == false);

// More connect test after we are re-writted getting of PDP context...
}
Expand Down
29 changes: 29 additions & 0 deletions UNITTESTS/features/cellular/framework/AT/at_cellulardevice/at_cellulardevicetest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,35 @@ TEST_F(TestAT_CellularDevice, test_AT_CellularDevice_create_delete_context)
delete dev;
}

TEST_F(TestAT_CellularDevice, TestAT_CellularDevice_set_pin_verify_debug)
{
EventQueue que;
FileHandle_stub fh1;
ATHandler at(&fh1, que, 0, ",");
AT_CellularDevice *dev = new AT_CellularDevice(&fh1);

ATHandler_stub::nsapi_error_value = NSAPI_ERROR_OK;
ATHandler_stub::get_debug_clear();
EXPECT_FALSE(ATHandler_stub::is_get_debug_run());
ATHandler_stub::debug_call_count_clear();
at.set_debug(true);
EXPECT_TRUE(NSAPI_ERROR_OK == dev->set_pin("12"));
EXPECT_TRUE(ATHandler_stub::is_get_debug_run());
EXPECT_TRUE(ATHandler_stub::set_debug_call_count_get() == 3);
EXPECT_TRUE(at.get_debug());

ATHandler_stub::get_debug_clear();
EXPECT_FALSE(ATHandler_stub::is_get_debug_run());
ATHandler_stub::debug_call_count_clear();
at.set_debug(false);
EXPECT_TRUE(NSAPI_ERROR_OK == dev->set_pin("11"));
EXPECT_TRUE(ATHandler_stub::is_get_debug_run());
EXPECT_TRUE(ATHandler_stub::set_debug_call_count_get() == 3);
EXPECT_FALSE(at.get_debug());

delete dev;
}

TEST_F(TestAT_CellularDevice, TestAT_CellularDevice_set_pin)
{
FileHandle_stub fh1;
Expand Down
31 changes: 31 additions & 0 deletions UNITTESTS/stubs/ATHandler_stub.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,29 @@ int ATHandler_stub::urc_amount = 0;
mbed::Callback<void()> ATHandler_stub::callback[kATHandler_urc_table_max_size];
char *ATHandler_stub::urc_string_table[kATHandler_urc_table_max_size] = {'\0'};

bool ATHandler_stub::get_debug_flag = false;
uint8_t ATHandler_stub::set_debug_call_count = 0;

bool ATHandler_stub::is_get_debug_run()
{
return ATHandler_stub::get_debug_flag;
}

void ATHandler_stub::get_debug_clear()
{
ATHandler_stub::get_debug_flag = false;
}

uint8_t ATHandler_stub::set_debug_call_count_get()
{
return ATHandler_stub::set_debug_call_count;
}

void ATHandler_stub::debug_call_count_clear()
{
ATHandler_stub::set_debug_call_count = 0;
}

ATHandler::ATHandler(FileHandle *fh, EventQueue &queue, uint32_t timeout, const char *output_delimiter, uint16_t send_delay) :
_nextATHandler(0),
_fileHandle(fh),
Expand All @@ -76,9 +99,17 @@ ATHandler::ATHandler(FileHandle *fh, EventQueue &queue, uint32_t timeout, const

void ATHandler::set_debug(bool debug_on)
{
++ATHandler_stub::set_debug_call_count;
ATHandler_stub::debug_on = debug_on;
}

bool ATHandler::get_debug() const
{
ATHandler_stub::get_debug_flag = true;

return ATHandler_stub::debug_on;
}

ATHandler::~ATHandler()
{
ATHandler_stub::ref_count = kATHandler_destructor_ref_ount;
Expand Down
7 changes: 7 additions & 0 deletions UNITTESTS/stubs/ATHandler_stub.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,13 @@ extern int resp_stop_success_count;
extern bool process_oob_urc;
extern int urc_amount;
extern char *urc_string_table[kATHandler_urc_table_max_size];

extern bool get_debug_flag;
bool is_get_debug_run();
void get_debug_clear();
extern uint8_t set_debug_call_count;
uint8_t set_debug_call_count_get();
void debug_call_count_clear();
}

#endif
5 changes: 5 additions & 0 deletions features/cellular/framework/AT/ATHandler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,11 @@ void ATHandler::set_debug(bool debug_on)
_debug_on = debug_on;
}

bool ATHandler::get_debug() const
{
return _debug_on;
}

ATHandler::~ATHandler()
{
while (_oobs) {
Expand Down
7 changes: 7 additions & 0 deletions features/cellular/framework/AT/ATHandler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,13 @@ class ATHandler {
*/
void set_debug(bool debug_on);

/**
* Get degug state set by @ref set_debug
*
* @return current state of debug
*/
bool get_debug() const;

/** Set debug_on for all ATHandlers in the _atHandlers list
*
* @param debug_on Set true to enable debug traces
Expand Down
7 changes: 7 additions & 0 deletions features/cellular/framework/AT/AT_CellularContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,8 +315,15 @@ nsapi_error_t AT_CellularContext::do_user_authentication()
_at.cmd_start("AT+CGAUTH=");
_at.write_int(_cid);
_at.write_int(_authentication_type);

const bool stored_debug_state = _at.get_debug();
_at.set_debug(false);

_at.write_string(_uname);
_at.write_string(_pwd);

_at.set_debug(stored_debug_state);

_at.cmd_stop_read_resp();
if (_at.get_last_error() != NSAPI_ERROR_OK) {
return NSAPI_ERROR_AUTH_FAILURE;
Expand Down
7 changes: 7 additions & 0 deletions features/cellular/framework/AT/AT_CellularDevice.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,14 @@ nsapi_error_t AT_CellularDevice::set_pin(const char *sim_pin)

_at->lock();
_at->cmd_start("AT+CPIN=");

const bool stored_debug_state = _at->get_debug();
_at->set_debug(false);

_at->write_string(sim_pin);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason that this line wasn't instead removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what you are referring here as injected code:

  1. stores current tracing state (enable/disable)
  2. disable tracing
  3. resumes stored tracing state after sim pin has been written to the modem (cellular subsystem)


_at->set_debug(stored_debug_state);

_at->cmd_stop_read_resp();
return _at->unlock_return_error();
}
Expand Down