Joss-reviews: [REVIEW]: mTower: Trusted Execution Environment for MCU-based devices

[whedon]

Submitting author: @omoliavko (Oleksandr Moliavko)
Repository: https://github.com/Samsung/mTower.git
Version: v0.1
Editor: @gkthiruvathukal
Reviewer: @gradvohl, @federeghe
Archive: 10.5281/zenodo.3366076

Status

Status badge code:

HTML: <a href="http://joss.theoj.org/papers/407ce6c4610a1c5628beb57dd30ccf07"><img src="http://joss.theoj.org/papers/407ce6c4610a1c5628beb57dd30ccf07/status.svg"></a>
Markdown: [![status](http://joss.theoj.org/papers/407ce6c4610a1c5628beb57dd30ccf07/status.svg)](http://joss.theoj.org/papers/407ce6c4610a1c5628beb57dd30ccf07)

Reviewers and authors:

Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)

Reviewer instructions & questions

@gradvohl, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

1. Make sure you're logged in to your GitHub account
2. Be sure to accept the invite at this URL: https://github.com/openjournals/joss-reviews/invitations

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @gkthiruvathukal know.

✨ Please try and complete your review in the next two weeks ✨

Review checklist for @gradvohl

Conflict of interest

• [x] As the reviewer I confirm that I have read the JOSS conflict of interest policy and that there are no conflicts of interest for me to review this work.

Code of Conduct

• [x] I confirm that I read and will adhere to the JOSS code of conduct.

General checks

• [x] Repository: Is the source code for this software available at the repository url?
• [x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
• [x] Version: Does the release version given match the GitHub release (v0.1)?
• [ ] Authorship: Has the submitting author (@omoliavko) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

• [ ] Installation: Does installation proceed as outlined in the documentation?
• [ ] Functionality: Have the functional claims of the software been confirmed?
• [ ] Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

• [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• [ ] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
• [ ] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
• [x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
• [ ] Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
• [x] Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

• [ ] Authors: Does the paper.md file include a list of authors with their affiliations?
• [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• [x] References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?

Review checklist for @federeghe

Conflict of interest

• [x] As the reviewer I confirm that I have read the JOSS conflict of interest policy and that there are no conflicts of interest for me to review this work.

Code of Conduct

• [x] I confirm that I read and will adhere to the JOSS code of conduct.

General checks

• [x] Repository: Is the source code for this software available at the repository url?
• [x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
• [ ] Version: Does the release version given match the GitHub release (v0.1)?
• [ ] Authorship: Has the submitting author (@omoliavko) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

• [ ] Installation: Does installation proceed as outlined in the documentation?
• [ ] Functionality: Have the functional claims of the software been confirmed?
• [ ] Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

• [ ] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• [ ] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
• [ ] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
• [ ] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
• [ ] Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
• [ ] Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

• [ ] Authors: Does the paper.md file include a list of authors with their affiliations?
• [ ] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• [ ] References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?

[whedon]

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @gradvohl it looks like you're currently assigned to review this paper :tada:.

:star: Important :star:

If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews 😿

To fix this do the following two things:

1. Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

1. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

For a list of things I can do to help you, just type:

@whedon commands

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

:point_right: Check article proof :page_facing_up: :point_left:

[gradvohl]

Review items for JOSS paper #1494

Paper title: mTower: Trusted Execution Environment for MCU-based devices

Review criteria

• A list of the authors of the software: :heavy_check_mark:
• Author affiliations: :heavy_check_mark:
• A summary describing the high-level functionality and purpose of the software for a diverse, non-specialist audience: :heavy_check_mark:
• A clear statement of need that illustrates the purpose of the software: :heavy_check_mark:
• Mentions (if applicable) of any ongoing research projects using the software or recent scholarly publications enabled by it: :heavy_check_mark:
• A list of key references including a link to the software archive: :heavy_exclamation_mark: :arrow_heading_up: (Needs improvements)

Review itens

• Software license: :exclamation::question:

• Documentation:

  • A statement of need: :thumbsup: (good)
  • Installation instructions: :wave: (sort of)
  • Example usage: :thumbsdown: (bad)
  • API documentation: :thumbsup: (good)
  • Community guidelines: :thumbsup: (good)
  • Functionality: :confused:
  • Tests: :thumbsdown: (bad)

• Software paper

  • Authors: :exclamation::question:
  • A statement of need: :heavy_check_mark:
  • References: :heavy_exclamation_mark: :arrow_heading_up: (Needs improvements)

Reviewer comments

The following comments refer to items that I considered were not met in full when I reviewed this submission.

The authors mention two licenses in the repository. While the repository itself uses the Apache 2.0 license, the README.md text, in section 2, indicates the BSD-2 license.

The first author made many contributions to the repository, although he was not the one who made the most contributions. Regarding the list of authors of the paper, it seems that one of them -- Oleg Kopysov -- did not contribute to the repository.

Regarding the references, I found only references to the specification of the TEE API or the TEE architecture. Still, to download these references, you need to fill out a form. I believe that the authors could add other references. For instance, references that better explore the security issues that mTower intends to address or references to discuss the importance of FreeRTOS as the operating system for these MCU-based devices.

Considering the examples of use, I have not found any well-documented examples that illustrate the operation of mTower. I saw that there are some directories with examples, but they are scattered in the repository and there is not a step-by-step guide to running the examples.

Concerning the functionalities, unfortunately, I do not have the specific platform the software runs to execute some tests. In this sense, it would be interesting -- although I do not consider it mandatory -- if the authors indicated a container or a virtual machine where the users could run some experiments.

Finally, I could not find mechanisms for automatic testing or test results. I suggest trying to add some tests and results reports.

Conclusion

Analyzing all the aspects mentioned earlier, I classify the paper in the category of major revisions.

[gkthiruvathukal]

@gradvohl Thank you for your detailed review.

We will await @federeghe's review but it's clear there are some major issues to be addressed by @omoliavko before we can proceed.

[federeghe]

I checked the LICENSE and COPYING file verifying that all the used software is compliant with respect to the Apache license. However, not only the README file uses the BSD license (instead of the Apache of the LICENSE file) but also some source code files have BSD license in the header. Please make the license uniform across files and documentation.

I did not perform a in-depth review because I have the same concerns of @gradvohl. In particular:

• The submitter to JOSS is not the author that made a major contribution to the software. Actually, @omoliavko pushed commits only about the JOSS paper. However, I see that there are some large commits from @tdrozdovsky, that are possibly multi-author works. I think that a confirmation from @tdrozdovsky about the software authorship may be sufficient to solve this issue.
• There are some examples in apps folder, but it is not clear how to build them.
• Please expand the documentation on how to add a new architecture, in particular about the apps folder and related files.
• As outlined by @gradvohl, neither automated nor manual tests are present. They are required by the journal policy.

[tdrozdovsky]

Thanks for the constructive comments! Soon we'll make changes according to them.

[federeghe]

@gkthiruvathukal I'm no more able to flag checkboxes or change the first post. It seems I'm no more a collaborator of the joss-reviews repo.

[omoliavko]

Dear @federeghe, @gradvohl, @gkthiruvathukal, thank you for your effort. I've opened tickets in github to address the issues you've found with mTower:
https://github.com/Samsung/mTower/issues/4
https://github.com/Samsung/mTower/issues/5
https://github.com/Samsung/mTower/issues/6
https://github.com/Samsung/mTower/issues/7
https://github.com/Samsung/mTower/issues/8
https://github.com/Samsung/mTower/issues/9
https://github.com/Samsung/mTower/issues/10
We'll get to fixing these issues as soon as possible.
Regarding testing and functionality demonstration - yes, unfortunately, mTower can for now only run on specific H/W platform, but in the future we intend to create a virtual machine (probably QEMU-based) that can demonstrate mTower operation on generic Linux-based PC. However, QEMU does not currently support TrustZone emulation for ARM Cortex M family of MCUs.

[omoliavko]

@whedon commands

[whedon]

Here are some things you can ask me to do:

# List Whedon's capabilities
@whedon commands

# List of editor GitHub usernames
@whedon list editors

# List of reviewers together with programming language preferences and domain expertise
@whedon list reviewers

EDITORIAL TASKS

# Compile the paper
@whedon generate pdf

# Compile the paper from alternative branch
@whedon generate pdf from branch custom-branch-name

# Ask Whedon to check the references for missing DOIs
@whedon check references

[omoliavko]

@whedon generate pdf

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

:point_right: Check article proof :page_facing_up: :point_left:

[omoliavko]

@whedon generate pdf

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

:point_right: Check article proof :page_facing_up: :point_left:

[omoliavko]

Dear @federeghe, @gradvohl, @gkthiruvathukal, @tdrozdovsky, we have fixed the issues with mTower, except test suite - writing a comprehensive test suite would be very time-consuming due to hardware limitations (it would have to consist of some sort of external shell that would flash, run and verify tests for individual functions). Most of functionality can be checked by example applications already included into mTower source code on gitHub. Could you please resume the review process?

[gkthiruvathukal]

@omoliavko Thanks for the follow up. I think I am comfortable with your suggestion. To be clear, however, are there actual tests present? I don't see any reason why there cannot be some tests, even if the tests need to run on specific hardware. As long as this is documented, I think it is ok to say that the tests need the specific hardware platform in order to run (at this stage of development). That way, you do not need to put so much work into an emulation layer, which I agree would be too much work at this stage.

[omoliavko]

@gkthiruvathukal Ok, we will try to prepare a test sequence showing the API operation under standard scenarios and a documentation that describes this test sequence. If we can do anything else to improve the mTower and the article, please let us know.

[gkthiruvathukal]

@omoliavko I think that is all we need to move toward acceptance. Please let all of us (me and the reviewers) know when the test sequence/scenarios and documentation are ready.

[omoliavko]

@gkthiruvathukal @federeghe @gradvohl We have added a document describing test suite for mTower - it can be found at docs/mtower_test_suite_description.md in project repository. Tests cover the client API functions and some of internal API functions. Some of the functions are difficult to test because they are supposed to be critical, that is, their failures are unrecoverable, and if they fail, system is supposed to halt. Note that due to potential licensing issues we had to remove GP specifications that were included into repository before. These specifications can still be obtained from GP web site after free registration.

[gkthiruvathukal]

@federeghe @gradvohl Can you let me know if you have any more issues? I'm going to assume you are ok, given that @omoliavko has addressed our last point of feedback.

So here is the final checklist for @omoliavko to fill in. Can you please do the following? (This is something I am doing for all submissions now, so you can just check the ones you already did above.)

• [ ] Make a tagged release of your software, and list the version tag of the archived version here.
• [ ] Archive the reviewed software in Zenodo
• [ ] Check the Zenodo deposit has the correct metadata, this includes the title (should match the paper title) and author list (make sure the list is correct and people who only made a small fix are not on it); you may also add the authors' ORCID.
• [ ] List the Zenodo DOI of the archived version here.

Once these are done, I will do a final proofread and signal the JOSS EICs to process acceptance.

[gradvohl]

@whedon generate pdf

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

:point_right: Check article proof :page_facing_up: :point_left:

[gradvohl]

Good Morning,

The authors addressed most of the issues I pointed. However, I still find some copyrighted code, such as tee_client/public/teec_trace.h and mTower/tee_client/libteec/src/tee_client_api.c. Is it a problem for JOSS?
Regarding the other issues, I consider that the authors addressed them.
Best regards.

[gkthiruvathukal]

@gradvohl This would be a problem for JOSS, especially if the code does not belong to the authors. Even if it does, the licensing needs to be clear.

@omoliavko Can you please clarify the status of the tee_client code?

[omoliavko]

@gkthiruvathukal We've rechecked licensing terms and all of the components used in mTower were originally published under BSD or Apache licenses that permit reuse or modification of code with attribution. Third-party components are used in mTower because they are essentially reference implementations of Open TEE standard, and we aimed to maintain maximal compatibility with existing Trusted Applications developed for ARM Cortex A series CPUs - it was necessary to perform our experiments.

[gkthiruvathukal]

@omoliavko Thank you for the update. My last question is whether this is clearly documented somewhere in your code. Are the licenses for these codes included in the folder containing them?

[omoliavko]

@gkthiruvathukal Each source code file contains its own licensing info. Also, there is a COPYING file in mTower project root folder that already contains this info for entire project.

[gkthiruvathukal]

@omoliavko So can you go to my earlier checklist and check off all of the items? I believe we are at a point where you can prepare the Zenodo archive and tag the current release.

[omoliavko]

@gkthiruvathukal I believe I can complete the checklist quickly if needed. However, if I understand correctly, after I complete the checklist, the article will be finalized, and no further edits will be possible. Are the reviewers satisfied with the current state of the article?

[gkthiruvathukal]

@omoliavko I believe all issues have been addressed thus far. Let's do this. If @gradvohl and @federeghe have any comments, I'd like to receive these by end of week. If there are no objections by the weekend (any time on earth) I will ask you to tag the release and create the Zenodo archive. We should be able to wrap up (and accept) this submission soon. Thanks for your patience and diligence to work on all of the issues raised during review.

[gradvohl]

I have no other comments. Best regards.

[gkthiruvathukal]

Thanks, @gradvohl. I am ready to move toward acceptance.
@omoliavko Can you please provide the release tag for the current (reviewed with all changes addressed version) and make a Zenodo archive?

[omoliavko]

@gkthiruvathukal I'll do that on Wednesday, Aug 7 - we plan to add an important test to source code, and then mark the version of code with a tag.

[omoliavko]

@gkthiruvathukal Sorry for the delay, we've deposited v0.2.0 source code on Zenodo:
https://zenodo.org/record/3366076#.XVFc80czaUk
DOI generated is:
10.5281/zenodo.3366076

Most relevant change in code is addition of example that demonstrates operation of hardware security mechanism in ARM Trustzone that generates exceptions on operations that should be blocked.

[ooo[bot]]

:wave: Hey @omoliavko...

Letting you know, @gkthiruvathukal is currently OOO until Sunday, August 18th 2019. :heart:

[gkthiruvathukal]

@whedon set 10.5281/zenodo.3366076 as archive

[whedon]

OK. 10.5281/zenodo.3366076 is the archive.

[gkthiruvathukal]

@whedon set v0.2.0 as release

[gkthiruvathukal]

@whedon check references

[whedon]

Attempting to check references...

[whedon]

```Reference check summary:

OK DOIs

• None

MISSING DOIs

• None

INVALID DOIs

• None
  ```

[gkthiruvathukal]

@whedon generate pdf

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

:point_right: Check article proof :page_facing_up: :point_left:

[gkthiruvathukal]

@openjournals/joss-eics I'm ready to accept this submission, pending your final checks.

[kyleniemeyer]

Hi @omoliavko, please merge these PRs with some copy edits for the paper: https://github.com/Samsung/mTower/pull/18 and https://github.com/Samsung/mTower/pull/19.

(For the latter PR, I noticed that nearly all of the references were "misc" entries with just URLs, even though some were articles or reports that need more information for a proper citation. Some of the titles did not match the things being cited, as well.)

[tdrozdovsky]

Hi @kyleniemeyer, I have merged your commits and deleted the broken link. Thank you for your support.

[tdrozdovsky]

@whedon generate pdf

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

:point_right: Check article proof :page_facing_up: :point_left:

[kyleniemeyer]

@whedon accept

[whedon]

Attempting dry run of processing paper acceptance...

[whedon]

Check final proof :point_right: https://github.com/openjournals/joss-papers/pull/931

If the paper PDF and Crossref deposit XML look good in https://github.com/openjournals/joss-papers/pull/931, then you can now move forward with accepting the submission by compiling again with the flag deposit=true e.g.
@whedon accept deposit=true

[kyleniemeyer]

@whedon accept deposit=true

[whedon]

Doing it live! Attempting automated processing of paper acceptance...

[whedon]

🐦🐦🐦 👉 Tweet for this paper 👈 🐦🐦🐦

[whedon]

🚨🚨🚨 THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! 🚨🚨🚨

Here's what you must now do:

1. Check final PDF and Crossref metadata that was deposited :point_right: https://github.com/openjournals/joss-papers/pull/932
2. Wait a couple of minutes to verify that the paper DOI resolves https://doi.org/10.21105/joss.01494
3. If everything looks good, then close this review issue.

4. Party like you just published a paper! 🎉🌈🦄💃👻🤘

   Any issues? notify your editorial technical team...

[kyleniemeyer]

Congrats @omoliavko on your article's publication in JOSS! Many thanks to @gradvohl and @federeghe for reviewing, and @gkthiruvathukal for editing, this submission.

[whedon]

:tada::tada::tada: Congratulations on your paper acceptance! :tada::tada::tada:

If you would like to include a link to your paper from your README use the following code snippets:

Markdown:
[![DOI](https://joss.theoj.org/papers/10.21105/joss.01494/status.svg)](https://doi.org/10.21105/joss.01494)

HTML:
<a style="border-width:0" href="https://doi.org/10.21105/joss.01494">
  <img src="https://joss.theoj.org/papers/10.21105/joss.01494/status.svg" alt="DOI badge" >
</a>

reStructuredText:
.. image:: https://joss.theoj.org/papers/10.21105/joss.01494/status.svg
   :target: https://doi.org/10.21105/joss.01494

This is how it will look in your documentation:

We need your help!

Journal of Open Source Software is a community-run journal and relies upon volunteer effort. If you'd like to support us please consider doing either one (or both) of the the following:

• Volunteering to review for us sometime in the future. You can add your name to the reviewer list here: https://joss.theoj.org/reviewer-signup.html
• Making a small donation to support our running costs here: https://numfocus.salsalabs.org/donate-to-joss