Remove the sslsample code from NSS source tree

RESOLVED FIXED in 3.12.1

Status

NSS
Tools
P4
normal
RESOLVED FIXED
11 years ago
9 years ago

People

(Reporter: Nelson Bolyard (seldom reads bugmail), Assigned: Julien Pierre)

Tracking

3.11.3
3.12.1

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

79.54 KB, patch
Nelson Bolyard (seldom reads bugmail)
: review+
Details | Diff | Splinter Review
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?
(Reporter)

Updated

11 years ago
Depends on: 264296
(Reporter)

Updated

10 years ago
Priority: -- → P4
Target Milestone: 3.12 → ---

Comment 1

10 years ago
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

10 years ago
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.
(Reporter)

Comment 3

10 years ago
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.
(Assignee)

Comment 4

9 years ago
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
(Assignee)

Updated

9 years ago
Target Milestone: --- → 3.12.1
(Assignee)

Comment 5

9 years ago
Created attachment 324863 [details] [diff] [review]
Remove SSLsample
Attachment #324863 - Flags: review?(nelson)
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+
(Assignee)

Comment 9

9 years ago
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.
(Assignee)

Comment 11

9 years ago
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
(Assignee)

Comment 13

9 years ago
Nelson,

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

Comment 14

9 years ago
I removed the sslsample links per comment #6 and comment #10
(Assignee)

Comment 15

9 years ago
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
Last Resolved: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.