Use singleton to avoid multiple Context allocation

[sarroutbi]

No description provided.

[codecov]

Codecov Report

Attention: Patch coverage is 38.23529% with 21 lines in your changes missing coverage. Please review.

Project coverage is 62.57%. Comparing base (4f1201a) to head (6aad744).
Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
keylime-push-model-agent/src/main.rs	38.23%	21 Missing ⚠️

Additional details and impacted files

Flag	Coverage Δ
e2e-testsuite	62.57% <38.23%> (-0.09%)	⬇️
upstream-unit-tests	62.57% <38.23%> (-0.09%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
keylime/src/context_info.rs	47.45% <ø> (ø)
keylime/src/tpm.rs	71.34% <ø> (ø)
keylime-push-model-agent/src/main.rs	70.33% <38.23%> (-9.01%)	⬇️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[sarroutbi]

/packit retest-failed

[sarroutbi]

@ansasaki : Please, do not pay attention to the coverage reports. There seems to be errors with latest change.

[ansasaki]

I was trying to understand why these are fixed in the context. I think you do not need to follow the pull agent so much, in the sense that there we had a single set of algorithms to be configured, while in the push model the agent will negotiate the parameters.

For me, it would make more sense if the context held the supported algorithms obtained from the TPM capabilities.

Then, after negotiating the algorithms with the verifier, you should use this to decide which keys to generate/load and send the requested AK.

[sarroutbi]

When you create an AK, you need to specify a particular one:

            let ak = self.tpm_context.create_ak(
                self.ek_handle,
                self.tpm_hash_alg,
                key_alg_for_ak,
                sign_alg,
            )?;

However, the amount of AKs that can be load is relatively small in TPM. That is the reason why I decided to follow this approach. Keeping the signing algorithm is just for BW compatibility. There is one AK generated for each RSA supported signing scheme.
So, if we change this ... how would you approach about this? A combination of all the options?

[ansasaki]

OK, sorry for my last message, I think I was probably tired by the end of the day and could not understand the problem here. Now I see the problem.

I think the Agent should be configurable so that the user can decide which keys to offer, with the possibility of offering multiple keys if they want. So, in my mind, we should allow the user to configure a list of AK to offer, which can be:

• by setting the parameters to generate the key
• by setting the key handle, if it was persisted
• by setting a file path to load a previously generated key (maybe this will be part of the first option as usually we want to persist the generated keys somewhere)

We also should have a default option where it will offer a single key using a default set of algorithms. I think this is the easiest way for you to move forward (basically implement the default case, which is to offer a single AK key that is loaded from a default location or generated if the file is not there, like it is today in the pull agent).

I think we have to discuss with HPE guys and agree how this should work in the final form.

We could use JSON strings to specify the keys parameters or create a specific configuration language and write the parser for it.

PS.: The default behavior could also be similar to what you are doing, which is basically generate some keys with some combination of algorithms and offer them (probably one RSA and one ECC key would be sufficient), but I don't think it should be all the possible combinations

[sarroutbi]

@ansasaki : Do you think it would be feasible to have a first version that sends one RSA and one ECC, then create all the possible ones from config once config is groomed? This way, we can have something with multiple AKs and deliver proposed changes (persistence and configuration based AKs creation) in other PRs

[ansasaki]

Yes, I think we can proceed like this and make the changes on a future PR

[sarroutbi]

Hello Anderson. After clarifying it, I understand original PR is not required. However, let me keep this open to have the Singleton to the Context Info maintained, as this will reduce allocating Context more than once. If you prefer me to open a newer PR, please, let me know.

[ansasaki]

OK, let's repurpose it to implement the singleton for the Context

[ansasaki]

LGTM