Last Comment Bug 399589 - PSM + tip of NSS, error ‘SECAlgorithmIDTemplate’ not declared
: PSM + tip of NSS, error ‘SECAlgorithmIDTemplate’ not declared
Status: RESOLVED FIXED
: fixed1.8.1.13
Product: Core
Classification: Components
Component: Security: PSM (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: ---
Assigned To: Kai Engert (:kaie) (on vacation)
:
: David Keeler [:keeler] (use needinfo?)
Mentors:
Depends on:
Blocks: 399590
  Show dependency treegraph
 
Reported: 2007-10-12 07:21 PDT by Kai Engert (:kaie) (on vacation)
Modified: 2008-02-28 17:10 PST (History)
9 users (show)
dveditz: wanted1.8.1.x+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (2.19 KB, patch)
2007-10-12 08:29 PDT, Kai Engert (:kaie) (on vacation)
rrelyea: review+
mtschrep: approval1.9+
Details | Diff | Splinter Review
Same against 1.8 branch (1.96 KB, patch)
2008-02-01 15:15 PST, Fabien Tassin
kaie: review+
dveditz: approval1.8.1.13+
Details | Diff | Splinter Review

Description Kai Engert (:kaie) (on vacation) 2007-10-12 07:21:03 PDT
I tried to compile Firefox with the tip of NSS.
I get this error:

/home/kaie/moz/ev/mozilla/security/manager/ssl/src/nsKeygenHandler.cpp:86: error: ‘SECAlgorithmIDTemplate’ was not declared in this scope
Comment 1 Kai Engert (:kaie) (on vacation) 2007-10-12 07:27:25 PDT
I expect the fix for this bug must land at the same time as updating PSM/Mozilla to use a newer NSS CVS tag, so I'm adding a dependency to bug 399590.
Comment 2 Kai Engert (:kaie) (on vacation) 2007-10-12 08:29:32 PDT
Created attachment 284632 [details] [diff] [review]
Patch v1

Maybe the fix is simpler as I had assumed, maybe it can be done at any time, without landing at the same time with newer NSS.

Look at the patch, you'll see SECAlgorithmIDTemplate 3 times.

First, it gets referenced, then PSM has a local copy, too. It seems SECAlgorithmIDTemplate is no longer provided by NSS headers.

I moved PSM's copy of this structure further up in the file, which makes it compile.
Comment 3 Nelson Bolyard (seldom reads bugmail) 2007-10-12 09:45:42 PDT
It may be that SECAlgorithmIDTemplate was declared in a public header file
but was not a public symbol, that is, the symbol was not exported from any
shared library.  If so, then it was correct to remove it from the public 
header file.  
Comment 4 Julien Pierre 2007-10-12 12:23:18 PDT
Nelson's explanation in comment 3 is correct . SECAlgorithmIDTemplate is a DERTemplate . The old DER_ decoder/encoder functions do not have the proper infrastructure to use templates in shared libraries on windows, unlike the current decoders/encoder, SEC_ASN1 and SEC_QuickDER . Legacy DER templates can only work when statically linked. The symbol was in a header file previously because util was being linked statically in multiple shared libraries. Now that util is a shared library, it is not possible to export this symbol from util anymore. So, I ended up duplicating this template in the 2 places that it was used in NSS, and taking it out from the header file, which was the right thing to do.

PSM shouldn't have been depending on the symbol in this header file since it was there only for users of the util static library. As it turns out, you stated in comment 2 that PSM has a local copy of the symbol too. That is the one you were actually using. You need to fix this by adding the symbol to a PSM header file.
Comment 5 Nelson Bolyard (seldom reads bugmail) 2007-10-12 12:38:45 PDT
I would also suggest that PSM stop using the old OLD "DER_" decoder and 
switch to the newer faster better cheaper smarter SEC_QuickDER decoder.
Comment 6 Robert Relyea 2007-10-12 13:34:09 PDT
It's unlikely a decoder issue, but an encoder issue.
SECAlgorithmIDTemplate is the last vestige of the old DER template code.

I'm murky as to why it was still used, but I vaguely remember something about how signed objects are encode differently then our default encoding for ASN1 templates. I seem to recall the early days of switching the default encoding of an optional NULL attribute between dropping the attribute and encoding and explicit NULL. One way is the way the standard was written, the other way was the way that most code implementented it.

I believe when we switched to the new ASN1 encoder, that encoder implemented the standard. We preserved the various places where signatures were being encoded to use the old decoder to guarrentee compatibility.

At some point it would be nice to maybe remove the old DER decoder (it has never been exported in NSS), and code the old DER encoder to call the SEC_ASN1 encoder after appropriately tweaking the old DER template. At the same time we can expunge SECAlgorithmIDTemplate from all of NSS. (and PSM could then follow suite).

AFAIK, SECOID_AlgorithmIDTemplate is used for all decoding (even in PSM), since we don't export the DERDecode function.k

bob
Comment 7 Robert Relyea 2007-10-12 16:28:30 PDT
Comment on attachment 284632 [details] [diff] [review]
Patch v1

r+
Comment 8 Kai Engert (:kaie) (on vacation) 2007-11-12 14:21:58 PST
Thanks for approval. I might delay this landing until we land bug 399590.
Comment 9 Robert Relyea 2007-11-15 12:41:06 PST
This one should be safe to land without waiting for NSS.

bob
Comment 10 Kai Engert (:kaie) (on vacation) 2007-11-19 09:03:29 PST
fixed
Comment 11 Fabien Tassin 2008-02-01 15:15:21 PST
Created attachment 300950 [details] [diff] [review]
Same against 1.8 branch

The same patch is needed in the 1.8 branch using system nss.
Comment 12 Nelson Bolyard (seldom reads bugmail) 2008-02-01 15:45:45 PST
Umm, Since NSS 3.12 is not yet released, systems should not be using it
as their "system NSS", IMO.  
Comment 13 Fabien Tassin 2008-02-01 15:56:23 PST
The thing is, many linux distros are already providing betas of Firefox 3 so NSS 3.12 (beta) was needed too. We're talking about the dev branch of those linux distros.
Comment 14 Daniel Veditz [:dveditz] 2008-02-11 12:10:54 PST
Comment on attachment 300950 [details] [diff] [review]
Same against 1.8 branch

approved for 1.8.1.13, a=dveditz for release-drivers
Comment 15 Reed Loden [:reed] (use needinfo?) 2008-02-13 02:29:21 PST
Checking in security/manager/ssl/src/nsKeygenHandler.cpp;
/cvsroot/mozilla/security/manager/ssl/src/nsKeygenHandler.cpp,v  <--  nsKeygenHandler.cpp
new revision: 1.36.2.1; previous revision: 1.36
done

Note You need to log in before you can comment on or make changes to this bug.