45878 - Win-Linux-Cannot pickup cert from verisign

Status: VERIFIED FIXED

(Core Graveyard :: Security: UI, defect, P3)

Description

[John Unruh]

1.) Important - With a new profile, open PSM, click on Certificates, and create 
a password, or at least log into your DB if you already have a profile.
2.) Visit the above URL, click on Enroll now, and request a trial cert.
3.) Wait a few minutes for email from Verisign.
4.) Follow the cert pickup instructions, paste in your PIN, and click on Submit.
What happens: WinNT build 71910- crash in netscp6.exe.
Linux Build 71908- crash and Netscape disappears.
Not yet tested on Mac.
The Linux stack trace is available at http://cyclone/main/talkback.cfm
Click on FastFind, enter [e-mail], and look for 14470232.

Comment 1

[Javier Delgadillo]

This crashes downloading the cert.  I'll need help from the networking group to 
dig further into this.  

dougt: any ideas on who may be able to help out on this?

Here's the assert that seems to hint at the problem:

D:\moz-client\mozilla\netwerk\streamconv\converters\nsMultiMixedConv.cpp:412
   // if we hit this assert, it's likely that the data producer isn't sticking
   // headers after a token to delineate a new part. This is required. If
   // the server's not sending those headers, the server's broken.
       NS_ASSERTION(mPartChannel, "our channel went away :-(");

Comment 2

[Javier Delgadillo]

I've tried getting a TracePlus output to see what's going on, but Mozilla hangs 
right before I submit the form that gets the cert when TracePlus is enabled.  If 
I don't use TracePlus, then everything works fine.  Any other clues on how to 
get the desired info?  Switching over to Linux would be quite painful for me.

Comment 3

[Javier Delgadillo]

I've been looking at this bug and really don't know how to proceed.  Can someone 
from the networking group look into this?  All of our internal test with 
certificate servers work, there's some weirdness with VeriSign's servers at the 
network level that's causing this to crash.

Comment 4

[Judson Valeski]

traceplus worked for me. I'm attatching a patch which fixes the crash. The 
problem is that the verisign server spits back a multi mixed boundary that is 
wrapped in double quotes *and* contains whitespace inside the boundary. We 
weren't handling this case (it's illegal), now we are. I'm attatching a patch, 
someone (gagan) please review. I'm doing the string manipulation by hand becuase 
I couldn't figure out CopressSpace() on our string lib (it takes a PRUnichar for 
something and I couldn't figure out what).

This fixes the crash and we now download all the data. I didn't notice the cert 
being installed though, that wouldn't be my area.

Comment 5

[Judson Valeski]

Attachment: proposed fix

Comment 6

[Javier Delgadillo]

I applied valeski's patches and Mozilla no longer crashes.  But now, I need some 
coaching on how the stream listener stuff works.  The cert downloader gets 
called with the HTML of the page as the contents, so PSM then gets an HTML which 
it tries to interpret as a cert package and fails.  So the cert is never 
successfully imported.

I need pointers on where to start looking in necko to debug this problem 
further.

Comment 7

[Judson Valeski]

need code review for this patch, gagan?

also, did this feature ever work in SeaMonkey?

Comment 8

[Javier Delgadillo]

This feature works with CMS (Certificate Management System) 4.x

That's what I used to test when I wrote the code originally.  QA has started 
testing with outside CA's now and it's not working with their implementations.

I'm working on getting PSM to spit out what it's sending back to the client so I 
can have a better idea of what's going on.

Will post anything else I find in this bug report.

Comment 9

[Javier Delgadillo]

Attachment: The responses sent back from VeriSign

Comment 10

[Javier Delgadillo]

Comparing what PSM gets sent as the cert blob to the returned responses, mozilla 
sends the contents of rsa2.bits in the attachment.

Comment 11

[Judson Valeski]

updated patch. turns out there is indeed a string routine that does what I want.

Index: nsMultiMixedConv.cpp
===================================================================
RCS file: /cvsroot/mozilla/netwerk/streamconv/converters/nsMultiMixedConv.cpp,v
retrieving revision 1.39
diff -c -r1.39 nsMultiMixedConv.cpp
*** nsMultiMixedConv.cpp        2000/07/07 22:02:26     1.39
--- nsMultiMixedConv.cpp        2000/07/25 21:37:11
***************
*** 240,246 ****

      nsCAutoString boundaryString(bndry);
      if (attrib) *attrib = ';';
!     boundaryString.StripWhitespace();

      mToken = boundaryString.ToNewCString();
      if (!mToken) return NS_ERROR_OUT_OF_MEMORY;
--- 240,247 ----

      nsCAutoString boundaryString(bndry);
      if (attrib) *attrib = ';';
!
!     boundaryString.Trim(" \"");

      mToken = boundaryString.ToNewCString();
      if (!mToken) return NS_ERROR_OUT_OF_MEMORY;

Comment 12

[Javier Delgadillo]

Another question, valeski said I needed to have a nsIContentListener around.  
dougt already implemented a CertDownLoader class that inherits from 
nsIStreamListener.  Are both necessary?  Which one should stay?

Comment 13

[John Unruh]

Nominating for nsbeta2.

Comment 14

[Judson Valeski]

patch is checked in.

Comment 15

[leger]

marking Fixed...junruh, please verify ASAP.

Comment 16

[Javier Delgadillo]

This is not fixed, valeski's patch is just a step along the way to getting this 
work.  I'm working on adding a content listener per valeski's and mscott's 
suggestion.

Comment 17

[Javier Delgadillo]

Have code in my tree that fixes this (almost).  One outstanding issue with 
regards to properly registering/de-registering my content listener.  Sending 
e-mail to mscott for clarification.

Comment 18

[Javier Delgadillo]

Re-opening bug because this still doesn't work.

Comment 19

[Javier Delgadillo]

Attachment: Diffs for new CertContentListener, need someone to review.

Comment 20

[Javier Delgadillo]

valeski reviewed the changes.  Is it too late for PR2?  If not is there a branch 
that I'd need to check into?

Comment 21

[Javier Delgadillo]

Attachment: Proposed patch after mscott and valeski review.

Comment 22

[leger]

javi, please check in as soon as branch opens.  THanks!

Comment 23

[Javier Delgadillo]

Checked in fix to the tip.  Will check in fix to branch when it opens.

Comment 24

[Javier Delgadillo]

Fix checked into M17 branch and the tip.

Comment 25

[John Unruh]

Reopening. Using WinNT 072904, Linux 073104. Everything seems to go smoothly, 
and success looks assured. The problem is that when it is all over, I open 
Security Advisor, click on certificates, and do not see the verisign cert. Mac 
behaves differently, (does not go to page "step 2"), so I will open a separate 
Mac bug. Removing crash keyword.

Comment 26

[Javier Delgadillo]

Are you sure this isn't because you're entering duplicate information and 
VeriSign isn't letting you get another cert with the same information?

Comment 27

[John Unruh]

Fixed. That seems to be the case. I just used a new yahoo email address and fake 
name, and now it worked. Verisign apparently doesn't warn you that they aren't 
going to give the same email address another cert.

Comment 28

[John Unruh]

Verified.

Comment 29

[John Unruh]

Mass changing Security:Crypto to PSM

Comment 30

[John Unruh]

Mass changing Security:Crypto to PSM