Open pfalcon opened 4 years ago
atigyi
commented
4 years ago Private key in code:
static char private_key_pem[] = "..."
For initial version this will be ok. The drawback is each device requires individual binary. This isn't mass production friendly which, again, is ok for the initial version.
atigyi
commented
4 years ago +thanks for filing the known issues here in the issue tracker 👏
pfalcon
commented
4 years ago Private key in code: static char private_key_pem[] = "..."For initial version this will be ok. The drawback is each device requires individual binary. This isn't mass production friendly which, again, is ok for the initial version.
That's true, but using a plain filesystem for storing security keys in mass-production devices isn't ideal either due to security concerns. As I mentioned, our longer-term plan is to integrate this sample with some secure-storage solution (when Zephyr gets support for it!). So, yes, for the initial version, let's target just network communication part, I'm glad we're on the same line here.
I tried to post initial RFC for this change as https://github.com/atigyi/zephyr/pull/7, but github clearly glitches on it, I wonder if #1 might help. So, in the meantime this RFC-like change is in https://github.com/pfalcon/zephyr/commit/52fbc943d16449d4dc943b850d6e676f67bf218c
pfalcon
commented
4 years ago I tried to post initial RFC for this change as #7, but github clearly glitches on it
Ok, my mistake - I already have rebased pfalcon/google_iot_device_sdk_integration to master. Shouldn't have done that, it's upstream branch. Now reset to upstream state, and https://github.com/atigyi/zephyr/pull/8 posted.
As we decided, we'll be using github issue tracker to discuss/handle known issue with the sample app, as there're many detailed issues, and email wouldn't scale. I'm going to open individual tickets as needed, but still would like to give "en masse" comments to the list of issues in https://github.com/atigyi/zephyr/blob/google_iot_device_sdk_integration/samples/net/google_iot_device/README.rst:
This is very much resembles the issue I had with my older branch, https://github.com/pfalcon/iot-device-sdk-embedded-c . I also did some investigation, confirmed that root certs (seems) to be loaded, the similar set of ciphers (similar to samples/net/google_iot_mqtt) is enabled, timing source is ok, and yet verification fails. @d3zd3z, I'm not sure where we paused on that, did I invite you to have a look, and did you have a chance to do that?
I opened https://github.com/atigyi/zephyr/issues/2 on that. We definitely need to show working (at least) with qemu_x86 before we can claim "Zephyr support".
Well, I'd say it would be very nice to avoid dependency on filesystem for GoogleIoT usage, and thus considerably cut Flash storage requirements, and thus make it available to wider selection of MCU devices. There may be different ways to address this. For example, I know that iot-device-sdk-embedded-c includes a kind of in-memory (ROM) VFS (that's where root certificates are stored). Fairly speaking, I'm not sure if Zephyr offers a similar VFS layer, and if it is, there would be dichotomy which one to use. So, in my branch, https://github.com/pfalcon/iot-device-sdk-embedded-c, I went for the simplest possible solution:
static char private_key_pem[] = "...". I'd propose that we take that as a baseline, and see if we need something more than that in the initial version of the sample app we want to merge into Zephyr.See above - the proposal is to keep this optional in general, and then see if it's required for "v1" of the sample app to be pushed in Zephyr.
Safe entropy source depends on its availability on the actual hardware. For emulation platforms like native_posix/qemu_x86 it's not implemented. I propose that qemu_x86 support should be taken as a baseline for "v1" and Zephyr merging.
Fairly speaking, most of the samples in Zephyr tree are intended to demonstrate one feature at time. This keeps them short and simple, and thus allows to be a real good learning material for beginners, instead of confusing them with "wall of code". It also helps with maintenance too. So, let's see if there're specific stakeholder requirements to add more functionality to that samples. Note that from our (Linaro's) side, it's always was told that we want to integrate it with platform-level security services, for keys/certs management, OTA upgrades, etc. So, it's expected to grow in functionality noticeably.
Ack. I try to keep my comments short, but if you want to know, I tried to run older sample on BOARD=frdm_k64f, and while that board is pretty hefty on resources, I got an OOM or something.
That depends on elaborating non-blocking socket support in Zephyr. So, on us (and hopefully not for "v1" ;-).
One thing I did almost immediately after starting to look at your code (few months ago) is to improve POSIX time handling on Zephyr side. Refactors based on that are in my branch https://github.com/pfalcon/iot-device-sdk-embedded-c, and I'd be looking at forward-porting them to you new branch.
Ack, it's similar concern to what I expressed above, glad we're on the same line.
Apache-2 is very suitable, that's the license on Zephyr itself. Using any other license within the main tree requires Zephyr TSC approval. Submodules with a difference license should be ok. So, we should be clean here (thanks again for taking time to structure it well).
Yeah, no command line in Zephyr, it's purely a feature of native_posix simulator. In my branch, I just had config.h with #define PARAM1 ..., define PARAM2 ... . Using per-sample Kconfig options is also possible, we'll look what makes more sense here.
Thanks again for this work!