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

Dtls #18

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Dtls #18

Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion .gitmodules
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
[submodule "RIOT"]
path = RIOT
url = https://github.com/AlexFuhr/RIOT.git
url = https://github.com/seojeongmoon/RIOT.git
branch = dtls
Copy link
Collaborator

Choose a reason for hiding this comment

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

You verified, that the changes you made in this RIOT fork, are merged into the RIOT upstream; do I remember this correctly? So we wouldn’t need to change this submodule, correct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, as long as the upstream is updated it is fine.


7 changes: 7 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,13 @@ INCLUDE += $(RIOTPKG)/tinycbor/cbor.h
CFLAGS += -DGNRC_IPV6_NIB_CONF_SLAAC=1


#choose tiny dtls stack
#if errors, disable
USEMODULE += tinydtls_sock_dtls
CFLAGS += -DDTLS_PSK
# when using plain CoAP. Uncomment the next line to fix this.
CFLAGS += -DGCOAP_PDU_BUF_SIZE=256

# Comment this out to disable code in RIOT that does safety checking
# which is not needed in a production environment but helps in the
# development process:
Expand Down
2 changes: 1 addition & 1 deletion RIOT
Submodule RIOT updated 1065 files
56 changes: 56 additions & 0 deletions credentials.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*
* Copyright (C) 2018 Inria
*
* This file is subject to the terms and conditions of the GNU Lesser
* General Public License v2.1. See the file LICENSE in the top level
* directory for more details.
*/

/**
* @ingroup tests
* @{
*
* @file
* @brief tlsman test application (PSK and ECC keys)
*
* Small test for TLSMAN. Many definitions defined here are also available at
* sock_secure (and are intended to be used in standard applications)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This file is originally meant to be a test. What do we use it for? To define the constants below?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, the keys are defined.

*
* @author Raul Fuentes <[email protected]>
*
* @}
*/

#ifdef MODULE_SOCK_DTLS
#ifdef DTLS_PSK
const char psk_key[] = "secretPSK";
const char psk_id[] = "Client_identity";
const unsigned psk_key_len = sizeof(psk_key) - 1;
const unsigned psk_id_len = sizeof(psk_id) - 1;
#endif /* DTLS_PSK */

#ifdef DTLS_ECC
const unsigned char ecdsa_priv_key[] = {
0x41, 0xC1, 0xCB, 0x6B, 0x51, 0x24, 0x7A, 0x14,
0x43, 0x21, 0x43, 0x5B, 0x7A, 0x80, 0xE7, 0x14,
0x89, 0x6A, 0x33, 0xBB, 0xAD, 0x72, 0x94, 0xCA,
0x40, 0x14, 0x55, 0xA1, 0x94, 0xA9, 0x49, 0xFA
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe I don’t know / understand enough about DTLS in RIOT. But it doesn’t feel secure to hard code a private key in a public repository. Is this meant to be used that way?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You are right. This is an example code. When we use it, we should replace with our own keys. (using special random generator for cryptography)


const unsigned char ecdsa_pub_key_x[] = {
0x36, 0xDF, 0xE2, 0xC6, 0xF9, 0xF2, 0xED, 0x29,
0xDA, 0x0A, 0x9A, 0x8F, 0x62, 0x68, 0x4E, 0x91,
0x63, 0x75, 0xBA, 0x10, 0x30, 0x0C, 0x28, 0xC5,
0xE4, 0x7C, 0xFB, 0xF2, 0x5F, 0xA5, 0x8F, 0x52
};

const unsigned char ecdsa_pub_key_y[] = {
0x71, 0xA0, 0xD4, 0xFC, 0xDE, 0x1A, 0xB8, 0x78,
0x5A, 0x3C, 0x78, 0x69, 0x35, 0xA7, 0xCF, 0xAB,
0xE9, 0x3F, 0x98, 0x72, 0x09, 0xDA, 0xED, 0x0B,
0x4F, 0xAB, 0xC3, 0x6F, 0xC7, 0x72, 0xF8, 0x29
};
#endif /* DTLS_ECC */
#endif /* MODULE_SOCK_DTLS */

typedef int unused_workaround;
59 changes: 59 additions & 0 deletions saul_coap.c
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,23 @@ uint8_t class_press = SAUL_SENSE_PRESS;
uint8_t class_temp = SAUL_SENSE_TEMP;
uint8_t class_voltage = SAUL_SENSE_VOLTAGE;

#ifdef MODULE_SOCK_DTLS
#include "net/credman.h"

#define SOCK_DTLS_GCOAP_TAG (10)

#ifdef DTLS_PSK
extern const char psk_key[];
extern const char psk_id[];
extern const unsigned psk_key_len;
extern const unsigned psk_id_len;
#else /* DTLS_PSK */
extern const unsigned char ecdsa_priv_key[];
extern const unsigned char ecdsa_pub_key_x[];
extern const unsigned char ecdsa_pub_key_y[];
#endif /* DTLS_ECC */
#endif /* MODULE_SOCK_DTLS */

Copy link
Collaborator

Choose a reason for hiding this comment

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

Just some minor code ordering issue: It suggest to move this up a bit. So that all included files are visible in the same spot.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think that won't be a problem

/* CoAP resources. Must be sorted by path (ASCII order). */
static const coap_resource_t _resources[] = {
{ "/hum", COAP_GET, _saul_type_handler, &class_hum },
Expand Down Expand Up @@ -288,5 +305,47 @@ CborError export_phydat_to_cbor(CborEncoder *encoder, phydat_t data, int dim)

void saul_coap_init(void)
{
#ifdef MODULE_SOCK_DTLS
#ifdef DTLS_PSK
credman_credential_t credential = {
.type = CREDMAN_TYPE_PSK,
.tag = SOCK_DTLS_GCOAP_TAG,
.params = {
.psk = {
.key = { .s = (char *)psk_key, .len = psk_key_len },
.id = { .s = (char *)psk_id, .len = psk_id_len },
},
},
};
#else /* DTLS_PSK */
ecdsa_public_key_t other_pubkeys[] = {
{ .x = ecdsa_pub_key_x, .y = ecdsa_pub_key_y },
};

credman_credential_t credential = {
.type = CREDMAN_TYPE_ECDSA,
.tag = SOCK_DTLS_GCOAP_TAG,
.params = {
.ecdsa = {
.private_key = ecdsa_priv_key,
.public_key = {
.x = ecdsa_pub_key_x,
.y = ecdsa_pub_key_y,
},
.client_keys = other_pubkeys,
.client_keys_size = ARRAY_SIZE(other_pubkeys),
}
},
};
#endif /* DTLS_ECC */
if (credman_add(&credential) < 0) {
puts("gcoap_cli: unable to add credential");
return;
}

/* tell gcoap with tag to use */
gcoap_set_credential_tag(SOCK_DTLS_GCOAP_TAG);
#endif
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thinking about the usage of this repository, I have some thoughts:

  • We would need a bit of documentation, to explain at least, how DTLS_PSK and DTLS_ECC differ. Maybe pointing to different resources.

  • Securing the network communication would be great. But in our context, I would prioritize compatibility with other groups (the web application or other nodes). Could you find out, what they would need to do, to support DTLS on their side? Maybe point to some documentation, as well.

  • More fundamental question: should we do this in our repository? I imagine, that this is part of a bigger system and is combined with other projects. What if this system combines two coap projects with DTLS configuration? Would this work? Or should the systems main.c handle the encryption on its own?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  1. I asked the guy(pokgak), he said ECC is too slow on hardware so we should use PSK.

  2. If they use the latest RIOT version that merged the pull request of pokgak, then it wouldn't be a problem. If not, that would be impossible because both communicators must have dtls.

  3. Yes that is a good point. I was thinking about it too. I think we can have two branches, one with DTLS and one without and use one without first. It is good but I would prioritize everything working over security right now :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If you create a dtls branch, then I will pull request to that branch.

Copy link
Collaborator

Choose a reason for hiding this comment

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


gcoap_register_listener(&_listener);
}