httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David Reid <>
Subject Re: [PATCH] ssl_ext_lookup
Date Mon, 12 Sep 2005 13:34:39 GMT
Joe Orton wrote:
> On Sat, Sep 10, 2005 at 02:47:17AM +0100, David Reid wrote:
>>Following patch makes some changes to ssl_ext_lookup and changes it's
>>API, hence the post for review.
>>Add some more warnings when things don't go as advertised.
> I don't think it's appropriate to log warnings (at least at 
> APLOG_WARNING level) from a function like this - only the caller knows 
> whether or not failures require user-visibile warnings or not.

OK. I think it makes sense to log them at some level though as otherwise
there is no indication what's wrong. When testing these caught me out
for a while, hence my additions. Maybe INFO would be a better level to
log at?

>>We now allow the "known" names to be used as extensions to lookup
>>expanding the flexability of the function.
>>Add an index to allow repeated calls to get different values to handle
>>the case when the same extension is present multiple times (there is no
>>restriction how often they can appear I'm aware of).
> Use of the index seems fine though this is starting to overlap in 
> functionality with the ssl_extlist_by_oid function?

Hmm, maybe. Maybe ssl_extlist could just call ssl_ext_lookup?

>>X509V3_EXT_print seems to have trouble printing some simple strings and
>>despite having a default fallback it's still not able to decode them, so
>>we allow a plain return of the data. This could also (concievably) be a
>>small binary section, so we return the length to allow the caller to
>>know how much data is provided. This can probably be improved on.
> This is similar to what Dirk proposed recently, right?  I'll reiterate 
> my concern with this: if OpenSSL cannot return a human-readable char * 
> representation of this extension value then this will instead produce 
> some unusable binary blob?  This interface is supposed to return char * 
> strings not binary blobs.

Right. I agree with you, but the function we're using fails to cope with
even a simple string, so we need another solution. I'll try and come up
with an alternative...


View raw message