Hi there,
I'm gonna spill my splein here because, just like Richard and Paul have
done now and in the past, I have suffered at the hands of a well-meaning
mission to constify parts of OpenSSL.
On Wed, 8 Nov 2000, Ben Laurie wrote:
> Richard Levitte - VMS Whacker wrote:
> >
> > From: "Paul D. Smith" <[EMAIL PROTECTED]>
> >
> > psmith> This is similar to standard C functions which take a const
> > psmith> char*, for example, and return a char* that points into the
> > psmith> string.
> >
> > Like strstr()...
>
> Just because the C libraries are broken doesn't mean we should break
> ours. In Apache we fix these rather than live with them.
Apples and oranges, Apache is an application, OpenSSL is a library and API
(that just happens to have applications in it). I know Apache has an "API"
for writing modules, one that I'm rather painfully familiar with in fact,
and I'd hardly call it an API designer's case study in encapsulation, let
alone a worthwhile example of "const"ification. For starters, generally
there is no illusion of binary compatibility maintained, and direct access
to Apache structures is actively encouraged anyway.
I've butted heads with this constification issue in the past too, and
unfortunately sooner or later you follow the code-dependency trails into
stuff like the ASN garbage where perfectly acceptable const logic fights
tooth and nail against immovable macro & casting sludge. Until such time
as that sludge can be legitimately moved, I have to say that I think
de-consting is not only acceptable but necessary - and whether or not we
like it, it's already present in core componentry of OpenSSL.
Normally, I'd agree that const means const, and that's the end of it (the
RO memory issue being but one very good reason). However, in this
particular situation that is simply not a pragmatic attitude to take.
(a) OpenSSL already violates this, so systems that use real RO memory
would *already* be a problem.
(b) This will continue to be the case until the real cause of the
problem(s) is(are) fixed.
(c) The alternative is to declare virtually nothing as const, including
return values.
(d) The alternative (c) means that applications themselves that attempt
to pass around various structures internally as "const" for
discipline reasons either have to abandon the idea (and suffer the
risks), or perform lots of risky deconstification in their own code
every time they try to touch the OpenSSL API.
So all the hand-wavey arguments about "const" being sacred and not to be
violated (which under all other circumstances I'd completely agree with)
aren't presently really worth a scrap of <deleted> right now. X509_cmp()
is a case in point ... there's no guarantee that the ASN sludge won't
actually modify the passed certs when doing the compare, yet to not
declare X509_cmp() with const parameters filters *all* the way back out to
the point where nothing at all (virtually) can be const. This in turn
requires the API to virtually never use const and that starts to make
writing applications more difficult because application programmers can't
use ordinary programming principles (and protect themselves from
themselves by using const wherever possible) because our API requires
non-const all the time.
Anyway, back to the case at hand: the BIGNUM code, IMHO, is in nearly as
messy a state as the ASN code, and is equally responsible for const (and
other normally clearcut design principle) weirdness. The "logic" (for want
of a more accurate term) is extremely warped in many places, often opaque,
and it is about as decentralised, uncoordinated, and utterly
uncontrollable as any block of code I have ever seen that is supposed to
(for the most part) simply manipulate one basic type. Try to imagine a
BN_METHOD (as I have many times) and you'll see what I mean.
Unfortunately, it is also a fundamental factor in OpenSSL's "performance",
it has to deal with the fact that there are assembly implementations of
various underlying primitives for each (optimised) platform supported, and
is currently coded very much like a house of cards. All Richard wanted to
achieve was some "constification" of the API so that even if internally
the code is utter spaghetti, at least people using the API aren't being
forced into dumb programming practices. However, trying to follow the
constification through (rather than casting it away) has managed to knock
a big chunk out of the performance numbers and also (apparently) created
an error that fails a test somewhere. That alone is testimony to where our
problems lie - and it is *not* in the fact we try to constify the API even
if it means some necessary (for now) deconstifying in the code.
>
> > psmith> IMHO this is a legitimate reason to cast away const, and that
> > psmith> the "const" notation on the arguments to lhash is useful for
> > psmith> self-documentation purposes, at the least.
> >
> > Hmm, perhaps you're right. I'm just a bit worried about how hard it
> > might become if someone is stupid enough to actually use the returned
> > (non-const) pointer to change something that is on RO memory...
>
> Exactly.
Well, if I'm not mistaken - none of the current changes should involve
returning non-consts pointers to const memory, and that is not even the
issue at hand: in fact it's quite the opposite. The issue *is* (again, if
I'm not mistaken) that we want the exposed API to be locked up a bit
tighter with in-values being declared as "const" where in any sane world
they should be (and thus, application programmers can benefit from the
discipline that brings back into their own compilation checking) even
though OpenSSL itself is in certain weird ways sometimes modifying what
that "const" parameter was referring to. Bear in mind also that in *all*
cases, deconstification is to allow us non-const pointers to structures
whose internals are generally only managed inside OpenSSL anyway. Eg. the
caller may pass a const X509* pointer, but the caller is generally not
supposed to be using the structure's internals directly anyway - the use
of "const" is 80% for the application programmer's own benefit, whether we
violate it or not.
An illustration of the benefit of putting up with the internal
deconstification if it allows more logical constification in the API: if
an application has an X509* value inside an application "foo_context"
structure and they have a function that is supposed to be passed a "const
foo_context*" pointer, they should be allowed to pass that structure's
X509 value into X509_cmp() but not into X509_free() ... that is just
logical and sound programming practice. If, because of ASN weirdness,
X509_cmp() can actually modify certs when comparing them (eg. computing
cachable values if they've not already been computed), that's no reason to
deconstify X509_cmp() and create an illogical API for the application
programmer. It's a problem to be fixed in OpenSSL, not propagated onto the
application programmer. The only justification for doing that could have
been something equivalent to the RO memory issue, and we're already
breaking that in fundamental ways so I think the point is moot.
Cheers,
Geoff
______________________________________________________________________
OpenSSL Project http://www.openssl.org
Development Mailing List [EMAIL PROTECTED]
Automated List Manager [EMAIL PROTECTED]
