Daniel Stenberg wrote:

Hi Daniel,
Thanks for your reply.

>> 1) QsoSSL is obsolete. I propose to remove it completely. Any
objection ?
>Not at all. Rip it out!

Done.

>> 2) In Curl_ssl_random(), I propose to return -1 if curlssl_random is
not defined.
> I didn't provide a fallback for curlssl_random not being defined
simply to make sure we discover the affected backends clearly and make
each backend make an active choice on how to provide it.

The problem is: iSeries provides an API to get random bytes, but this is
an OS-specific API and not from the TLS backend (I.e.: GSKit). If I use
it, the GSKit backend could be hardly adapted to another platform (btw:
I've heard GSKit is available for M$W...).
For now, I implemented GSKit curlssl_random as -1.

>> 3) I can understand the backend-specific md5sum function...
> Oh right. That would indeed be a sensible default alternative if the
TLS/crypto library doesn't have one provided.

Internal MD5 implementation is used now if curlssl_md5sum is not
defined.

>> In addition, ifdefs on have_curlssl_md5sum can be replaced by ifdefs
on curlssl_md5sum.
> I agree, as they're tightly associated already.

Done.

>> 4) I would like to split the backend-specific
pkp_pin_peer_pubkey()...
> Sounds wise. I think we should view the current pinning code as a
first-shot and a good entry, but I'm sure we can polish it up going
forward to get even better.

Split done: all future features about the pinned key can now be done at
a single place.

>> 5) ...If the PPK file does not exist, the curl_easy_setopt() string
is checked for being a PEM public key . In addition, the file data is
checked for PEM format.
> I can see how it can try to detect the file format, as I believe DER
and PEM are quite different, but is it really wise to offer direct data
using the same option? I'm just scared that there's a risk that if you
provide wrong file name or just remove the file name, libcurl will then
treat that as a key and that could lead to some untended consequences.

I don't fear so: A PEM key should contain "<bol>-----BEGIN PRIVATE
KEY-----<eol>", etc. Someone naming a file as such (note the eol!) is
seeking for troubles...
Anyway, separate option(s) can be desirable for semantic purpose. But
one might wonder what happens if both are specified ;-)
Adding two options (instead of one) can even let us specify a length an
pass an in-core DER key.

In all cases, this is a new feature and I won't start such a development
for the current round.
The discussion is open...

>> Of course, all these changes can be delayed after the feature
freeze...
> I don't think we have to wait. These all sounds like fixes to me, and
when it comes to the pinning code I think it is a better time to adjust
things now if we should, before we ship it in a public release the first
time.

These points' current statuses are listed above. In addition, GSKit now
supports pinned public key too.

Cheers,
Patrick

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2014-10-14