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

Move nvram_read functions to the XTT library #98

Merged
merged 6 commits into from
Nov 29, 2018

Conversation

kathrynfejer
Copy link
Contributor

@kathrynfejer kathrynfejer commented Nov 27, 2018

This will move the nvram.[h,c] code from xaptum-tpm into xtt, and remove read_nvram from the tool's client in favor of xtt_read_object. This will allow us to read out data of variable length from the TPM.

It also creates 'read_in_from_TPMandread_in_from_files` which read in DAA specific information (TPM or software defined), in an attempt to separate the two different ways to run DAA a bit more.

Fixes #78

Copy link
Collaborator

@zanebeckwith zanebeckwith left a comment

Choose a reason for hiding this comment

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

Few minor requests, but otherwise looking good!

One bigger request: could we add an nvram command to the tool? So that we can use the xtt tool to dump things from the TPMs nvram? So we could do something like:

xtt nvram -t gpk -o gpk.bin

to get the gpk into the file gpk.bin.

The xaptum-tpm project already has this code, in the util directory. Again, we can just move this over.

.travis.yml Show resolved Hide resolved
tool/client.c Outdated Show resolved Hide resolved
tool/client.c Show resolved Hide resolved
@zanebeckwith
Copy link
Collaborator

I just force-pushed, to rebase onto the new master (after my changes to remove server ID, which collided a little with some of your changes to the TPM handles).

Could you refactor the TCTI/SAPI stuff from tool/client.c into its own file, and use that in tool/nvraam.c (I know I said don't worry about it in person, but I take that back!). Also, can tool/nvram.c use util/file_io rather than defining its own file writing?

Copy link
Collaborator

@zanebeckwith zanebeckwith left a comment

Choose a reason for hiding this comment

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

I made a couple more requests, things I hadn't noticed earlier.

I've now tested this with the new xaptum-tpm release and with our current issuing architecture, and it looks good!

tool/parse_cli.c Outdated Show resolved Hide resolved
include/xtt/tpm/nvram.h Outdated Show resolved Hide resolved
@kathrynfejer
Copy link
Contributor Author

kathrynfejer commented Nov 29, 2018

One other thing, I noticed that we still have all the client and server data, but may not need anything but the gid.bin and daa_secretkey.bin anymore. I don't know if anyone depends on the other files being there, or won't want to use the tool to generate them each time. At the least, we can probably remove the server_id.bin from both client and server.

@zanebeckwith
Copy link
Collaborator

In the example data, we need everything except server id, right? (That was an oversight on my part; you're right, that server id file should be remove)

Until we mainstream the ecdaa tool, all the DAA-specific stuff will still have to be there, right?

@kathrynfejer
Copy link
Contributor Author

kathrynfejer commented Nov 29, 2018

Oops, sorry, yes we will need that. We can now read in the DAA specific data, but only if we are using a TPM.

@zanebeckwith
Copy link
Collaborator

Ah, I see! Yea, using a TPM, the DAA stuff required is minimal. But, yea, to support software-DAA, we still example DAA files.

Is this ready to go?

@kathrynfejer
Copy link
Contributor Author

Yep! Should be good to go!

@zanebeckwith
Copy link
Collaborator

Looks great! Thanks!

@zanebeckwith zanebeckwith merged commit 13220ff into xaptum:master Nov 29, 2018
@kathrynfejer kathrynfejer deleted the add-nvram-xtt branch November 30, 2018 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants