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)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: elio.maldonado.batiz, Assigned: cfu)

References

Details

Attachments

(2 files, 2 obsolete files)

      No description provided.
Downstream patch by Christina Fu, not ready yet for review
Updated for latest sources, 30/30 passed, regenerated after testing
Attachment #8798955 - Attachment is obsolete: true
Assignee: glenbeasley → cfu
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.
Attachment #8835184 - Attachment is obsolete: true
Attachment #8841119 - Flags: review?(cfu)
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!
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.
Fixes flaws Christina pointed out in her review plus another one I mentioned.
Attachment #8841244 - Flags: review?(cfu)
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+
Pushed: https://hg.mozilla.org/projects/jss/rev/4a6dd12a2734cf18e341e188f73fe2d4223cf22c
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
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
Attachment #8841119 - Flags: review?(cfu)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: