Add alternative test to FCS_CKM.6 to address #235

[jvdsn]

Address #235 by adding a new test which can be used if the volatile or non-volatile memory cannot be directly examined.

Also, Test 3 was slightly updated to change "memory" -> "the memory location" for clarity.

[woodbe]

It looks like this would be in place of #251, but I am not sure that the purpose of most of those changes are captured here. The earlier pull request had points about using key references to check that things were removed since it may not be possible to check memory directly. The way this one is written would seem to remove about all the possible tests if the vendor claims you can't access the memory directly.

Can you please look at https://github.com/DSC-iTC/cPP/pull/251#issuecomment-2000040169 and make sure that this is captured in this pull request? I don't see any issues with this pull request, but I think the two are somewhat in conflict as far as the intent goes. If you capture that here though, we can close the other one and apply this one instead.

Of course one thing to note here is that the Crypto WG may come back with something they want to see at some point in the SD which may override what we do here (but it may not, since we are looking at an implementation-specific SD).

[jvdsn]

It looks like this would be in place of #251, but I am not sure that the purpose of most of those changes are captured here.

Yes, this PR would be in place of #251. To me it looks like #251 would replace the existing procedures for Test 1 and Test 2, which I don't think is the best approach.

The earlier pull request had points about using key references to check that things were removed since it may not be possible to check memory directly. The way this one is written would seem to remove about all the possible tests if the vendor claims you can't access the memory directly.

This PR adds a test, test 4, which is performed if the memory cannot be accessed directly. I believe the steps in this PR's test 4 are equivalent to the steps from #251, but please let me know if I missed any. The reason I chose to add an extra test, instead of replacing the existing test procedures, is because I think the existing procedures are preferable if they can be performed.

Can you please look at #251 (comment) and make sure that this is captured in this pull request? I don't see any issues with this pull request, but I think the two are somewhat in conflict as far as the intent goes. If you capture that here though, we can close the other one and apply this one instead.

I'm having a hard time interpreting the comment. Are the bullet points the result of the discussion? Because some bullet points seem to be in conflict (e.g. bullet 1 and 3). I believe this PR complies with the intent of the first bullet point, with the exception that "keys are directly accessible" -> "memory is directly accessible". This conditional is applied to all three tests though, as all three tests require the memory to be dumped or examined.

Of course one thing to note here is that the Crypto WG may come back with something they want to see at some point in the SD which may override what we do here (but it may not, since we are looking at an implementation-specific SD).

Yes, that makes sense

[woodbe]

The bullets were from a discussion and a summary. I had missed test 4 being added (my bad).

Can you make the tests [conditional] and adjust the conditions for when to use them.

Also I think we need to have some wrapping text in the SD that makes it clear that at least one of these needs to be used to be considered a pass (or something to that effect) since these would all be conditional, but as an either/or.

[jvdsn]

I added the [conditional] and removed the bolding to be more consistent with the rest of the SD. I also added a note to say that each key must be tested

[woodbe]

So to be clear then, this will also close #251.

[jvdsn]

@woodbe I'll check with @yiatsec if this PR addresses all her concerns. I believe it should, but I'll double check.

[woodbe]

I'm sure it will be fine, I just want to make sure before we complete the merge since if we don't cover everything, then we would still need to make more edits. I think we are OK here with these changes (it covers the bullets and I think gets to the intent).