List:Commits« Previous MessageNext Message »
From:Davi Arnaut Date:March 25 2011 3:56pm
Subject:Re: bzr commit into mysql-trunk branch (Georgi.Kodinov:3749) Bug#11764778
View as plain text  
On 3/25/11 12:40 PM, Georgi Kodinov wrote:

[...]

>>>   // Make sure before and after dates are valid
>>>   bool ValidateDate(const byte* date, byte format, CertDecoder::DateType dt)
>>>   {
>>>       tm certTime;
>>> -    memset(&certTime, 0, sizeof(certTime));
>>> -    int i = 0;
>>> -
>>> -    if (format == UTC_TIME) {
>>> -        if (btoi(date[0])>= 5)
>>> -            certTime.tm_year = 1900;
>>> -        else
>>> -            certTime.tm_year = 2000;
>>> -    }
>>> -    else  { // format == GENERALIZED_TIME
>>> -        certTime.tm_year += btoi(date[i++]) * 1000;
>>> -        certTime.tm_year += btoi(date[i++]) * 100;
>>> -    }
>>> -
>>> -    GetTime(certTime.tm_year, date, i);     certTime.tm_year -= 1900; //
> adjust
>>> -    GetTime(certTime.tm_mon,  date, i);     certTime.tm_mon  -= 1;    //
> adjust
>>> -    GetTime(certTime.tm_mday, date, i);
>>> -    GetTime(certTime.tm_hour, date, i);
>>> -    GetTime(certTime.tm_min,  date, i);
>>> -    GetTime(certTime.tm_sec,  date, i);
>>> +    memset (&certTime, 0, sizeof (certTime));
>>>
>>> -    assert(date[i] == 'Z');     // only Zulu supported for this profile
>>> +    ASN1_TIME_extract(date, format,
>>> +&certTime.tm_year,&certTime.tm_mon,&certTime.tm_mday,
>>> +&certTime.tm_hour,&certTime.tm_min,&certTime.tm_sec);
>>
>> I suggest to just pass certTime and use struct tm all around, including in
> yaSSL_ASN1_TIME_to_string.
>
> This would force extra/yassl/include/openssl/ssl.h to include time.h, which I don't
> think is such a good idea.

No, I think you misunderstood my suggestion.

The point is to use struct tm internally. Something like:

char *yaSSL_ASN1_TIME_to_string(ASN1_TIME *time, char *buf, size_t len)
{
	struct tm certTime;
	TaoCrypt::ASN1_TIME_extract(time->data, time->type, &certTime);
	snprintf(buf, len, "%s %2d %02d:%02d:%02d %d GMT",
		... month_names[certTime.tm_mon], certTime.mday ...)
}

In asn.hpp you don't even have to include time.h, just use a forward 
declaration.

>>> === modified file 'sql/mysqld.cc'
>>> --- a/sql/mysqld.cc	2011-03-10 10:08:09 +0000
>>> +++ b/sql/mysqld.cc	2011-03-16 15:33:32 +0000
>>> @@ -6700,6 +6700,101 @@ static int show_ssl_get_cipher_list(THD
>>>     return 0;
>>>   }
>>>
>>> +
>>> +#ifdef HAVE_YASSL
>>
>> Add new lines between macros and code.
>
> Fixed.
>
>>> +static char *
>>> +my_asn1_time_to_string(ASN1_TIME *time, char *buf, size_t len)
>>> +{
>>> +  return yaSSL_ASN1_TIME_to_string(time, buf, len);
>>> +}
>>> +#else /* openssl */
>>
>> #elif defined HAVE_OPENSSL
>
> why ?
>
>> Although it might not even make much practical sense, the place where this
> function is used is wrapped in a HAVE_OPENSSL ifdef.
>
> Right.

So, could you add it?

[...]

>>> +
>>> +static int
>>> +show_ssl_get_server_not_before(THD *thd, SHOW_VAR *var, char *buff)
>>> +{
>>> +  var->type= SHOW_CHAR;
>>> +  if(thd->vio_ok()&&   thd->net.vio->ssl_arg)
>>> +  {
>>> +    SSL *ssl= (SSL*) thd->net.vio->ssl_arg;
>>> +    X509 *cert= SSL_get_certificate(ssl);
>>> +    ASN1_TIME *not_before= X509_get_notBefore(cert);
>>> +
>>> +    var->value= my_asn1_time_to_string(not_before, buff,
>>> +                                       SHOW_VAR_FUNC_BUFF_SIZE - 1);
>>
>> Why the -1 here?
>
> Right, it's already factored in. Removed.

BTW, I think it applies to the not_after variant too.

Regards,

Davi

Thread
bzr commit into mysql-trunk branch (Georgi.Kodinov:3749) Bug#11764778Georgi Kodinov25 Mar
Re: bzr commit into mysql-trunk branch (Georgi.Kodinov:3749) Bug#11764778Davi Arnaut25 Mar