Hi,
Bodo, thanks for taking the time to reply to my rant. I suspect
programming with OpenSSL requires one to vent one's splein from time to
time... :-)
On Wed, 17 Nov 1999, Bodo Moeller wrote:
> > It seems to be a play-off between backwards compatibility, and having a
> > good library-API. I prefer the latter, but I don't have as much legacy
> > code to maintain as some others do, and I probably find the current
> > "inconsistencies" a little more distasteful than some others do.
>
> I don't like them either (especially the SSL_CTX_set_tmp_rsa / dh
> example because they are "obviously" similar, but behave quite
> differently), but if we change this and get rid of the existing API
> then we have to change function names so that old code does not even
> compile, or there'll be tons of _non-obviously_ broken applications,
> which would be really, really bad.
Understood ... well at least for 0.9.4-0.9.5 but at some point there must
be a point where improvements can be made at the expense of some backwards
compatibility ... v1.0? Indeed, code that uses OpenSSL would be well
advised to perform some "#if"ing on the openssl version to ensure people
don't try to compile it with versions older (or newer) than it is known to
work with. However, as I outline below - I've already spotted the
behaviour changing and breaking existing code so I'm not sure this
argument of backwards compatibility is solid.
> > It seems that
> > everything works if you make the calls the way the authors had intended
> > rather than making the calls the way the authors made available.
>
> How can you tell what is the "intended" way? For SSL_get_session, the
> code consistently assumes that no free-ing is necessary (cf. apps/s_*.c),
> althoughly surely it would have made sense to demand it.
Intended way = (i) an SSL_SESSION pointer is not supposed to outlive the
SSL pointer from which it came. (ii) an SSL poiner is only supposed to
have one reference to it and programs that wish to have multiple
references (whether in the same thread or otherwise) had best work out
their own way of doing it using one reference.
Available way = reference counts, gets, sets, adds, and frees. If it looks
like reference counting and it talks like reference counting then callers
will reasonably hope it *is* reference counting.
> > If 0.9.4 has faults, surely it is preferable to fix them in 0.9.5 and for
> > dependant code to stick with 0.9.4 until it has been brought in line with
> > 0.9.5's fixes.
>
> The trouble is that 0.9.4 has various bugs which should be fixed for
> all applications before upgrading to the new library version becomes
> too difficult.
fair enough. The only thing I'd like to add though is that the behaviour
*IS* changing inside 0.9.5-dev and I noted an example on openssl-dev a
while back about it and ended up giving up on that piece of the API and
cutting down below it myself. Namely the SSL_CTX_load_verify_locations
behaviour changed in a way that broke my code *and* (as it turns out)
broke ssltest (if you specify a -CAfile). The only reason it didn't break
s_client and s_server was that its calls were being back-doored by;
[s_server.c:643]
if ((!SSL_CTX_load_verify_locations(ctx,CAfile,CApath)) ||
(!SSL_CTX_set_default_verify_paths(ctx)))
and because set_default_verify_paths succeeds, the fact that passing a
CAfile into load_verify_locations fails is ignored. Tracking through the
code it seems that the reason for this is the inclusion of CRLs into the
picture ... Ben made a change (this is from memory now, I posted the
details a while back) quite a few months ago whereby an underlying
function started returning a "fail" unless a CRL was also present. Then
Oct 26-27 someone (Bodo?) made a change that started paying attention to
this return value and hence the failure was propogated back up. This is my
understanding from a quick surf through the relevant bits of code - those
who wrote it probably understand far better than I what was in mind.
The point being that in this case I'm not even sure *why* the code's
behaviour has been modified and yet it has, and has broken existing code
in the process. The change I wanted to see would be of a similar nature
but was in the interst of a more orthogonal API rather than expansion of
functionality.
I was living on the head of the cvs tree and so sometimes you have to just
"suck it in" when behaviour changes and modify your code appropriately if
you want to stay with the latest and greatest. Otherwise you stay with a
"stable" version until you have the time and motivation to make the
necessary changes to move on. That's just life.
> If SSL_get_session was a newly invented function, it should increase
> the reference count. Unfortunately, it already exists and has a
> different behaviour.
True. But so was/is SSL_CTX_load_verify_locations - and it wouldn't
surprise me in the least if other functions are undergoing subtle
transformations as by-products of other "explicit" changes - that's just
natural. These would indicate that you shouldn't "go with the flow of cvs"
but target your source on known "stable" releases if you don't want your
own underbelly to start changing shape. Even ssltest broke due to
modifications in cvs and that is *supposed* to be in sync ... how much
hope can one hold out for old/legacy code?
> > Now why an SSL can't have a reference count higher than one, and be
> > relevant to multiple threads each wanting a safe reference to it is beyond
> > me.
>
> There's no locking done for accesses on SSL structures. Unlike with
> standard Unix file descriptors you cannot have one thread read and
> another thread write at the same time, or have multiple threads read
> at the same time or something like that, so it's best not to give the
> SSL pointer to more than one thread in the first place.
Which, for reasons fair or foul, is not at all obvious to anyone
attempting to use the API. Hence my comment about "if you use it like
*this* you'll be ok". Even forgetting threading for a moment - there'd
still be a strong argument in an non-blocking environment to hold multiple
references to an SSL structure (lots of little "workers" holding an SSL
each and one "master" keeping a watchful eye on all of them as well as the
SSL_CTX for gathering statistics and logging) - no synchronisation
required, just diligent handling of reference counts.
Does it behave properly with multiple references in one thread or does
that have to be avoided as well? In a consistent API, I shouldn't really
need to ask that question ... :-)
Oh well ... back into working with it rather than trying to pick it apart.
Thanks for your tolerance ;-)
Cheers,
Geoff
----------------------------------------------------------------------
Geoff Thorpe Email: [EMAIL PROTECTED]
Cryptographic Software Engineer, C2Net Europe http://www.int.c2.net
----------------------------------------------------------------------
May I just take this opportunity to say that of all the people I have
EVER emailed, you are definitely one of them.
______________________________________________________________________
OpenSSL Project http://www.openssl.org
Development Mailing List [EMAIL PROTECTED]
Automated List Manager [EMAIL PROTECTED]
