Closed Bug 359302 Opened 15 years ago Closed 13 years ago
Remove the sslsample code from NSS source tree
The sslsample programs (client and server) are anything but exemplary. They have bugs filed against them. Rather than fixing those bugs, I propose to get rid of all the sslsample code from the NSS source tree. Any objections?
Priority: -- → P4
Target Milestone: 3.12 → ---
Are they so bad that they are worse than nothing at all? Will they be replaced at some point? One of the common criticisms of NSS is that it is a difficult API to program in and the lack of documentation and good sample code doesn't help. gnuTLS is often raised as an example of what to do, with users claiming very clear documentation and samples on how to do common tasks (client auth, cert verification, generating a hash, etc).
Rob, if you can update and fix the bugs in the sslsample code, that will be ideal. The proposed replacement is to use our test programs tstclnt, strsclnt, and selfserv as sample code. We use these test programs every day, so they are "by definition" correct. But I'm afraid that they are too complicated to serve as sample code. An NSS user, Yahel Zamir, posted his simple SSL client and server in the mozilla.crypto newsgroup: http://groups.google.com/group/mozilla.dev.tech.crypto/browse_thread/thread/c2ca5a9bd66f37f5/c891b7864eb54af3?hl=en#c891b7864eb54af3 We can also use his code.
In reply to comment 1: Yes, IMO, it is worse than nothing at all. SSLSample does many things that we tell developers NOT to do. Many people have filed bugs saying, in effect, I did it just like SSLSample, and it didn't work. That is a waste of everyone's time. But the alternative is NOT "nothing at all". We have tstclnt (demonstrates use of non-blocking single threaded use), strsclnt (demonstrates blocking multi-threaded use), selfserv (demonstrates multi-thraded AND multi-process use). Our documentation should point to THOSE programs as the true examples. Yes, they're non-trivial, but they show how to do just about everything.
Rob, I agree with Nelson that this code is worse than nothing at all. I would like to delete this code now. The code for this tool continues to show up in code searches when doing enhancement work, such as bug 423839 . I propose to delete this now.
Assignee: nobody → julien.pierre.boogz
This change will also necessitate changes to the following web pages: http://www.mozilla.org/projects/security/pki/nss/buildnss_31.html http://www.mozilla.org/projects/security/pki/nss/ref/ssl/sslfnc.html
Also need to remove this line: http://lxr.mozilla.org/security/source/security/nss/cmd/manifest.mn#75
Comment on attachment 324863 [details] [diff] [review] Remove SSLsample This patch is OK as far as it goes, but it is incomplete. Please see the preceding comments for details. Please do not commit this patch until the other pieces are done or are in hand.
Attachment #324863 - Flags: review?(nelson) → review+
Nelson, Thanks for the review. I can make the change to manifest.mn at the same time I check in the patch. Do you really need to review that one-line change ? Rergarding the web page changes, I don't think a change to 3.12.1 necessitates a change to an NSS 3.1 build page. I'm not sure about the API reference you pointed to. Can you tell me what needs to be changed there ? And whatever needs to be changed, I don't think I have the permissons to change it.
Julien, No, I don't need to review that extra one-line patch. I just want all the changes to be made the same day. We don't make a new build instructions page with every release, so old build instruction pages end up being reused by users for versions after the one named in the page. If we're sure that the build page is NOT the most recent one, then I guess we needn't change it, except to ensure that the page does not contain any links that will become dangling links. The API page cited above has numerous references (links) to SSLSample in it, and this change will cause all of those links to become dangling links. All such dangling links should be removed or changed to point to some other page. http://www.mozilla.org/projects/security/pki/nss/ref/ssl/sslfnc.html#1088922 http://www.mozilla.org/projects/security/pki/nss/ref/ssl/sslfnc.html#1126643 http://www.mozilla.org/projects/security/pki/nss/ref/ssl/sslfnc.html#1126695 http://www.mozilla.org/projects/security/pki/nss/ref/ssl/sslfnc.html#1130593 http://www.mozilla.org/projects/security/pki/nss/ref/ssl/sslfnc.html#1127318 Please don't commit the changes to remove the sslsample sources until the above links have been changed or removed.
Nelson, The most current build instructions I could find are at : http://www.mozilla.org/projects/security/pki/nss/nss-3.11.4/nss-3.11.4-build.html I don't believe that I have permission to edit the other page.
Julien, if you have CVS access to any pages in www.mozilla.org then you should have cvs access to all of them. set CVSROOT to something like: :ext:<your email address>@cvs.mozilla.org:/www This is the same as for the source code, except for that last path name. Use your same ssh credentials as for source code. The file is mozilla-org/html/projects/security/pki/nss/ref/ssl/sslfnc.html
Nelson, I tried that, and I do not have access to the doc pages on www.mozilla.org .
Thanks, Glen ! I checked in the patch to the trunk. Checking in manifest.mn; /cvsroot/mozilla/security/nss/cmd/manifest.mn,v <-- manifest.mn new revision: 1.27; previous revision: 1.26 done Removing SSLsample/Makefile; /cvsroot/mozilla/security/nss/cmd/SSLsample/Makefile,v <-- Makefile new revision: delete; previous revision: 1.3 done Removing SSLsample/README; /cvsroot/mozilla/security/nss/cmd/SSLsample/README,v <-- README new revision: delete; previous revision: 1.2 done Removing SSLsample/client.c; /cvsroot/mozilla/security/nss/cmd/SSLsample/client.c,v <-- client.c new revision: delete; previous revision: 1.7 done Removing SSLsample/client.mn; /cvsroot/mozilla/security/nss/cmd/SSLsample/client.mn,v <-- client.mn new revision: delete; previous revision: 1.4 done Removing SSLsample/gencerts; /cvsroot/mozilla/security/nss/cmd/SSLsample/gencerts,v <-- gencerts new revision: delete; previous revision: 1.3 done Removing SSLsample/make.client; /cvsroot/mozilla/security/nss/cmd/SSLsample/make.client,v <-- make.client new revision: delete; previous revision: 1.3 done Removing SSLsample/make.server; /cvsroot/mozilla/security/nss/cmd/SSLsample/make.server,v <-- make.server new revision: delete; previous revision: 1.3 done Removing SSLsample/server.c; /cvsroot/mozilla/security/nss/cmd/SSLsample/server.c,v <-- server.c new revision: delete; previous revision: 1.10 done Removing SSLsample/server.mn; /cvsroot/mozilla/security/nss/cmd/SSLsample/server.mn,v <-- server.mn new revision: delete; previous revision: 1.4 done Removing SSLsample/sslerror.h; /cvsroot/mozilla/security/nss/cmd/SSLsample/sslerror.h,v <-- sslerror.h new revision: delete; previous revision: 1.3 done Removing SSLsample/sslsample.c; /cvsroot/mozilla/security/nss/cmd/SSLsample/sslsample.c,v <-- sslsample.c new revision: delete; previous revision: 1.12 done Removing SSLsample/sslsample.h; /cvsroot/mozilla/security/nss/cmd/SSLsample/sslsample.h,v <-- sslsample.h new revision: delete; previous revision: 1.5 done
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.