I have ported a few methods over to the Module object as C_* functions. @Vincent, Can you have a look and let me know your opinion before I do the rest in the same way.
*Guillaume Amringerhttps://keybase.io/gamr <https://keybase.io/gamr>https://www.linkedin.com/in/guillaumeamringer <https://www.linkedin.com/in/guillaumeamringer>https://github.com/gamringer <https://github.com/gamringer>* On Sun, Aug 16, 2020 at 2:42 PM Vincent Jardin <vjar...@free.fr> wrote: > +1 yes of course then it is a very good solution: > - it provides the legacy OASIS definitions with all the C_* > - anyone that is fine with your shortcut can leverage them. > > I am ready for a code review of your C_* as soon as you'll get them. > > Best regards, > Vincent > > Le 16 août 2020 18:42:04 Guillaume Amringer <g.amrin...@gmail.com> a > écrit : > >> So what do you think of this suggestion: >> >> I keep the OO API as-is, but I add the C_* functions as methods of the >> Pkcs11\Module object. >> >> Example 1: >> $module->C_GetSlotList() would be an alias of $module->getSlotList() >> >> Example 2: >> $session = $module->C_OpenSession($slotList[0], Pkcs11\CKF_RW_SESSION); >> $module->C_Login($session, Pkcs11\CKU_USER, '123456' ); >> would be the same as >> $session = $module->openSession($slotList[0], Pkcs11\CKF_RW_SESSION); >> $session->login(Pkcs11\CKU_USER,'123456'); >> >> >> >> >> >> >> *Guillaume Amringerhttps://keybase.io/gamr >> <https://keybase.io/gamr>https://www.linkedin.com/in/guillaumeamringer >> <https://www.linkedin.com/in/guillaumeamringer>https://github.com/gamringer >> <https://github.com/gamringer>* >> >> >> On Sun, Aug 16, 2020 at 3:27 AM Vincent Jardin <vjar...@free.fr> wrote: >> >>> Hi Guillaume, >>> >>> I agree with you that PECL is the best way to move forward. Nevertheless >>> looking at the API I get more and more concern about the differences with >>> the functional OASIS/pkcs11 definitions that is a standard for years and >>> yours knowing it shall become a PECL and mainstream reference. >>> >>> Having an object oriented model is a good idea, but you should highlight >>> that it is not the standard pkcs11 API that we can refer to when one needs >>> to understand the behaviors: more comments about your visions as part of >>> the README would help. >>> >>> That's why for a PECL project I would like your PHP extension to be >>> named with a prefix or suffix that reminds that it is not the legacy pkcs11 >>> mapped to PHP. For instance the names could be: >>> php-op11 (o = object), >>> php-cp11 (c = class), >>> etc./TBD >>> >>> Best regards, >>> Vincent >>> >>> Le 16 août 2020 05:32:45 Guillaume Amringer <g.amrin...@gmail.com> a >>> écrit : >>> >>>> Hi, >>>> >>>> I worked on my pkcs11 extension some more, and I believe it is ready >>>> for at least a 0.1 release. I have addressed the memory management issues >>>> and implemented the remaining important features. >>>> >>>> According to the PECL registration form, I need to obtain determination >>>> that "it's time for me to have a PECL account". >>>> >>>> Does that sound like the next step ? Also, feel free to provide more >>>> commentary on my work if you think it is pertinent. >>>> >>>> >>>> >>>> >>>> >>>> *Guillaume Amringerhttps://keybase.io/gamr >>>> <https://keybase.io/gamr>https://www.linkedin.com/in/guillaumeamringer >>>> <https://www.linkedin.com/in/guillaumeamringer>https://github.com/gamringer >>>> <https://github.com/gamringer>* >>>> >>>> >>>> On Tue, Jul 21, 2020 at 10:38 PM Guillaume Amringer < >>>> g.amrin...@gmail.com> wrote: >>>> >>>>> Hi all, >>>>> >>>>> Thanks for the feedback. I was mostly concerned with the happy path at >>>>> first, fixing memory management was on my list, but given it's the >>>>> recurring comment, I am starting on that now. This is my first real C >>>>> project, so your criticism is welcome. >>>>> >>>>> Regarding the API, I can't simply expose the same API as the >>>>> definition, since at least, there won't be the need to pass the various >>>>> argument lengths in PHP land. Since I was going to change the API anyway, >>>>> I >>>>> thought it best to use the recommended code convention for class and >>>>> method >>>>> names. It is also in line with other languages PKCS11 implementations. One >>>>> compromise I think is appropriate would be to provide the vanilla API as >>>>> functions (as they are) taking a Pkcs11\Session object as the session >>>>> handler. and leave it up to the user to choose how to use it. @vjardin, >>>>> what do you think ? >>>>> >>>>> Regarding C_GetAttributeValue() return code, this is more of a design >>>>> choice, since I absolutely despise passing a variable reference to be >>>>> filled by the method, returning a partial result, along with an >>>>> explanation >>>>> would require some hoop jumping. Not impossible, but worth thinking it >>>>> over. What I see is returning a new AttributeSet object, with >>>>> isComplete(): >>>>> bool (true if CKR_OK, false otherwise), and getError() returning the code. >>>>> Not married to it though, I am open to suggestions. >>>>> >>>>> https://python-pkcs11.readthedocs.io/en/latest/applied.html >>>>> https://godoc.org/github.com/miekg/pkcs11 >>>>> >>>>> Thank you for taking the time to comment. >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> *Guillaume Amringerhttps://keybase.io/gamr >>>>> <https://keybase.io/gamr>https://www.linkedin.com/in/guillaumeamringer >>>>> <https://www.linkedin.com/in/guillaumeamringer>https://github.com/gamringer >>>>> <https://github.com/gamringer>* >>>>> >>>>> >>>>> On Mon, Jul 20, 2020 at 3:46 PM <vjar...@free.fr> wrote: >>>>> >>>>>> Hi Guillaume, >>>>>> >>>>>> same comments than Johannes, ecalloc()/efree()/emalloc() should be >>>>>> used. Since they are many crypto sensitive contents, only ecalloc() >>>>>> should >>>>>> be used (no emalloc()). >>>>>> >>>>>> Regarding the Oasis' semantic, I feel that it is important to keep it >>>>>> "as is" so the vanilla documentation would be the upstream Oasis PKCS11 >>>>>> standard. >>>>>> It means, for instance: >>>>>> getInfo() -> C_GetInfo() >>>>>> >>>>>> It is nice to have some sign() methods in order to avoid combining >>>>>> C_SignInit() + C_Sign(), but they are some cases when we need to stick >>>>>> with >>>>>> the Oasis' specifications, so I believe that >>>>>> C_SignInit() >>>>>> C_VerifyInit() >>>>>> C_FindObjectsInit() >>>>>> C_FindObjectsFinal() >>>>>> etc. >>>>>> should still be exposed per the Oasis' specification, along having >>>>>> some nice sign(), verify(), etc. methods too. >>>>>> >>>>>> Regarding: >>>>>> C_GetAttributeValue() >>>>>> it has many "OK" cases: >>>>>> See Oasis' comments: >>>>>> "Note that the error codes CKR_ATTRIBUTE_SENSITIVE, >>>>>> CKR_ATTRIBUTE_TYPE_INVALID, and CKR_BUFFER_TOO_SMALL do not denote true >>>>>> errors for C_GetAttributeValue. If a call to C_GetAttributeValue returns >>>>>> any of these three values, then the call MUST nonetheless have processed >>>>>> every attribute in the template supplied to C_GetAttributeValue. Each >>>>>> attribute in the template whose value can be returned by the call to >>>>>> C_GetAttributeValue will be returned by the call to C_GetAttributeValue." >>>>>> >>>>>> my 2 cents, >>>>>> Vincent >>>>>> >>>>>> ----- Mail original ----- >>>>>> De: "Johannes Schlüter" <johan...@schlueters.de> >>>>>> À: "Guillaume Amringer" <g.amrin...@gmail.com>, "pecl-dev" < >>>>>> pecl-dev@lists.php.net> >>>>>> Envoyé: Lundi 20 Juillet 2020 14:55:33 >>>>>> Objet: Re: [PECL-DEV] PKCS11 Extension >>>>>> >>>>>> Hi, >>>>>> >>>>>> On Sun, 2020-07-19 at 22:33 -0400, Guillaume Amringer wrote: >>>>>> > The extension is currently hosted here: >>>>>> > https://github.com/gamringer/php-pkcs11 >>>>>> >>>>>> I scrolled over it without much attention (thus probably missed other >>>>>> issues) a few comments to this line I noticed: >>>>>> >>>>>> pSlotList = (CK_SLOT_ID_PTR) malloc(ulSlotCount * >>>>>> sizeof(CK_SLOT_ID)); >>>>>> https://github.com/gamringer/php-pkcs11/blob/master/pkcs11.c#L300 >>>>>> >>>>>> 1. The return value of malloc() is not checked. If the system runs >>>>>> out >>>>>> of memory (OOM) or for some other reason can't return a memory >>>>>> block >>>>>> this will return NULL and then lead to undefined behavior later. >>>>>> (unlikely on today's systems, but might happen) >>>>>> 2. emalloc should be used instead of malloc. For one it counts >>>>>> towards >>>>>> PHP's memory_limit, ten it also ensures the memory is freed if >>>>>> something weird happens by the end of the request, thus reduces >>>>>> risk >>>>>> of memory leaks and it fixes point 1. by terminating the >>>>>> request in >>>>>> an OOM situation. >>>>>> 3. ulSlotCount * sizeof(CK_SLOT_ID) could eventually overflow. >>>>>> Better >>>>>> use >>>>>> safe_emalloc(ulSlotCount, sizeof(CK_SLOT_ID), 0) >>>>>> this >>>>>> calculates >>>>>> ulSlotCount * sizeof(CK_SLOT_ID) + 0 >>>>>> in a overflow-safe >>>>>> way and errors out in case of a problem. >>>>>> >>>>>> johannes >>>>>> >>>>>> -- >>>>>> PECL development discussion Mailing List (https://pecl.php.net/) >>>>>> To unsubscribe, visit: https://www.php.net/unsub.php >>>>>> >>>>> >>> >
