Moparisthebest wrote:

> Attached is my patch for adding PEM file format support for public key
pinning.
> Let me know if anything needs changed.

Hi,
Thanks for having started this job: it is just happiness for me since I
do not have time to work on it now.

I did not try (yet) your code, but I've looked at it and noted the
following potential problems:

- Since data has been read in binary, \r should be ignored too.
- The (supposed) PEM data is not null-terminated, but you use strlen()
to measure it.
- Non-PEM data may appear outside the PEM armor: it is currently taken
as base64 data.
- PEM data may be preceded by mime-like headers and an empty line:
currently taken as base64 data.
- PEM files may contain more than one element: the first "^-----BEGIN
PUBLIC KEY-----$" line should mark our target element. Processing this
will probably require to scan the (huge) file itself, chunk by chunk.
- The size factor 2 is arbitrary (I understand it's there to limit the
allocated memory amount :-) and will probably be ok for most of the
currently considered subset, but will surely fail for "extended" (see
above) PEM files.
- Malformed files are only checked at the base64 level.

Except for the 2 first of those, I understand your "minimal" PEM parsing
(for sake of simplicity). However it limits the allowed file format.
Thus the subset should be documented if the full (unofficial) PEM spec
is not supported (no real standard, but a "de-facto" one).

Thanks for your work.

Cheers,
Patrick

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