Skip to content
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.

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

Add support for RADIUS configuration options to Wi-SUN #13412

Merged
merged 1 commit into from
Aug 20, 2020
Merged

Add support for RADIUS configuration options to Wi-SUN #13412

merged 1 commit into from
Aug 20, 2020

Conversation

mikaleppanen
Copy link

Summary of changes

Added support for external RADIUS server configuration to Wi-SUN Border Router.
Added configuration functions and .json configuration options for:

  • external RADIUS server IPv6 address
  • RADIUS shared secret.

Impact of changes

None

Migration actions required

None

Documentation

None


Pull request type

[] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[X] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[X] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers

@mikter @artokin


@0xc0170 0xc0170 changed the title Added support for RADIUS configuration options to Wi-SUN Add support for RADIUS configuration options to Wi-SUN Aug 11, 2020
0xc0170
0xc0170 previously approved these changes Aug 11, 2020
tr_error("Failed to set network size");
return NSAPI_ERROR_PARAMETER;
}
#endif

#ifdef MBED_CONF_MBED_MESH_API_RADIUS_SHARED_SECRET
// Set on here, for compatibility with applications not using WisunBorderRouter class */
Copy link

Choose a reason for hiding this comment

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

Is there any such applications that can be used without the Border router interface? the Border router is not started without it

Copy link
Author

Choose a reason for hiding this comment

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

There are applications, e.g. nanostack border router that use directly the nanostack c-api and does not use the border router class to start the BBR.

Copy link
Author

Choose a reason for hiding this comment

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

Removed the changes from wisuninterface. Applications using RADIUS should use the wisunborderrouter class.

@@ -186,3 +225,37 @@ int WisunBorderRouter::routing_table_get(ws_br_route_info_t *table_ptr, uint16_t

return ws_bbr_routing_table_get(_mesh_if_id, (bbr_route_info_t *)table_ptr, table_len);
}

mesh_error_t WisunBorderRouter::set_radius_server_ipv6_address(const char *address)
Copy link

Choose a reason for hiding this comment

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

Do we need the GET interfaces for these

Could this be combined in the higher level api to have radius_configure(address,password)?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I can add the get interface. There are no defaults for these, so the application must always program these first, but it might be easier for application, if the values could also be read from Nanostack.

I would prefer having the two functions for this since either one can be changed individually.

Copy link
Author

@mikaleppanen mikaleppanen Aug 12, 2020

Choose a reason for hiding this comment

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

Added get interfaces. Added storing of IP address and shared secret to border router class. Changed configuration to two phases, on class init the .json configuration is applied (if set) and then on BBR start the BBR is configured with the address and shared secret using the then defined (mesh) interface id.

@mergify mergify bot added the needs: work label Aug 11, 2020
@mergify mergify bot dismissed 0xc0170’s stale review August 12, 2020 11:15

Pull request has been modified.

@mikaleppanen
Copy link
Author

@mikter @0xc0170 updated based on comments, please review.


mesh_error_t WisunBorderRouter::get_radius_server_ipv6_address(char *address)
{
if (!_radius_ipv6_addr_set) {
Copy link

Choose a reason for hiding this comment

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

You would not need these booleans if you would always read these variables from the Mbed json in the begining?

Copy link
Author

Choose a reason for hiding this comment

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

The RADIUS configuration could be unspecified in the .json (use internal TLS) and then later enabled by configuration function calls.

#ifdef MBED_CONF_MBED_MESH_API_RADIUS_SHARED_SECRET
// Set on here, for compatibility with applications not using WisunBorderRouter class */
const char radius_shared_secret[] = {MBED_CONF_MBED_MESH_API_RADIUS_SHARED_SECRET};
if (ws_bbr_radius_shared_secret_set(get_interface_id(), strlen(radius_shared_secret), (const uint8_t *) radius_shared_secret) != 0) {
Copy link

Choose a reason for hiding this comment

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

These are still here?

Copy link
Author

Choose a reason for hiding this comment

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

Corrected.

@mikaleppanen
Copy link
Author

Changed shared secret length to uint16 and added support for byte sequences for shared secret to .json configuration.

@mikaleppanen
Copy link
Author

@artokin Here's the radius if pull request. Could you review.

mikter
mikter previously approved these changes Aug 18, 2020
@mikaleppanen
Copy link
Author

@0xc0170 @artokin nanostack release this depends to has been release to feature-wisun. This can proceed to testing.

@mergify mergify bot dismissed mikter’s stale review August 19, 2020 09:38

Pull request has been modified.

@mikaleppanen
Copy link
Author

Re-based over latest feature-wisun, @0xc0170 could you trigger the CI.

artokin
artokin previously approved these changes Aug 19, 2020
@artokin artokin requested a review from mikter August 19, 2020 09:44
@mergify mergify bot added needs: CI and removed needs: work labels Aug 19, 2020
@0xc0170
Copy link
Contributor

0xc0170 commented Aug 19, 2020

I've got few jobs running from master, once al lcomplete will start all the rest

@0xc0170
Copy link
Contributor

0xc0170 commented Aug 20, 2020

CI started

@mbed-ci
Copy link

mbed-ci commented Aug 20, 2020

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 1
Build artifacts

@mergify
Copy link

mergify bot commented Aug 20, 2020

This PR cannot be merged due to conflicts. Please rebase to resolve them.

Added support for external RADIUS server configuration to Wi-SUN Border Router.
Added configuration functions and .json configuration options for:
- external RADIUS server IPv6 address
- RADIUS shared secret.
@mergify mergify bot dismissed artokin’s stale review August 20, 2020 08:44

Pull request has been modified.

@mergify mergify bot added needs: CI and removed needs: work labels Aug 20, 2020
@0xc0170 0xc0170 removed the needs: CI label Aug 20, 2020
@0xc0170
Copy link
Contributor

0xc0170 commented Aug 20, 2020

CI restarted

@mbed-ci
Copy link

mbed-ci commented Aug 20, 2020

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 2
Build artifacts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants