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

Reply via email to