+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 Amringer
https://keybase.io/gamr
https://www.linkedin.com/in/guillaumeamringer
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 Amringer
https://keybase.io/gamr
https://www.linkedin.com/in/guillaumeamringer
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 Amringer
https://keybase.io/gamr
https://www.linkedin.com/in/guillaumeamringer
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
