Add definition for NV_Extend

[nathanrossi]

Add a implementation for NV_Extend, which is used with a NV Index defined with the NvIndexType::Extend to provide a PCR equivalent that uses NV memory. The hashing algorithm used is defined by the index name algorithm.

An example and test are included for this function, setting up the NV index with the required type and typical attributes and performing an extend of the index and confirming the resulting hash value.

[Superhepper]

Thank you for the PR. This looks good to me.

[Firstyear]

I think my major thought here is that while the example is really good, it has a few caveats.

First, examples are run during "cargo test". I don't think this will be a problem, but this does differ from the normal tests that exist in the project.

Second, a lot of the example is commented out - the issue with examples sometimes is that while it's great to show usage of the API, it's also important to show the supporting elements. For example I would uncomment everything from let session and below.

Finally as commented, you use a magic number and it's unclear where you derived that from, so it'll be great to explain the origin of that.

[Firstyear]

Where does this magic number come from?

[nathanrossi]

This is an arbitrary NV Index. Specifically this is an index in the same range (0x0150002*) as other examples in this file, with the value being the highest existing number +1 (to avoid conflicts with the other tests). There is nothing about this index/range that is specific to the NV Extend functionality. This is why the value is aliased as a variable and commented out in the example, and nv_index appears as a placeholder.

However perhaps there is a better way to dynamically pick an index for testing with instead of hard coding it?

[Firstyear]

More that if I was reading this, I would think "NvIndexTpmHandle::new" wants an integer but I would not know where to pull that value from. So at least documenting "the value should be an int from X to Y" or "you can get the address by doing Z" would help, because else this is just a magic number.

[nathanrossi]

I think my major thought here is that while the example is really good, it has a few caveats.

First, examples are run during "cargo test". I don't think this will be a problem, but this does differ from the normal tests that exist in the project.

I can copy it to a separate integration test and leave only the important example portions? I did not want to create unnecessary duplication is all.

Second, a lot of the example is commented out - the issue with examples sometimes is that while it's great to show usage of the API, it's also important to show the supporting elements. For example I would uncomment everything from let session and below.

I mostly copied the existing example to make it consistent. While including the details of session configuration may be helpful it adds a lot of somewhat boiler plate session configuration that is not a strictly important detail of how to define a NV index for use with nv_extend.

For example in the application where I am using nv_extend, the extend is performed in a platform auth session against a platform defined NV index, which has a much simpler session to configure:

context.execute_with_session(Some(AuthSession::Password), |ctx| -> Result<()> {
		Ok(ctx.nv_extend(NvAuth::Platform, nv_index_handle, buffer)?)
	})?;

I can update the change to uncomment that portion of the example if you still think it would be ideal to show it?

[Superhepper]

Second, a lot of the example is commented out - the issue with examples sometimes is that while it's great to show usage of the > API, it's also important to show the supporting elements. For example I would uncomment everything from let session and
below.

Previously we have either just copied the existing integration test into the documentation examples and commented out all the stuff that is not directly related to the function call (such as but not limited to, TCTI config loading, Context creation and Session creation) in order to make the examples smaller and easier to read. Some of the APIs are quite complex and requires a lot of things before they can be used which would create examples that would be crazy long. I can agree though that it can be quite deceiving for some one who looks at the documentation and thinks that it looks simple (when in my opinion nothing about the TPM is simple). We should probably add to the library documentation that code in the examples is actually not all the code that is needed in order to actually call the API successfully and that the full code can be found in the integration tests.

I wouldn't mind getting the test duplicated into the integration tests and if it could be done with some variation that would be even better!

Finally as commented, you use a magic number and it's unclear where you derived that from, so it'll be great to explain the
origin of that.

Yeah, guess they can seem magical but it is actually exactly as described by @nathanrossi. Every NV handle used in the tests uses a unique value in the NV handle range. So the way to get a new handle is actually to search for the highest number in the code base and a +1. This is done this way in order to make it easier to determine where a problem regarding NV handles in test originated from.

We should probably also add some kind of Repo Development Guide where this things can be described in order to make it easier for new people to contribute.

[Firstyear]

such as but not limited to, TCTI config loading, Context creation and Session creation) in order to make the examples smaller and easier to read. Some of the APIs are quite complex and requires a lot of things before they can be used which would create examples that would be crazy long. I can agree though that it can be quite deceiving for some one who looks at the documentation and thinks that it looks simple (when in my opinion nothing about the TPM is simple).

Sometimes the session creation is just as important as what method we call :)

Yeah, guess they can seem magical but it is actually exactly as described by @nathanrossi. Every NV handle used in the tests uses a unique value in the NV handle range. So the way to get a new handle is actually to search for the highest number in the code base and a +1.

More than I wouldn't even know where to get this base number from. Can I just pick a random number? Does it have to be within some bounds? I'm trying to think about this from "I've never used an NV Index before".

[Superhepper]

We have specified which specifications that are going to be referenced in the documentation. But in the documentation for the context methods we seldom refer to them. Maybe just add a "for valid ranges for NV handles please see the specifcation".

[Firstyear]

Sounds reasonable to me.

[ionut-arm]

Maybe just add a "for valid ranges for NV handles please see the specifcation".

And ideally mention which specification / section (e.g., Commands spec, section x.y.z). Leaving it as "see the spec" works, but it's a bit more helpful like this for anyone who's not familiar with the beast that is the TPM2 spec.