Closed Bug 399589 Opened 14 years ago Closed 14 years ago

PSM + tip of NSS, error ‘SECAlgorithmIDTemplate’ not declared


(Core :: Security: PSM, defect)

Not set





(Reporter: KaiE, Assigned: KaiE)



(Keywords: fixed1.8.1.13)


(2 files)

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
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.
Blocks: 399590
Attached patch Patch v1Splinter Review
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.
Attachment #284632 - Flags: review?(rrelyea)
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.  
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.
No longer depends on: 286642
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.
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

Comment on attachment 284632 [details] [diff] [review]
Patch v1

Attachment #284632 - Flags: review?(rrelyea) → review+
Attachment #284632 - Flags: approval1.9?
Attachment #284632 - Flags: approval1.9? → approval1.9+
Thanks for approval. I might delay this landing until we land bug 399590.
This one should be safe to land without waiting for NSS.

Closed: 14 years ago
Resolution: --- → FIXED
The same patch is needed in the 1.8 branch using system nss.
Attachment #300950 - Flags: review?
Attachment #300950 - Flags: review? → review?(kengert)
Flags: wanted1.8.1.x?
Umm, Since NSS 3.12 is not yet released, systems should not be using it
as their "system NSS", IMO.  
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.
Attachment #300950 - Flags: review?(kengert) → review+
Attachment #300950 - Flags: approval1.8.1.13?
Comment on attachment 300950 [details] [diff] [review]
Same against 1.8 branch

approved for, a=dveditz for release-drivers
Attachment #300950 - Flags: approval1.8.1.13? → approval1.8.1.13+
Keywords: checkin-needed
Checking in security/manager/ssl/src/nsKeygenHandler.cpp;
/cvsroot/mozilla/security/manager/ssl/src/nsKeygenHandler.cpp,v  <--  nsKeygenHandler.cpp
new revision:; previous revision: 1.36
Flags: wanted1.8.1.x? → wanted1.8.1.x+
You need to log in before you can comment on or make changes to this bug.