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 >>>> >>> >
