[xmlsec] Big patch to xmlsec in recent OpenOffice.org sources

Aleksey Sanin aleksey at aleksey.com
Sat Mar 5 19:35:36 PST 2005


Andrew, Chandler!

I started reviewing xmlsec-nss patch and it looks great!
But it is also really huge and it would take me time
to process it :) Please find bellow my comments after
first pass.

Aleksey

xmlsec-nss comments
--------------------------------------------------------
1) src/nss/digests.c src/nss/hmac.c src/nss/pkikeys.c
src/nss/signatures.c src/nss/x509.c src/nss/x509vfy.c

More detailed error message inluding nspr error code.
Looks good, checked in.

2) src/nss/akmngr.c
I would really love to merge xmlSecNssAppliedKeysMngr and
xmlSecNssDefaultKeysMngr if there is no objections.

3) src/nss/crypto.c, xmlSecCryptoGetFunctions_nss()
I  wonder why the patch sets all the App xmlsec-nss functions
to NULL. This means that xmlsec command line utility will not
run with xmlsec-nss at all.

4) src/nss/ciphers.c
Quite a lot of things are moved around and this makes patch
hard to read. Also almost all "#ifndef XMLSEC_NO_<ALG>" were
removed thus it is hard to strip down xmlsec if needed.

5) src/nss/keytrans.c
Does this file eliminates need for kt_rsa.c?

6) src/nss/keywrappers.c
Does this file eliminates need for kw_aes.c and kw_des.c?

7) src/nss/pkikeys.c, xmlSecNSSPKIKeyDataCtxDup()
I don't think there is a need to set ctxDst values to NULL here
because it is already done in xmlSecNSSPKIKeyDataCtxFree()

8) src/nss/pkikeys.c, xmlSecNssPKIKeyDataAdoptKey() and
xmlSecNssPKIAdoptKey()
Good idea to check input public/private key types! Checked in.

9) src/nss/symkeys.c, conversion to NSS SymKey
I like the idea! I just need to meditate on this code a little bit
and run tests.

10) src/nss/tokens.c
If I understand the patch correctly, it provides a way for application
to select crypto slot on which xmlsec would perform the crypto
operation for each signature/encryption separately. NSS already has
built in controls for defining "best" slot for a given crypto algorithm
but this is done globaly for all application. I do see the difference.
However, it is hard for me to believe that a user would really go and 
specify a crypto slot (device) before each operation. It is much more
likely that this would be done in ten levels of preferences menu and
it will be done globaly.

Is it a requirement for you to support have such level of customization?
I feel that this is more of crypto layer responsibility and I would love
not to go into such details on xmlsec-crypto layer.

11) src/nss/x509.c
Why the certificate xxxNodeWrite() functions were removed? Is it really
necessary? How one would write certs into the output xml file?

12) src/nss/x509vfy.c
StringRead/NameRead functions are gone and I am not sure I see the
replacement. Am I missing something?





More information about the xmlsec mailing list