His comments are:
This patch fixes the problem of modern Kerberos using "derived keys"
to encrypt the authenticator by disabling the authenticator check
for all derived keys enctypes.
I think I've got all the bugfixes that Jeffrey and I discussed rolled
into this. There were some problems with Jeffrey's code to convert
the authenticator's Kerberos timestring into struct tm (e.g. Z, -1900;
it helps to have an actual decryptable authenticator to play with).
So I've shamelessly pushed in my code, while stealing some bits from
Jeffrey.
Note that since some private kssl functions were exported, the
simplest way to rebuild the number table was to toss everything that
was new since OpenSSL 0.9.6b. This is safe, since those functions
have not yet been exported in an OpenSSL release. Beware, people who
trust intermediary snapshots!
His comments are:
. adds use of replay cache to protect against replay attacks
. adds functions kssl_tgt_is_available() and
kssl_keytab_is_available() which are used within s3_lib.c
and ssl_lib.c to determine at runtime whether or not
KRB5 ciphers can be supported during the current session.
Jeffrey Altman <jaltman@columbia.edu>
(Really, the time that's being parsed is a GeneralizedTime, so if
ASN1_GENERALIZEDTIME_get() ever gets implemented, it should be used
instead)
His comments are:
. Fixed all of the Windows dynamic loading functions, prototypes, etc.
. Corrected all of the unsigned/signed comparison warnings
. Replaced the references to krb5_cksumarray[] for two reasons.
First, it was an internal variable that should not have been
referenced outside the library; nor could it have been with
a shared library with restricted exports. Second, the
variable is no longer used in current Kerberos implementations.
I replaced the code with equivalent functionality using functions
that are exported from the library.
SSL according to RFC 2712. His comment is:
This is a patch to openssl-SNAP-20010702 to support Kerberized SSL
authentication. I'm expecting to have the full kssl-0.5 kit up on
sourceforge by the end of the week. The full kit includes patches
for mod-ssl, apache, and a few text clients. The sourceforge URL
is http://sourceforge.net/projects/kssl/ .
Thanks to a note from Simon Wilkinson I've replaced my KRB5 AP_REQ
message with a real KerberosWrapper struct. I think this is fully
RFC 2712 compliant now, including support for the optional
authenticator field. I also added openssl-style ASN.1 macros for
a few Kerberos structs; see crypto/krb5/ if you're interested.
applications to use EVP. Add missing calls to HMAC_cleanup() and
don't assume HMAC_CTX can be copied using memcpy().
Note: this is almost identical to the patch submitted to openssl-dev
by Verdon Walker <VWalker@novell.com> except some redundant
EVP_add_digest_()/EVP_cleanup() calls were removed and some changes
made to avoid compiler warnings.
compiler bug reported in <01032110293775.22278@weba3.iname.net>
(the '++seq[i]' condition is evaluated as 256 rather than 0
when the previous value is 255).
an SSL_CTX's session cache, it is necessary to compare the ssl_version at
the same time (a conflict is defined, courtesy of SSL_SESSION_cmp(), as a
matching id/id_length pair and a matching ssl_version). However, the
SSL_SESSION that will result from the current negotiation does not
necessarily have the same ssl version as the "SSL_METHOD" in use by the
SSL_CTX - part of the work in a handshake is to agree on an ssl version!
This is fixed by having the check function accept an SSL pointer rather
than the SSL_CTX it belongs to.
[Thanks to Lutz for illuminating the full extent of my stupidity]
and make all files the depend on it include it without prefixing it
with openssl/.
This means that all Makefiles will have $(TOP) as one of the include
directories.
the ID will be padded out to 16 bytes if the callback attempted to generate
a shorter one. The problem is that the uniqueness checking function used in
callbacks may mistakenly think a 9-byte ID is unique when in fact its
padded 16-byte version is not. This makes the checking function detect
SSLv2 cases, and ensures the padded form is checked rather than the shorter
one passed by the callback.
SSL/TLS session IDs in a server. According to RFC2246, the session ID is an
arbitrary value chosen by the server. It can be useful to have some control
over this "arbitrary value" so as to choose it in ways that can aid in
things like external session caching and balancing (eg. clustering). The
default session ID generation is to fill the ID with random data.
The callback used by default is built in to ssl_sess.c, but registering a
callback in an SSL_CTX or in a particular SSL overrides this. BTW: SSL
callbacks will override SSL_CTX callbacks, and a new SSL structure inherits
any callback set in its 'parent' SSL_CTX. The header comments describe how
this mechanism ticks, and source code comments describe (hopefully) why it
ticks the way it does.
Man pages are on the way ...
[NB: Lutz was also hacking away and helping me to figure out how best to do
this.]
sure they are available in opensslconf.h, by giving them names starting
with "OPENSSL_" to avoid conflicts with other packages and by making
sure e_os2.h will cover all platform-specific cases together with
opensslconf.h.
I've checked fairly well that nothing breaks with this (apart from
external software that will adapt if they have used something like
NO_KRB5), but I can't guarantee it completely, so a review of this
change would be a good thing.
"doall" functions to using type-safe wrappers. As and where required, this
can be replaced by redeclaring the underlying callbacks to use the
underlying "void"-based prototypes (eg. if performance suffers from an
extra level of function invocation).
objects) or OPENSSL_BUILD_SHLIBSSL (for files that end up as libssl
objects) is defined, redefine OPENSSL_EXTERN to be OPENSSL_EXPORT.
This is actually only important on Win32, and can safely be ignored in
all other cases, at least for now.
DECLARE/IMPLEMENT macros now exist to create type (and prototype) safe
wrapper functions that avoid the use of function pointer casting yet retain
type-safety for type-specific callbacks. However, most of the usage within
OpenSSL itself doesn't really require the extra function because the hash
and compare callbacks are internal functions declared only for use by the
hash table. So this change catches all those cases and reimplements the
functions using the base-level LHASH prototypes and does per-variable
casting inside those functions to convert to the appropriate item type.
The exception so far is in ssl_lib.c where the hash and compare callbacks
are not static - they're exposed in ssl.h so their prototypes should not be
changed. In this last case, the IMPLEMENT_LHASH_*** macros have been left
intact.
casts) used in the lhash code are about as horrible and evil as they can
be. For starters, the callback prototypes contain empty parameter lists.
Yuck.
This first change defines clearer prototypes - including "typedef"'d
function pointer types to use as "hash" and "compare" callbacks, as well as
the callbacks passed to the lh_doall and lh_doall_arg iteration functions.
Now at least more explicit (and clear) casting is required in all of the
dependant code - and that should be included in this commit.
The next step will be to hunt down and obliterate some of the function
pointer casting being used when it's not necessary - a particularly evil
variant exists in the implementation of lh_doall.
test was never triggered due to an off-by-one error.
In s23_clnt.c, don't use special rollback-attack detection padding
(RSA_SSLV23_PADDING) if SSL 2.0 is the only protocol enabled in the
client; similarly, in s23_srvr.c, don't do the rollback check if
SSL 2.0 is the only protocol enabled in the server.
could be done automagically, much like the numbering in libeay.num and
ssleay.num. The solution works as follows:
- New object identifiers are inserted in objects.txt, following the
syntax given in objects.README.
- objects.pl is used to process obj_mac.num and create a new
obj_mac.h.
- obj_dat.pl is used to create a new obj_dat.h, using the data in
obj_mac.h.
This is currently kind of a hack, and the perl code in objects.pl
isn't very elegant, but it works as I intended. The simplest way to
check that it worked correctly is to look in obj_dat.h and check the
array nid_objs and make sure the objects haven't moved around (this is
important!). Additions are OK, as well as consistent name changes.
This is mostly a work around for the old VC++ problem
that it treats func() as func(void).
Various prototypes had been added to 'compare' function
pointers that triggered this. This could be fixed by removing
the prototype, adding function pointer casts to every call or
changing the passed function to use the expected arguments.
I mostly did the latter.
The mkdef.pl script was modified to remove the typesafe
functions which no longer exist.
Oh and some functions called OPENSSL_freeLibrary() were
changed back to FreeLibrary(), wonder how that happened :-)
After some messing around this seems to work but needs
a few more tests. Working out the syntax for sk_set_cmp_func()
(cast it to a function that itself returns a function pointer)
was painful :-(
Needs some testing to see what other compilers think of this
syntax.
Also needs similar stuff for ASN1_SET_OF etc etc.
Also, "make update" has added some missing functions to libeay.num,
updated the TABLE for the alpha changes, and updated thousands of
dependancies that have changed from recent commits.
was a really bad idea. For example, the following:
#include <x509.h>
#include <bio.h>
#include <asn1.h>
would make sure that things like ASN1_UTCTIME_print() wasn't defined
unless you moved the inclusion of bio.h to above the inclusion of
x509.h. The reason is that x509.h includes asn1.h, and the
declaration of ASN1_UTCTIME_print() depended on the definition of
HEADER_BIO_H. That's what I call an obscure bug.
Instead, this change makes sure that whatever header files are needed
for the correct process of one header file are included automagically,
and that the definitions of, for example, BIO-related things are
dependent on the absence of the NO_{foo} macros. This is also
consistent with the way parts of OpenSSL can be excluded at will.
like Malloc, Realloc and especially Free conflict with already existing names
on some operating systems or other packages. That is reason enough to change
the names of the OpenSSL memory allocation macros to something that has a
better chance of being unique, like prepending them with OPENSSL_.
This change includes all the name changes needed throughout all C files.
yet tighter, and also put some heat on the rest of the library by
insisting (correctly) that compare callbacks used in stacks are prototyped
with "const" parameters. This has led to a depth-first explosion of
compiler warnings in the code where 1 constification has led to 3 or 4
more. Fortunately these have all been resolved to completion and the code
seems cleaner as a result - in particular many of the _cmp() functions
should have been prototyped with "const"s, and now are. There was one
little problem however;
X509_cmp() should by rights compare "const X509 *" pointers, and it is now
declared as such. However, it's internal workings can involve
recalculating hash values and extensions if they have not already been
setup. Someone with a more intricate understanding of the flow control of
X509 might be able to tighten this up, but for now - this seemed the
obvious place to stop the "depth-first" constification of the code by
using an evil cast (they have migrated all the way here from safestack.h).
Fortunately, this is the only place in the code where this was required
to complete these type-safety changes, and it's reasonably clear and
commented, and seemed the least unacceptable of the options. Trying to
take the constification further ends up exploding out considerably, and
indeed leads directly into generalised ASN functions which are not likely
to cooperate well with this.
"Jan Mikkelsen" <janm@transactionsite.com> correctly states that the
OpenSSL header files have #include's and extern "C"'s in an incorrect
order. Thusly fixed.
in SSL_new.
If SSL_OP_SINGLE_DH_USE is set, don't waste time in SSL_[CTX_]set_tmp_dh
on computing a DH key that will be ignored anyway.
ssltest -dhe1024dsa (w/ 160-bit sub-prime) had an unfair performance
advantage over -dhe1024 (safe prime): SSL_OP_SINGLE_DH_USE was
effectively always enabled because SSL_new ignored the DH key set in
the SSL_CTX. Now -dhe1024 takes the server only about twice as long
as -dhe1024dsa instead of three times as long (for 1024 bit RSA
with 1024 bit DH).
don't dynamically create them. This allows using ssltest
for approximate performance comparisons:
$ time ./ssltest -num 50 -tls1 -cert ../apps/server2.pem \
[-no_dhe|-dhe1024dsa|-dhe1024]
(server2.pem contains a 1024 bit RSA key, the default has only
512 bits.) Note that these timings contain both the server's and
the client's computations, they are not a good indicator for
server workload in different configurations.