Hi,

The change makes sense, so 'Feature-ACK', but one remark:

On 20-04-17 16:41, Hristo Venev wrote:
> @@ -191,16 +191,23 @@ extract_x509_field_ssl(X509_NAME *x509, const char 
> *field_name, char *out,
>      X509_NAME_ENTRY *x509ne = 0;
>      ASN1_STRING *asn1 = 0;
>      unsigned char *buf = NULL;
> -    int nid = OBJ_txt2nid(field_name);
> +    ASN1_OBJECT *obj = OBJ_txt2obj(field_name, 0);
> +
> +    if (obj == NULL)
> +    {
> +        crypto_msg(M_FATAL, "Cannot get ASN1_OBJECT for %s", field_name);
> +    }

M_FATAL is currently acceptable, because field_name always comes from
the local config file, but I'd prefer to make this non-fatal, and return
FAILURE.  That ensures that if someone decides to use this function in a
way where field_name *is* attacker controlled, we do not introduce a
remote DoS.

As for the log level of this message: I think D_TLS_DEBUG_LOW (--verb 3)
or possibly D_TLS_ERRORS (--verb 1) are appropriate.

-Steffan

Attachment: signature.asc
Description: OpenPGP digital signature

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to