On Sat, May 17, 2014 at 11:08:11AM +0100, Jeremy Harris wrote:
> Created an attachment (id=729)
> --> (http://bugs.exim.org/attachment.cgi?id=729)
> patch version 2
I see you implemented multiple hostname support, thanks! One
comment on the OpenSSL verify_callback. You're accessing the
current_cert structure element of X509_STORE_CTX directly.
OpenSSL provides an accessor method:
X509 *X509_STORE_CTX_get_current_cert(X509_STORE_CTX *ctx);
you should probably use that, to avoid potential binary compatility
issues if the structure changes.
If (likely when) my patch for X509_check_host() is adopted you'll want
to specify a non-zero value for the final "flags" argument:
X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS
I'll drop you a note when this happens. With this flag wildcards
can only take the form "*.example.com", not "mx*.example.com" or
"*mx.example.com". The latter are not applicable to SMTP per
RFC 6125, the DANE draft or the upcoming MTA-to-MTA namechecks
draft from the UTA working group.
> Made the option into a list. Renamed option "tls_verify_cert_hostnames".
> Ignored SANs with embedded NULs.
Thanks. I believe OpenSSL always NUL terminates the ASN1 string data
that represents a DNS altname. So you don't need to check for lack of
NUL termination.
+ if (ele[len]) /* not nul-terminated */
+ ele = string_copyn(ele, len);
+
+ if (strnlen(CS ele, len) == len) /* ignore any with embedded nul */
+ list = string_append_listele(list, sep,
The copy code looks like a potential memory leak to me, but since the
check is not needed, you can just delete that code.
The payload of a DNS alternative name is always parsed as IA5String,
which is always nul-terminated when decoded. Postfix is paranoid, so
I check the ASN.1 string type:
#define TRIM0(s, l) do { while ((l) > 0 && (s)[(l)-1] == 0) --(l); } while (0)
if (ASN1_STRING_type(gn->d.ia5) != V_ASN1_IA5STRING) {
msg_warn("%s: %s: invalid ASN1 value type in subjectAltName",
myname, TLScontext->namaddr);
return (0);
}
/*
* Safe to treat as an ASCII string possibly holding a DNS name
*/
dnsname = (char *) ASN1_STRING_data(gn->d.ia5);
len = ASN1_STRING_length(gn->d.ia5);
TRIM0(dnsname, len);
/*
* Per Dr. Steven Henson of the OpenSSL development team, ASN1_IA5STRING
* values can have internal ASCII NUL values in this context because
* their length is taken from the decoded ASN1 buffer, a trailing NUL is
* always appended to make sure that the string is terminated, but the
* ASN.1 length may differ from strlen().
*/
if (len != strlen(dnsname)) {
msg_warn("%s: %s: internal NUL in subjectAltName",
myname, TLScontext->namaddr);
return 0;
}
The TRIM0 macro deals with names that are NUL-padded (observed in
some oddly constructed certs, where there are are trailing NUL
octets in the ASN.1 encoded name).
--
Viktor.
