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

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

[Kai Engert [:KaiE:]]

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:]]

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:]]

Attachment: 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)]

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]

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

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]

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]

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

r+

Comment 8

[Kai Engert [:KaiE:]]

Thanks for approval. I might delay this landing until we land bug 399590.

Comment 9

[Robert Relyea]

This one should be safe to land without waiting for NSS.

bob

Comment 10

[Kai Engert [:KaiE:]]

fixed

Comment 11

[Fabien Tassin]

Attachment: 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)]

Umm, Since NSS 3.12 is not yet released, systems should not be using it
as their "system NSS", IMO.  

Comment 13

[Fabien Tassin]

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]]

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]]

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