Added import key command to tpm2_ptool and updated test script #64
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ichetty
commented
Oct 31, 2018
ichetty
force-pushed
the
import_rsa_take_2
branch
from
bfe070c
to
f79a093
Compare
williamcroberts
requested changes
Nov 2, 2018
|
You still have make check failures. |
|
-----Original Message-----
From: Jay Chetty ***@***.***
Sent: Monday, November 5, 2018 11:45 AM
To: tpm2-software/tpm2-pkcs11 ***@***.***>
Cc: Roberts, William C ***@***.***>; Comment
***@***.***>
Subject: Re: [tpm2-software/tpm2-pkcs11] Added import key command to
tpm2_ptool and updated test script (#64)
@ichetty commented on this pull request.
________________________________
In tools/tpm2_ptool.py <https://github.com/tpm2-software/tpm2-
pkcs11/pull/64#discussion_r230886478> :
> '''
- Adds a key to a token within a tpm2-pkcs11 store.
+ creates a key to a token within a tpm2-pkcs11 store.
+ '''
+ def generate_options(self, group_parser):
Now that I'm moving few options into the super class this will be needed here.
Agree?
Yep common options here, then you call super in the subclass if you override it and
need to add to them. Then just append to the parser object.
…
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub <https://github.com/tpm2-
software/tpm2-pkcs11/pull/64#discussion_r230886478> , or mute the thread
<https://github.com/notifications/unsubscribe-auth/AQ7bB9a7iUCx3eZCdutaw-
ka5BpGekY_ks5usJUsgaJpZM4YARjS> .
<https://github.com/notifications/beacon/AQ7bBwbHV9V9qmgHHCoa1PBaf5lpx
oyaks5usJUsgaJpZM4YARjS.gif>
|
|
-----Original Message-----
From: Jay Chetty ***@***.***
Sent: Monday, November 5, 2018 12:02 PM
To: tpm2-software/tpm2-pkcs11 ***@***.***>
Cc: Roberts, William C ***@***.***>; Comment
***@***.***>
Subject: Re: [tpm2-software/tpm2-pkcs11] Added import key command to
tpm2_ptool and updated test script (#64)
@ichetty commented on this pull request.
________________________________
In tools/tpm2_ptool.py <https://github.com/tpm2-software/tpm2-
pkcs11/pull/64#discussion_r230892435> :
> group_parser.add_argument(
'--label',
help='The tokens label to add a key too.\n',
required=True)
+ group_parser.add_argument(
+ '--key-label',
+ help='The label of the key imported.\n')
The super class method is doing that for both addkey and import command. if
key_label is not provided it will use the id as key_label, is that what you are
saying?
Yes keylabel is either user specified or the row id in the db. I never thought about this,
But in the db.c code if CKA_LABEL is not found one could just get the id and set it there,
But I found it useful to inform the user of how they can find their key via the API.
…
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub <https://github.com/tpm2-
software/tpm2-pkcs11/pull/64#discussion_r230892435> , or mute the thread
<https://github.com/notifications/unsubscribe-
auth/AQ7bBxheFlpWvW3u7FTnm6zbr1Mk5CHYks5usJlVgaJpZM4YARjS> .
<https://github.com/notifications/beacon/AQ7bB-
UQr8sKwmY_Hd8BnNH0BV32Pujaks5usJlVgaJpZM4YARjS.gif>
|
|
-----Original Message-----
From: Jay Chetty ***@***.***
Sent: Monday, November 5, 2018 11:07 AM
To: tpm2-software/tpm2-pkcs11 ***@***.***>
Cc: Roberts, William C ***@***.***>; Comment
***@***.***>
Subject: Re: [tpm2-software/tpm2-pkcs11] Added import key command to
tpm2_ptool and updated test script (#64)
@ichetty commented on this pull request.
________________________________
In tools/tpm2_ptool.py <https://github.com/tpm2-software/tpm2-
pkcs11/pull/64#discussion_r230873542> :
> - # update. A possible race exists if someone is looking for the key by
label between
- # these operations.
- # See:
- # - https://stackoverflow.com/questions/107005/predict-next-auto-
inserted-row-id-sqlite
- if keylabel is None:
- keylabel = str(id)
- attrs.append({CKA_LABEL : keylabel})
- db.updatetertiaryattrs(id, attrs)
-
- db.commit()
-
- print('Added key as label: "{keylabel}"'.format(keylabel=keylabel))
+ def __call__(self, args):
+ super(self.__class__, self).__call__(args)
+ keylabel= args['key_label']
+ print('Added key as label: "{keylabel}"'.format(keylabel=keylabel))
Sorry, I dnd't get what changes you are looking for? Do you want me to make the
keylabel default to rowid?
Yes, keylabel is optional and defaults to rowid. Essentially you made label required and didn't
Even mark it as so in the Argument parser. Did you test what happens if you forget the keylabel
Option?
…
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub <https://github.com/tpm2-
software/tpm2-pkcs11/pull/64#discussion_r230873542> , or mute the thread
<https://github.com/notifications/unsubscribe-
auth/AQ7bB4XmIIYVSLBh2mRUhaZE4lVDQcegks5usIw-gaJpZM4YARjS> .
<https://github.com/notifications/beacon/AQ7bB_rWeh9DQwctiZjbW5DrJB19T4
3fks5usIw-gaJpZM4YARjS.gif>
|
I see that the addkey command is defaulting the keylabel to str(id) if the keylable is not specified and the same will be done for import. Once the command is complete I'll check for the ketlabel option and if it is not specified then will use the id to print the message. |
|
-----Original Message-----
From: Jay Chetty ***@***.***
Sent: Monday, November 5, 2018 3:28 PM
To: tpm2-software/tpm2-pkcs11 ***@***.***>
Cc: Roberts, William C ***@***.***>; Comment
***@***.***>
Subject: Re: [tpm2-software/tpm2-pkcs11] Added import key command to
tpm2_ptool and updated test script (#64)
@ichetty commented on this pull request.
________________________________
In tools/tpm2_ptool.py <https://github.com/tpm2-software/tpm2-
pkcs11/pull/64#discussion_r230952482> :
> @@ -135,7 +134,9 @@ def createprimary(self, ownerauth, objauth):
ctx = os.path.join(self._tmp, "context.out")
cmd = ['tpm2_createprimary',
'-p', 'hex:%s' % objauth.decode(),
- '-o', ctx]
+ '-o', ctx,
+ '-g', 'sha256',
+ '-G', 'rsa']
No, I didn't get the chance to find out. I suspect that it is not default just because
of the fact that I had to add these to get it to work.
Fair enough, leave them in. :-)
…
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub <https://github.com/tpm2-
software/tpm2-pkcs11/pull/64#discussion_r230952482> , or mute the thread
<https://github.com/notifications/unsubscribe-
auth/AQ7bB5yrIjSDMsfA4ptlpWYWsx5XqnMNks5usMl3gaJpZM4YARjS> .
<https://github.com/notifications/beacon/AQ7bB1jqriDpvpN399O9LXBgzkdbPkui
ks5usMl3gaJpZM4YARjS.gif>
|
ichetty
force-pushed
the
import_rsa_take_2
branch
3 times, most recently
from
a42dd49
to
9f4eb96
Compare
Signed-off-by: Jay Chetty <jay.chetty@intel.com>
ichetty
force-pushed
the
import_rsa_take_2
branch
from
9f4eb96
to
935297c
Compare
|
All checks passed all issues resolved with no conflicts so closing this PR. |
And I quote: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refactored the import and addkey commandlet code and fixed other issues reported in the earlier PR