Thank you Darren for your answer and for all explanations. For the webrev I've had some configuration problems, particularly on the setting of an environment variable, inducing me to use leaf files only, and resulting in some troubles v.r.t. the uploaded webrev, too. Thus, in the webrev appear some old changes (comments old code, etc.) which aren't actually present in the latest version. I apologize for the above and will try to post a correct webrev next time.
> There are several really big problems with this I'm afraid. > > You webrev is very strange it should have full paths from usr/src > downwards not just the leaf file names, this means some of the > webrev views don't work (in particular frames which is the version I > prefer to use). > > DJM-0a you can't just add a #define for CKM_BLOWFISH_CBC_PAD to > pkcs11t.h, those defines are controlled by the RSA PKCS#11 working > group. > > We also have a "policy" in the crypto group that we NEVER ever > modify that file, we only ever bring in complete new versions of it > from the RSA Cryptoki working group site. > > This is fundamental to this been acceptable and until the RSA > PKCS#11 working group assigns a mech number for CKM_BLOWFISH_CBC_PAD > we can't > do it that way. What can be done though is we can add a vendor > defined > mechanism (those with mech numbers above 0x80000000) but that must > get added to cryptoki.h not any of the pkcs11t.h files. > > DJM-0b The padding code should not be added to softtoken code, > instead CBC_PAD should be added as a new mode in $SRC/common/crypto/ > modes/. > Any existing algorithms that do CBC_PAD should be updated to use the > new algorithm independent mode code. By adding it as a new mode > it makes it available to both the userland and kernel land frameworks. > I see :( ... anyway, I would like to complete my task in the right way, changing my previous code submission as you sketched before. I think I could manage it without a substantial effort, if I can eventually rely on some more indications from you and Krishna during this process. What do you think about it? > > I'm sorry to say but the whole approach here is not acceptable given > the current way the crypto framework is implemented. If you have > been misguided by our team in the past I'm sorry but unfortunately > other changes have been made that makes your approach to fixing this > bug highly undesirable. > > > Given the above the rest of my comments are really not that relevant > because DJM-0a and DJM-0b are fundamental issues that will change the > way the rest of this is done anyway. I've listed the rest of the > comments below in the hope that you may find them a useful > codereview experience. > > > > DJM-1 softSlotToken.c has a mis merge you have readadded a removed > comment. > > DJM-2 You need to run 'hg nits' and fix all the things it complains > about, like #pragma idents etc that are no longer allowed. > > DJM-3 decrypt.c you have lots of changes to the carefully written > comment that I very strongly disagree with. In particular why have > you choosen to rewrite the initial block comment to remove all the > options and remove all the information about tokens. Maybe this is > just a missmerge. > > DJM-4 decrypt.c:197 We never comment out code like this, if it is > no longer needed it should be removed. > > DJM-5 decrypt.c:307 what is this for ? > > DJM-6 decrypt.c:400 This makes no sense. > > DJM-7 decrypt.c:441/443 Don't change these lines they are purposely > uint32_t because they are written to files and need to be the same > in a 32 bit and 64 bit compilation environment since the command is > delivered as both 32 and 64 bit binaries. Is this maybe yet another > mis merge ? > > DJM-8 decrypt.c:512/529. What is the (1) and (2) about ? > > DJM-9 decrypt.c:903/904 same comment as DJM-7 > > DJM-10 decrypt.c:1055 English comments only please. > > DJM-11 decrypt.c: crypt_multipart what are all the extra variables > for they don't appear (from the diffs) to be used. > > > > > -- > Darren J Moffat Let me know. Thank you. -Giuliano Santorelli _______________________________________________ opensolaris-code mailing list opensolaris-code@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/opensolaris-code
