Thanks for the review~
Valerie
On 10/4/2019 8:30 PM, Xuelei Fan wrote:
I have no more comment.
Xuelei
On 10/2/2019 5:09 PM, Valerie Peng wrote:
During the pre-integration mach5 run, I ran into a window-specific
data alignment problem when the custom GCM param structure is
declared inside pkcs11wrapper.h in webrev.01. So, I ended up adding a
separate header file pkcs11gcm2.h containing only this structure and
include it in the platform-specific header p11_md.h so the current
logic of using sizeof() works as expected.
Mach5 run and Solaris 11.4 run pass as expected with webrev.02.
http://cr.openjdk.java.net/~valeriep/8229243/webrev.02/
If there are more comments, please let me know in a few days.
Thanks!
Valerie
On 9/24/2019 9:45 PM, Valerie Peng wrote:
Great, thanks for the review and feedback~
Valerie
On 9/24/2019 9:20 PM, Xuelei Fan wrote:
I looked at the latest webrev:
http://cr.openjdk.java.net/~valeriep/8229243/webrev.01/
Which is good to me.
Thanks,
Xuelei
On 9/19/2019 5:46 PM, Valerie Peng wrote:
I am not on the PKCS#11 committee and not sure about the plan.
As for which one is right, I am more inclined to the "spec is
right" side which is also what NSS picked.
Comparing between spec and header, shouldn't the former get more
eyeballs in terms of review?
The header file also has a deprecated structure CK_AES_GCM_PARAMS
which contains both ulIvLen and ulIvBits fields.
As ulIvBits and ulIvLen are essentially same in terms of meaning
except bytes vs bits. Having both just creates confusion and
potential inconsistency and makes not much sense to me.
Valerie
----- Original Message -----
From: xuelei....@oracle.com
To: valerie.p...@oracle.com, security-dev@openjdk.java.net
Sent: Thursday, September 19, 2019 7:47:43 AM GMT -08:00 US/Canada
Pacific
Subject: Re: [14] RFR JDK-8229243 "SunPKCS11-Solaris provider
tests failing on Solaris 11.4"
Will the inconsistency structure be continue? I was just
wondering if
OpenHSM2/Solaris/NSS will fix the bug and use one structure in the
future, then we may not need to workaround the issue in the
calling side.
I had a quick look the PKCS#11 3.0 draft, there is no update of the
structure yet.
Xuelei
On 9/18/2019 6:01 PM, Valerie Peng wrote:
Ping?
Can someone help take a look?
Thanks,
Valerie
On 8/15/2019 4:49 PM, Valerie Peng wrote:
Anyone has time to help review this fix? PKCS#11 v2.40 has
inconsistent definition for CK_GCM_PARAMS struct. The mechanism
spec
(sec 2..12.3) has:
typedef struct CK_GCM_PARAMS {
CK_BYTE_PTR pIv;
CK_ULONG ulIvLen;
CK_BYTE_PTR pAAD;
CK_ULONG ulAADLen;
CK_ULONG ulTagBits;
} CK_GCM_PARAMS;
However, the header file pkcs11t.h has an extra "ulIvBits" field:
typedef struct CK_GCM_PARAMS {
CK_BYTE_PTR pIv;
CK_ULONG ulIvLen;
* CK_ULONG ulIvBits;*
CK_BYTE_PTR pAAD;
CK_ULONG ulAADLen;
CK_ULONG ulTagBits;
} CK_GCM_PARAMS;
Some vendors such OpenHSM2 and Solaris go with the header file
while
some vendor such as NSS go with the mech spec. Current SunPKCS11
provider impl works with NSS but not with other vendors whose
CK_GCM_PARAMS struct contains ulIvBits field. Based on testing
results, OpenHSM2 and Solaris error out at init time when the
parameter length does not have their expected value. Thus, one
way to
workaround this inconsistency is to re-try with a different
structure
for AES GCM when the init failed. In addition, given the parameters
contains Iv and AAD which some vendor may use during subsequent
enc/dec operations, I changed to use the same model as
RSASSA-PSS to
defer the freeing after the enc/dec operation is finished. Verified
the changes on Solaris 11.4 against existing GCM regression tests.
Bug: https://bugs.openjdk.java.net/browse/JDK-8229243
Webrev: http://cr.openjdk.java.net/~valeriep/8229243/webrev.00/
Thanks,
Valerie
