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

RESOLVED FIXED

Status

()

Core
Security: PSM
RESOLVED FIXED
10 years ago
9 years ago

People

(Reporter: kaie, Assigned: kaie)

Tracking

({fixed1.8.1.13})

Trunk
x86
Linux
fixed1.8.1.13
Points:
---
Bug Flags:
wanted1.8.1.x +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

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

Comment 1

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

Comment 2

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

Comment 4

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

Updated

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

Comment 6

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

10 years ago
Comment on attachment 284632 [details] [diff] [review]
Patch v1

r+
Attachment #284632 - Flags: review?(rrelyea) → review+
(Assignee)

Updated

10 years ago
Attachment #284632 - Flags: approval1.9?

Updated

10 years ago
Attachment #284632 - Flags: approval1.9? → approval1.9+
(Assignee)

Comment 8

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

Comment 9

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

bob
(Assignee)

Comment 10

10 years ago
fixed
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED

Comment 11

10 years ago
Created attachment 300950 [details] [diff] [review]
Same against 1.8 branch

The same patch is needed in the 1.8 branch using system nss.
Attachment #300950 - Flags: review?

Updated

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

Comment 13

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

Updated

10 years ago
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 1.8.1.13, 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: 1.36.2.1; previous revision: 1.36
done
Keywords: checkin-needed → fixed1.8.1.13
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.