Re: [edk2-devel] [RFC v1 5/4] CryptoPkg/TlsLib: accept peer certs via both DNS names and IP addresses

["David Woodhouse" <dwmw2@infradead.org>]

[-- Attachment #1: Type: text/plain, Size: 2405 bytes --]

On Wed, 2019-10-16 at 13:41 +0200, Laszlo Ersek wrote:
> On 10/16/19 11:19, David Woodhouse wrote:
> > In the end you did actually solve it all for yourself -- based on the
> > pointer I'd given in bugzilla, and then ignoring my subsequent
> > misdirection about callbacks and my overly complex attempt at doing it
> > myself :)
> 
> You're too kind; I followed your code closely. I only updated the coding
> style, added a bunch of comments, and supplied some error checks and
> debug messages. 

Hm, I am not widely renowned for my kindness. I'm sure there are those
who would suggest to you that there really must be some other
explanation than that.

They might point out that you also threw away all of the parts from my
actual code where I'd led you in the wrong direction, going back to my
initial suggestion that had been in Bugzilla in the first place. 

They might suggest that in posting poorly-formatted straw man patches
to TlsLib, all I have *actually* managed to do is what people so often
do to me — post a patch which is sufficiently incompetent that it has
provoked you into finding the time to do it properly for yourself :)

> In addition, Jiaxin suggested we should try to set the
> smart verification params in TlsSetVerifyHost() at once.
> 
> Anyway: we still have the issue that X509_VERIFY_PARAM_set_ip_asc()
> appears to reject IPv4 address literals. Could you check that please?
> 
> (Using a hosted (Linux userspace) program like "sconnect", it must be
> easier to debug. I tried connecting gdb to QEMU, running OVMF, but it
> crashed gdb. :)

Ah, but if you were using a hosted Linux userspace program like
sconnect, then your sscanf() implementation wouldn't look like this:

$ grep -B1 -A4 sscanf CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
/* Read formatted data from a string */
int sscanf (const char *buffer, const char *format, ...)
{
  //
  // Null sscanf() function implementation to satisfy the linker, since
  // no direct functionality logic dependency in present UEFI cases.
  //
  return 0;
}

I told you to stare hard at that, didn't I :)

I'm sure that OpenSSL upstream would welcome a patch to ditch that use
of the non-recommended sscanf() function and use inet_ntoa() where it's
available instead (although that might sensibly be guarded on
OPENSSL_NO_SOCK, which you set for the EDK2 build).

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5174 bytes --]