359302 - Remove the sslsample code from NSS source tree

Status: RESOLVED FIXED

(NSS :: Tools, defect, P4)

Description

[Nelson Bolyard (seldom reads bugmail)]

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?

Comment 1

[Rob Crittenden]

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

Comment 2

[Wan-Teh Chang]

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.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

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.

Comment 4

[Julien Pierre]

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.

Comment 5

[Julien Pierre]

Attachment: Remove SSLsample

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

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

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

Also need to remove this line:
http://lxr.mozilla.org/security/source/security/nss/cmd/manifest.mn#75

Comment 8

[Nelson Bolyard (seldom reads bugmail)]

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.

Comment 9

[Julien Pierre]

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.

Comment 10

[Nelson Bolyard (seldom reads bugmail)]

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.

Comment 11

[Julien Pierre]

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.

Comment 12

[Nelson Bolyard (seldom reads bugmail)]

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

Comment 13

[Julien Pierre]

Nelson,

I tried that, and I do not have access to the doc pages on www.mozilla.org .

Comment 14

[glen beasley]

I removed the sslsample links per comment #6 and comment #10

Comment 15

[Julien Pierre]

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