Closed
Bug 1308013
Opened 8 years ago
Closed 7 years ago
JSS - HSM token name was mistaken for manufacturer identifier
Categories
(JSS Graveyard :: Library, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: elio.maldonado.batiz, Assigned: cfu)
References
Details
Attachments
(2 files, 2 obsolete files)
9.96 KB,
patch
|
Details | Diff | Splinter Review | |
2.91 KB,
patch
|
cfu
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Reporter | ||
Comment 1•8 years ago
|
||
Downstream patch by Christina Fu, not ready yet for review
Reporter | ||
Comment 2•7 years ago
|
||
Updated for latest sources, 30/30 passed, regenerated after testing
Attachment #8798955 -
Attachment is obsolete: true
Reporter | ||
Updated•7 years ago
|
Assignee: glenbeasley → cfu
Reporter | ||
Comment 3•7 years ago
|
||
Comment on attachment 8835184 [details] [diff] [review] recognize the right HSM manufacturer id Review of attachment 8835184 [details] [diff] [review]: ----------------------------------------------------------------- ::: org/mozilla/jss/pkcs11/PK11KeyWrapper.c @@ +314,5 @@ > /* exception was thrown */ > goto finish; > } > > + if ( (PK11_GetTokenInfo(slot, &tokenInfo) == PR_SUCCESS) && comparison against PR_SUCCESS is wrong and the compiler warns us PK11KeyWrapper.c:318:47: warning: comparison between ‘SECStatus {aka enum _SECStatus}’ and ‘enum <anonymous>’ [-Wenum-compare] I'll change PR_SUCCESS to SECSuccess in the next version.
Reporter | ||
Comment 4•7 years ago
|
||
Attachment #8835184 -
Attachment is obsolete: true
Attachment #8841119 -
Flags: review?(cfu)
Assignee | ||
Comment 5•7 years ago
|
||
Comment on attachment 8841119 [details] [diff] [review] jss-HSM-marecognize the right HSM manufacturer id Review of attachment 8841119 [details] [diff] [review]: ----------------------------------------------------------------- Other than the correction for success value comparison that you pointed out, I expect this patch to closely resemble my original Fedora JSS downstream patch jss-HSM-manufacturerID.patch. However, the attached patch seems to contain some parts from the jss-wrapInToken.patch, which I already reviewed. Could you please explain? Thanks!
Reporter | ||
Comment 6•7 years ago
|
||
If you click on "Show obsolete" you'll see three versions of this patch. The first one, attachment 8798955 [details] [diff] [review], is much shorter and deesn't have those parts in common with the jss-wrapInToken patch, the second one, attachment 8835184 [details] [diff] [review], grows and that's where the problem starts and the third one keeps the mistake. I bet on the second version I didn't have the wrap In Token patch applied in my local copy of the sources because otherwise the patch tool would have complained of an attempt to reverse previously applied changes. A new version comming next and thank you for your careful review.
Reporter | ||
Comment 7•7 years ago
|
||
Fixes flaws Christina pointed out in her review plus another one I mentioned.
Attachment #8841244 -
Flags: review?(cfu)
Assignee | ||
Comment 8•7 years ago
|
||
Comment on attachment 8841244 [details] [diff] [review] HSM recognize the right HSM manufacturer id Review of attachment 8841244 [details] [diff] [review]: ----------------------------------------------------------------- Now it looks good. Thanks!
Attachment #8841244 -
Flags: review?(cfu) → review+
Reporter | ||
Comment 9•7 years ago
|
||
Pushed: https://hg.mozilla.org/projects/jss/rev/4a6dd12a2734cf18e341e188f73fe2d4223cf22c
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 10•7 years ago
|
||
Historical Note: # RHBZ 797351 - JSS - HSM token name was mistaken for manufacturer # https://bugzilla.redhat.com/show_bug.cgi?id=797351 # author: cfu@redhat.com # Upstream: https://bugzilla.mozilla.org/show_bug.cgi?id=1308013 Patch19: jss-HSM-manufacturerID.patch
Reporter | ||
Updated•7 years ago
|
Attachment #8841119 -
Flags: review?(cfu)
You need to log in
before you can comment on or make changes to this bug.
Description
•