Closed Bug 942172 Opened 7 years ago Closed 6 years ago
algtag set but not used in CERTUTIL
_Generate Private Key
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:28.0) Gecko/20100101 Firefox/28.0 (Beta/Release) Build ID: 20131121171534 Steps to reproduce: Compiled Firefox against changeset 156520:cf378dddfac8 Actual results: Spotted a warning in build logs: gcc -o /home/maxim/projects/mozilla/obj-x86_64-unknown-linux-gnu/security/nss/cmd/certutil/keystuff.o -c -O2 -gdwarf-2 -D_POSIX_SOURCE -D_BSD_SOURCE -D_XOPEN_SOURCE -fPIC -DLINUX2_1 -Wall -Werror-implicit-function-declaration -Wno-switch -pipe -DLINUX -Dlinux -DHAVE_STRERROR -DXP_UNIX -DNSPR20 -UDEBUG -DNDEBUG -D_REENTRANT -DNSS_ENABLE_ECC -DUSE_UTIL_DIRECTLY -DNO_NSPR_10_SUPPORT -I/home/maxim/projects/mozilla/obj-x86_64-unknown-linux-gnu/dist/include/nspr -I/home/maxim/projects/mozilla/obj-x86_64-unknown-linux-gnu/dist/include/nspr -I/home/maxim/projects/mozilla/obj-x86_64-unknown-linux-gnu/dist/include/nss -I/home/maxim/projects/mozilla/obj-x86_64-unknown-linux-gnu/dist/private/nss -I/home/maxim/projects/mozilla/obj-x86_64-unknown-linux-gnu/dist/include/dbm -I/home/maxim/projects/mozilla/obj-x86_64-unknown-linux-gnu/dist/include/seccmd keystuff.c keystuff.c: In function ‘CERTUTIL_GeneratePrivateKey’: keystuff.c:497:15: warning: variable ‘algtag’ set but not used [-Wunused-but-set-variable] SECOidTag algtag; ^ Verified that this variable is actually set and not used under any macro directive. I'm not sure whether it's a bug or just some forgotten code.
I just came across this today. Actually, while checking the security of key/csr/cert generation I saw that it gets set to a very insecure algorithm: algtag = SEC_OID_PKCS1_MD5_WITH_RSA_ENCRYPTION; and spent quite a while making double extra super sure it isn't actually used for anything. I'm 99.999999% sure it isn't. what's odd is that it *never appears to have done anything*, ever, in the history of Mozilla NSS - if you go back to the very first check-in of the file, hg rev 205 in March 2000, algtag is set but never used in that version of the file. Being a bit obsessive I've been trying to dig out a copy of the initial incomplete open source tarball dump of NSS from somewhere to see if it did anything in *that*, but not having any luck so far. :) would be good to take it out so other people grepping for MD5 don't waste their time, I guess.
This patch just gets rid of algtag.
Assignee: nobody → cykesiopka.bmo
Status: NEW → ASSIGNED
Attachment #8530582 - Flags: review?(emaldona)
Thanks for the review.
+ Correct a minor issue with the patch comment
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.18
mass change target milestone to 3.17.4
Target Milestone: 3.18 → 3.17.4
You need to log in before you can comment on or make changes to this bug.