Aleksey Sanin wrote:

Hi, Andrew!

Thanks for your comments! Please see my comments interlaced with
yours bellow. I also have one huge request. Can you provide copyright
information for the new files, please?

I has been asking for the copyright license from legal apart for about three years. I'll try to get it as soon as possible. :-P


And also I would be glad to
extend the xmlsec authors list if you provide me with a list of
names :)

Aleksey



xmlsec-nss comments
--------------------------------------------------------
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.

:-( Pardon me for my lazy and time limitation when I implemented the patch. However, it's in our plan to implement the apps. I'd be grateful if you would help us on the implementation. :-) If you have no time, please let me know what urgent you need these implementations.

Well, I would need these functions in order to test the code :) Anyway, I am not sure I understand why the existing versions would not work. I'll look into this.


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.



In fact, it is a complete new implement. After Tej contribute his engine, I don't think his impl meet the requirements, so I didn't made much changes over this file. Yes, I forgot to add the compiler conditions before klass definition and *GetKlass* functions. Yes, it is a little hard to read, I'm ready to answer any questions. :-)


Ah... OK, if it is a new code then it makes sense :) Let me look at it
from the new angle :)

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()

Yes, you're right. The values have been set to null at initializor and finalizor.

Good, this change is dropped.

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.

:-)

Still in progress... :)

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.


I think we have talked about the topic at 2003, and you also give me the idea that binding the slot with mechanisms. OpenOffice really use this function in order to support multiple crypto database/slot. I don't think it is the crypto layer responsibility, crypto layer have provide the getBest for general usage, and it also provide the funtions to specify a certain slot. Here, I just use the specified slot while I don't want to get best from a dozen, I want to get the one that I want. Crypto layer has provide multiple choices and the ways, why xmlsec only permit just one way, and close anyother doors?

Yes, I remember that discussion. However, I was under impression that you want to do it in the application itself. Anyway, I wonder if you can take a look at NSS folks reply to my question about GetBestSlot here

http://groups-beta.google.com/group/netscape.public.mozilla.crypto/browse_thread/thread/8b39688b4d707111/af472f5a8552f8b6?q=Aleksey+Sanin+nss#af472f5a8552f8b6


It might make sense to go directly to NSS instead of creating yet another level.

I took a look from Nelson's mail. But I think maybe we have a little difference about how to use NSS crypto framework. NSS could manage three database, secmod.db, key3.db and cert8.db. "secmod.db" tells about the pkcs11 modules configuration, key3.db tells about key/trusted cert store, and cert8.db tells about certificate store. From my personal viewpoint, I do not want to relay on the secmod.db, just as Nelson said, user have the ability to use they own method to achieve what the moduitl want. At xmlsec library layer, I don't want to have any dependence over secmod.db, ( indeed, it does except the getBestSlot() function introduce the dependence ), I just want to purely depends on key store and cert store, I don't want to care what happened on secmod.db, and what on earth the user how to initialize his modules and how he use his dozen of slots in his environment. Just give me the key store and the cert store handler, OK, I'll let xmlsec works. Don't worry if the user have no options, xmlsec will use the default module by getting "best".

Once again, you think the application also have the ability do something make the "best" is what it supporsed to be. Yes, I does. But for one applicate, the "best" slot is number 1, then what about the second application if its "best" slot is number2, should they are running at the same time? So I perfer decouple the dependence at xmlsec library layer instead of the application layer.

OpenOffice's solution will try to find the appreciative key store from mozilla-style browsers, include firefox, mozilla, etc.( which means it doesn't import third party independent pkcs11 modules dynamically at present), then the keystore will be adopted into the xmlsec crypto engine. Which means that getBestSlot works at current for OpenOffice if there's a interface to adopt the key store specified. I just worry about the conflict that on one side, we use a specified key store/slot; on the other side, we use the getBestSlot() internally, at the situation that we don't understand how the user initialize his security environment.

I agree on with you if you insist on the getBestSlot() is enough for xmlsec, but please keep those interface to adopt a key store into xmlsec crypto engine.


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?

I think, xxxNodeWrite() is used by xxxXmlWrite. In the xmlSecNssKeyDataX509XmlWrite(), no xxxNodeWrite() is called. So I think they're useless. Then why I get rid of the xxxNodeWrite(), and create a new wheel of xxxXmlWrite()? In the older implementation, xml writer will get rid of the old and create a completely new signature/encryption template regardless what ever in it. It is not the expected experience. Sometimes, it is necessary to keep the template unchanged during signning or at least keep its original struct and add a little necessary stuff. However, I have to say that my implement also have the drawbacks over the requirement, I faild to find any usable control info from keyInfoCtx to character the template.

Thanks for explanations and let me look at this more.


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

I'm not use the NameRead function, StringRead funtion is only used by NameRead, so both of them are gone. I couldn't see any necessary to call NameRead before handling nss name.

OK, I see :)




_______________________________________________ xmlsec mailing list [email protected] http://www.aleksey.com/mailman/listinfo/xmlsec


_______________________________________________
xmlsec mailing list
[email protected]
http://www.aleksey.com/mailman/listinfo/xmlsec

Reply via email to