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

PSA storage: Conform to "PSA 1.0.0" spec release #10847

Merged
merged 1 commit into from
Jul 23, 2019

Conversation

davidsaada
Copy link
Contributor

@davidsaada davidsaada commented Jun 17, 2019

Description

PSA storage: Conform to "PSA 1.0.0" spec release

  • Add the no confidentiality & no replay protection flags
  • Add actual size parameter in PS/ITS get APIs
  • Change a few size parameters from uint32_t to size_t

Pull request type

[ ] Fix
[ ] Refactor
[ ] Target update
[x] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@jenia81 @shelib01

Release Notes

PSA Storage: Add the integrity only and no replay protection flags.

@ciarmcom ciarmcom requested review from jenia81, shelib01 and a team June 17, 2019 15:00
@ciarmcom
Copy link
Member

@davidsaada, thank you for your changes.
@shelib01 @jenia81 @ARMmbed/mbed-os-maintainers please review.

@davidsaada
Copy link
Contributor Author

@shelib01 @jenia81 Changed the the PSA_STORAGE_FLAG_INTEGRITY_ONLY flag to PSA_STORAGE_FLAG_NO_CONFIDENTIALITY to support the upcoming change in the spec.

@@ -95,14 +95,20 @@ psa_status_t psa_ps_set(psa_storage_uid_t uid, uint32_t data_length, const void
}

uint32_t kv_create_flags = def_kvstore_flags;
if (create_flags & PSA_STORAGE_FLAG_INTEGRITY_ONLY) {
kv_create_flags &= ~(KVStore::REQUIRE_CONFIDENTIALITY_FLAG | KVStore::REQUIRE_REPLAY_PROTECTION_FLAG);

Choose a reason for hiding this comment

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

@davidsaada , if PSA_STORAGE_FLAG_INTEGRITY_ONLY flag is on, the item should be not encrypted but rollback protected

@Patater
Copy link
Contributor

Patater commented Jun 21, 2019

"the latest changes in the PSA storage spec"

Could you please specify the spec version and preferably link to it in the commit message description?

Thanks

@davidsaada
Copy link
Contributor Author

"the latest changes in the PSA storage spec"

Could you please specify the spec version and preferably link to it in the commit message description?

Thanks

Sorry, should have been more specific. Not sure whether this was officially released or not, but it was added to the master branch of the PSA storage repo, following this PR.

@Patater
Copy link
Contributor

Patater commented Jun 21, 2019

Thanks, @davidsaada. Now I can review these changes against the spec.

It'd still be preferred to update the git commit message description with a link to the PSA storage repo at the commit hash you are updating to. It's been hard to find out which version of the PSA Storage APIs Mbed OS provided in the past, and having descriptive commit messages would go a long way in helping with this.

Copy link
Contributor

@Patater Patater left a comment

Choose a reason for hiding this comment

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

The changes look mostly fine. I found a few places where our implementation was not updated to match the specification version linked.

The biggest problem with this PR is a lack of testing.

This PR needs some more unit tests. It also needs the PSA compliance tests to be updated, in order to continue passing the PSA storage compliance tests.

  1. Please update and ensure the unit tests pass https://github.com/ARMmbed/mbed-os/blob/master/TESTS/psa/its_ps/main.cpp (they still use uint32_t, not size_t yet)
  2. Please make sure these compliance tests are updated and pass https://github.com/ARMmbed/mbed-os/tree/master/components/TARGET_PSA/TESTS/compliance_its
  3. Please make sure these compliance tests are updated and pass https://github.com/ARMmbed/mbed-os-psa-compliance-tests-example/
  4. Please add testing of flags to https://github.com/ARMmbed/mbed-os/blob/master/TESTS/psa/its_ps/main.cpp

@@ -47,7 +47,7 @@ psa_status_t psa_its_set(psa_storage_uid_t uid, uint32_t data_length, const void
return res;
}

psa_status_t psa_its_get(psa_storage_uid_t uid, uint32_t data_offset, uint32_t data_length, void *p_data)
psa_status_t psa_its_get(psa_storage_uid_t uid, size_t data_offset, size_t data_length, void *p_data)
Copy link
Contributor

Choose a reason for hiding this comment

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

According to the spec you referenced that we are updating to, this function should be:

psa_status_t psa_its_get(psa_storage_uid_t uid,
                         size_t data_offset,
                         size_t data_size,
                         void *p_data,
                         size_t *p_data_length);

It looks like the p_data_length parameter needs adding in order to be spec compliant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will change.

uint32_t data_offset,
uint32_t data_length,
size_t data_offset,
size_t data_length,
void *p_data);
Copy link
Contributor

Choose a reason for hiding this comment

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

According to the spec version you pointed at, psa_ps_get() needs updating to match the following function signature:

psa_status_t psa_ps_get(psa_storage_uid_t uid,
                        size_t data_offset,
                        size_t data_size,
                        void *p_data,
                        size_t *p_data_length );

It looks like p_data_length is currently missing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will change.

#define PSA_STORAGE_FLAG_WRITE_ONCE (1 << 0) /**< The data associated with the uid will not be able to be modified or deleted. Intended to be used to set bits in `psa_storage_create_flags_t`*/
#define PSA_STORAGE_FLAG_NONE 0 /**< No flags to pass */
#define PSA_STORAGE_FLAG_WRITE_ONCE (1 << 0) /**< The data associated with the uid will not be able to be modified or deleted. Intended to be used to set bits in `psa_storage_create_flags_t`*/
#define PSA_STORAGE_FLAG_NO_CONFIDENTIALITY (1 << 1) /**< The data associated with the uid is public and therefore does not require confidentiality. It therefore only needs to be integrity protected */
Copy link
Contributor

Choose a reason for hiding this comment

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

According to the spec linked, this should be PSA_STORAGE_FLAG_INTEGRITY_ONLY, not PSA_STORAGE_FLAG_NO_CONFIDENTIALITY.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please look at the up to date version of storage_common.h in master. It holds the PSA_STORAGE_FLAG_NO_CONFIDENTIALITY value.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could we ask for a release tag in psa_trusted_storage_api? The latest release, Release-PSA1.0RC2 looks out of date.

Then, we can reference the fresh tag from this PR's commits so it's clear which version of the PSA Storage API Mbed OS implements.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Asked for it in there.

uint32_t data_offset,
uint32_t data_length,
size_t data_offset,
size_t data_length,
void *p_data);
Copy link
Contributor

Choose a reason for hiding this comment

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

Per spec, we should have a size_t *p_data_length parameter here (for psa_its_get())

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will change.

// In case we're on external storage, need to add some logics in order to remove missing flags
if (def_kvstore_flags) {
if ((kv_get_flags & ~KVStore::REQUIRE_CONFIDENTIALITY_FLAG) == kv_get_flags) {
p_info->flags |= PSA_STORAGE_FLAG_NO_CONFIDENTIALITY;
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this trying to do?

From the looks of it, if all bits are set in kv_get_flags except just the one single REQUIRE_CONFIDENTIALITY_FLAG bit, we'll set PSA_STORAGE_FLAG_NO_CONFIDENTIALITY. However, if both REQUIRE_CONFIDENTIALITY_FLAG and REQUIRE_REPLAY_PROTECTION_FLAG are missing from kv_get_flags, it appears neither of the PSA flags would get set.

Am I reading that right? Is that the intended behavior? Please make sure there is an automated test to ensure this works as intended.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, need to shed some light here.
First of all, the spec chose to work in reverse logic comparing to the KVStore implementation, in regards to the confidentiality an replay protection flags. Meaning that KVStore assumes these flags are clear, while the PSA storage spec assumes they are set. Reason is backward compatibility.
Then, there's the difference between ITS & PS and in PS, between internal and external storage. These flags are ignored in ITS & in PS, when the latter resides in internal storage. Reason is that internal storage doesn't require neither confidentiality nor replay protection. The def_kvstore_flags tells us the default flags set for KVStore - added confidentiality and replay protection in the external PS case, none in the ITS and internal PS case. So, the only time we need to handle this logic is when def_kvstore_flags (set in init) is non-zero.

Copy link
Contributor

Choose a reason for hiding this comment

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

If an application requests that it get replay protection, the protected storage implementation is not allowed to satisfy that request with storage that isn't replay protected, whether the backing storage is internal or otherwise. This is a good feature, because the algorithms employed atop replay-protected storage are often quite different from those employed atop non-replay-protected storage; applications that assume they are getting replay protected storage may become less secure due to the algorithms they employ, if the underlying storage changes to non-replay-protected.

The implementation is permitted to treat these flags as indicative and to apply a higher level of protection if it does not implement every protection class. It MUST NOT apply a lower level of protection than that selected.

See the spec

How can we make sure we don't "apply a lower level of protection than that selected"? I think we need an automated test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Our secure storage model refers to the external storage as the one that needs to be protected (replay & confidentiality), while our internal storage is considered to be safe in terms of the threat model. According to this model, if our protected storage resides on the external storage (depending on the board's characteristics), these security measures should be applied. However, if protected storage resides on the internal storage, these security measures are redundant (and even harmful, as they require lots of storage space and CPU resources). Replay protection, for instance, uses the internal storage for keeping the tags for each stored record, so using it in the internal storage case is quite absurd. So we don't actually "apply a lower level of protection than that selected" in this case.

Copy link
Contributor

@Patater Patater Jun 25, 2019

Choose a reason for hiding this comment

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

An interesting perspective on compliance!

@MarcusJGStreets Would it be a spec violation to provide internal storage that isn't roll-back protected to an application that requested PSA Protected Storage with roll-back protection? It's obviously a violation if we provide external storage that is not roll-back protected in such a case, but what if we serviced the storage request with internal storage where it's technically not roll-back protected? Are there some security guarantees the application is looking for that would no longer be provided if we serviced the application's request for rollback-protected storage with non-rollback-protected internal storage?

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, internal storage is considered inherently roll-back protected.

For Protected Storage that's implemented using internal flash: could we just call ITS functions instead of having special handlers for both internal and external KvStore flash?

This still needs automated tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ITS is not always equal to PS over internal storage. In boards that have real secure and non secure partitioning, PS will be mapped to the internal storage on the non secure side, while ITS will be mapped to the internal storage on the secure side (and these have different address spaces).
Regarding automated tests: the ps_its test will test the internal storage case for boards configured to have internal storage as their primary storage, such as K66F.

}
if (create_flags & PSA_STORAGE_FLAG_NO_REPLAY_PROTECTION) {
kv_create_flags &= ~KVStore::REQUIRE_REPLAY_PROTECTION_FLAG;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

What if def_kvstore_flags is missing REQUIRE_CONFIDENTIALITY_FLAG and the PSA create_flags is missing PSA_STORAGE_FLAG_NO_CONFIDENTIALITY? Don't we want to enable additional bits in def_kvstore_flags to ensure that the PSA set that requires confidentiality would get it?

Since this is tricky logic, could you point me to the automated test that ensures this works correctly or create one if it doesn't exist yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As explained above, the PSA_STORAGE_FLAG_NO_CONFIDENTIALITY was replaced to PSA_STORAGE_FLAG_NO_REPLAY_PROTECTION.

Copy link
Contributor

Choose a reason for hiding this comment

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

This looks better now, thanks!

@MarcusJGStreets
Copy link

MarcusJGStreets commented Jun 25, 2019 via email

@davidsaada davidsaada force-pushed the david_ps_add_sec_flags branch 2 times, most recently from 7473a3f to d9c4889 Compare June 26, 2019 08:59
@davidsaada
Copy link
Contributor Author

@Patater Thanks for the thorough review.

Pushed a fix with the following now:

  • Fixes for all missed PSA spec repo changes, namely missing actual size parameter in get function and size_t instead of uint32_t types.
  • Added testing of flags in the its_ps test.
  • Fixed logic of flag handling where required.

Tested on K64F (both default SD mode, covering external storage and without SD, simulating internal storage only) with its_ps test and all current ITS compliance tests.

@@ -5,7 +5,8 @@

int mbed_default_seed_read(unsigned char *buf, size_t buf_len)
{
psa_status_t rc = psa_its_get(PSA_CRYPTO_ITS_RANDOM_SEED_UID, 0, buf_len, buf);
size_t actual_size;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we do anything with actual_size after the call to get?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't think so. We could check that the returned size is the same as the one we expect, but this seems a bit overcautious to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is for reading entropy, the foundation upon which all cryptography in the system relies for security. It's worth the caution!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, good point. Will add the check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, looking at the code, I see that this function is placed at a very deep layer, while the actual entropy algorithm is in a much higher layer. Not sure that how legitimate it is to check for the expected random seed size at this deep layer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To add to that, this is really out of scope for this PR. One could perform this check using the get_info API as well. The returned actual size argument from psa_its_get is just a more convenient way to achieve this value.

@@ -35,7 +35,8 @@ using namespace mbed;

static const uint32_t delete_flag = (1UL << 31);
static const uint32_t internal_flags = delete_flag;
static const uint32_t supported_flags = KVStore::WRITE_ONCE_FLAG;
// Only write once flag is supported, other two are kept in storage but ignored
static const uint32_t supported_flags = KVStore::WRITE_ONCE_FLAG | KVStore::REQUIRE_CONFIDENTIALITY_FLAG | KVStore::REQUIRE_REPLAY_PROTECTION_FLAG;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is TDBStore the internal flash storage for PS?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes (among the rest).


status = get_info_func(stype, 6, &info);
TEST_ASSERT_EQUAL(PSA_SUCCESS, status);
TEST_ASSERT_EQUAL(flags, info.flags);
Copy link
Contributor

Choose a reason for hiding this comment

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

How do we know that the KvStore flags are correct in this test? Should we also do an assert to make sure KVStore::REQUIRE_REPLAY_PROTECTION_FLAG is clear in kv_info.flags, to ensure our translation went properly?

I understand the get_info_func() as currently implemented will do the translation from kv_info.flags to PSA flags, but if in the future that get_info_func() gets a bug or simply replies with previously saved PSA flag values, we would miss out on test coverage. From a defensive programming point of view, we should check directly what we want to know about what KvStore is doing; it'll be a more robust check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, this test is a kind of a black box test, in the sense that we work with what the APIs provide us (PSA flags in that case). What you suggest here is a "white box" test, which tests the various impacts of the APIs on the internal components. Seems a bit excessive to me - this should IMO be part of the unit tests. Otherwise, one could argue that we shouldn't only check the KVStore flags, but also the key name, sizes etc.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you can think of a way to ensure that the correct flags took actual effect (for example, that storage is actually encrypted when needed, not encrypted when using internally-backed PS storage) with a black box test, that'd work too. I think whitebox would be easier, though, if you trust the underlying KvStore tests. As written now, the test is too easy to fool.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

KVStore tests (namely the SecureStore test) check that the actual encryption is performed on storage. But yet again, saying that the test is "easy to fool" can be applied on many other part of this test (for instance, checking that the stored key is what we expect, not only the flags) or other tests as well. My point here is that the KVStore flags shouldn't be "superior" in that sense.

Copy link
Contributor

@Patater Patater Jun 26, 2019

Choose a reason for hiding this comment

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

"KVStore tests (namely the SecureStore test) check that the actual encryption is performed on storage" given that it uses the flag that tells it to do the encryption. However, we have no visibility from the PSA level that KvStore will use the right flags. We don't have any test that if PSA requests encryption, KvStore will use encryption. We have a test that PSA will be told that the encryption flag it asked for was in fact asked for and remembered, but no test that the KvStore will listen to that flag. We could get a better guarantee of this if we check the KvStore flags used for the storage backing the request from PSA.

We don't need to do integration testing to check that the storage is actually encrypted, but we do need at a minimum to unit test flag translation (psa_ps_set().)

Could we break out the flag translation logic from psa_ps_set() so that we can test PSA flags as input, and KvStore flags as output, ensuring that for all combinations of PSA input flags, we get the expected KvStore flags? The test would not ask KvStore for anything, nor would it be dependent on physical storage or any of its properties. We should do the same for the flag translation logic in psa_ps_get_info(), breaking it out into its own function so that we can test it in isolation from KvStore.

Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, but I really have to disagree. What you are actually suggesting here is to expose the implementation of KVStore flags to the outside world. However, this is part of the internal implementation of psa_ps_set and psa_ps_get_info APIs. Exposing this implementation to the outside world gives us nothing other than the ability to test the implementation, which again is something that should be tested by unit tests. However, the real downside here is that if we decide at some point to change the implementation of these APIs (for instance, to abandon KVStore), we will have a much harder time to do that following the exposure of these flags, as it would lead to a breaking change. This is not far fetched: the initial implementation of the device key used NVStore, which was later replaced by KVStore.
If you are worried about how the KVStore flags behave when calling these APIs, I can attach their print log following these actions. Don't think anything beyond this is required.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm suggesting to test the flag translation logic. This is not currently being done adequately. Exposing the KVStore flag translation functions to the outside world isn't required as only unit tests need to use it.

If KvStore is no longer used in the future, then the flag translation functions no longer need testing and the tests can be changed or removed. It's pretty common to update tests at the same time an implementation changes significantly.

The flag translation is quite tricky and deserves testing in isolation with a unit test. Crypto and many other services and applications depend on storage to provide the security guarantees they request from storage. The impact of a bug in this area is quite high, so please adequately test this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Translation logic is really not that complicated - two flags that use reverse logic in PSA Storage & KVStore. Translation of uid to KVStore key is much more complex, to give one example. Breaking the implementation to a function that exposes an internal implementation in a header file (even if only internally used by test code) seems like a total overshoot to me. That's what unit testing is for IMHO.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's what unit testing is for IMHO.

Yes, I'm only asking for a unit test of the flag translation logic. High risk of trouble if this translation code does something wrong, even if we don't think it's that complicated.

Requesting unit tests for flag translation logic shouldn't also require adding unit tests for uid to KVStore keys. Although you really should have unit tests for that as well, given that you've shipped that already, I'd say fixing it is out of scope of this PR. I would like to prevent more code from being added to Mbed OS that isn't sufficiently tested, however.

Where are the unit tests? Not part of Mbed OS? Someplace else?


status = get_info_func(stype, 6, &info);
TEST_ASSERT_EQUAL(PSA_SUCCESS, status);
TEST_ASSERT_EQUAL(flags, info.flags);
Copy link
Contributor

Choose a reason for hiding this comment

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

How do we know that the KvStore flags are correct in this test? Should we also do an assert to make sure both KVStore::REQUIRE_REPLAY_PROTECTION_FLAG and KVStore::REQUIRE_CONFIDENTIALITY_FLAG are clear in kv_info.flags, to ensure our translation went properly?

}
if (create_flags & PSA_STORAGE_FLAG_NO_REPLAY_PROTECTION) {
kv_create_flags &= ~KVStore::REQUIRE_REPLAY_PROTECTION_FLAG;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks better now, thanks!

@Patater
Copy link
Contributor

Patater commented Jun 26, 2019

We still need a spec version to link to in the commit message.

@davidsaada
Copy link
Contributor Author

We still need a spec version to link to in the commit message.

@MarcusJGStreets Do we have it yet?

@Patater
Copy link
Contributor

Patater commented Jul 3, 2019

We still need a spec version to link to in the commit message.

@MarcusJGStreets Do we have it yet?

Yes, we have it now. See https://github.com/ARMmbed/psa_trusted_storage_api/releases/tag/v1.0.0

@jenia81
Copy link

jenia81 commented Jul 10, 2019

@Patater @davidsaada We need this PR to be merged to Mbed OS master.
I understand that spec revision exists already - so I guess this can be solved
Can you please agree regarding unit tests and merge this PR in?

Copy link

@MarcusJGStreets MarcusJGStreets left a comment

Choose a reason for hiding this comment

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

Changes look good to me.

@davidsaada davidsaada force-pushed the david_ps_add_sec_flags branch 2 times, most recently from 5b2b19f to a04e35a Compare July 14, 2019 15:48
- Add the no confidentiality & no replay protection flags
- Add actual size parameter in PS/ITS get APIs
- Change a few size parameters from uint32_t to size_t
@davidsaada davidsaada changed the title PSA storage: Implement additional flags, change ints to size_t PSA storage: Conform to "PSA 1.0.0" spec release Jul 14, 2019
@davidsaada
Copy link
Contributor Author

  • Changed commit message (and PR description) to include correct PSA storage spec version.
  • Modified the its_ps test to make sure that KVStore flags are set with the right values (Used implementation from common psa_storage_get_info_impl function, which exposes the kvstore flags - FYI @Patater).

@jenia81
Copy link

jenia81 commented Jul 22, 2019

any estimation when this PR can be merged?

@SeppoTakalo
Copy link
Contributor

@jenia81 Thanks for asking. I'll start the CI tests.

However, I see that you are listed in original "reviewers" field, but not approved yet. Please review & approve. Once CI passes and your review is in, I can merge this. Thanks.

@jenia81
Copy link

jenia81 commented Jul 22, 2019

@SeppoTakalo Done

@mbed-ci
Copy link

mbed-ci commented Jul 22, 2019

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 1
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.

None yet

10 participants