[xmlsec] xmlsec-mscrypto code review (patch #3)
aleksey at aleksey.com
Mon Sep 22 13:29:10 PDT 2003
The certificate chain is a list of certificates cert_0, cert_1, cert_2,
Subject(cert_k) = Issuer(cert_k+1) where k = 0, ..., N - 1
The chain is valid if and only if
1) each cert in the chain is good (i.e. it has valid signature on
itself, valid time
restrictions not before/not after, it is not revoked by some CRL, ...);
2) the root of the chain (cert_0) is trusted by application
You are right and xmlsec wants to use crypto engines to create certs
chain. In typical
case xmlsec has a certificate it is interested in and it just asks
crypto engine if there is a
valid chain for this certificate (using trusted certs in crypto engine
and untrusted certs
from the xmlsec and crypto engine). Unfortunately, MSCrypto does not provide
such a function (to be precise, as I wrote in the email, this function
is not available
on Windows 9x). Instead it has a function
that (as far as I can understand) simply does a check that
- cert is good (see above what does this mean)
- Subject(issuerCert) == Issuer(cert)
This check validates that issuerCert + cert can be a part of
certificates chain but it does not
validate the cert! Your code did not construct the certs chain longer
than 2 certs and it did not
verify that chain's root is trusted. Which is a big security hole from
my point of view :)
For example, I can sign a cert with another cert I generated. Now this
function would be OK with
that but would you trust an online bank that presents such certificate?
I wouldn't because I don't
know who is behind the server I am connecting to. Is it my bank server
or some kiddie hacker who
knows how to run OpenSSL to generate certificates.
The whole question about trust in the X509 PKI model is based on which
certs you (your
application) are marked as "trusted". xmlsec library does not involved
in these descsision.
It's just something your have to do yourself as a system architect.
However, xmlsec command line
utility provides a way to say: I trust this certificate ("--trusted"
option). The exact meaning
of this is
"the cert specified with --trusted option can be root cert
cert_0 for a valid certificates chain".
If you have no trusted certs then you have no valid certificates chain.
Current xmlsec-mscrypto implementation maintains its own list of trusted
certs in the keys manager.
As I wrote before, certs marked as "trusted" in the MS Crypto certs
store are not considered as such
by xmlsec due to the MSCyrpto API restrictions. I think it's a good idea
to enable this functionality
but it's a separate big task from the code review I did. My change was
to just plug in a security hole
in the certificates processing.
>>What is the reason why you duplicate cert passed to the function? I
>>removed this dup call
>>but it might be wrong.
>Hmm, I did that in any case where the cert passed with this function
>would be deleted in the calling context of that function, assuming that
>any AdiptCert/AdoptKey type function takes over the control of life and
>dead of that key/cert. If this is not clear, could you give me pointers
>in the src where this is done wrong according to your opinion?
The point is that your code did something like this:
cert2 = DuplicateCert(cert);
/* put cert2 in the key data */
If the xmlSecMSCryptoKeyDataX509AdoptCert() caller would try to destroy
the cert it passed to
this function then there would be an error anyway. Also the name of the
function (Adopt) tells the
caller that if the call to this function succeeded then caller does not
own passed in object anymore.
My change was to remove DuplicateCert/DeleteCert calls:
/* put cert in the key data */
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the xmlsec