Closed
Bug 230655
Opened 21 years ago
Closed 16 years ago
Cert viewer does not show readable subject alt names
Categories
(Core Graveyard :: Security: UI, enhancement, P2)
Tracking
(Not tracked)
RESOLVED
WORKSFORME
People
(Reporter: ken2006, Assigned: jgmyers)
References
()
Details
Attachments
(3 files)
35.50 KB,
patch
|
ddrinan0264
:
review+
jag+mozilla
:
superreview+
|
Details | Diff | Splinter Review |
18.89 KB,
patch
|
ddrinan0264
:
review+
|
Details | Diff | Splinter Review |
9.98 KB,
patch
|
ddrinan0264
:
review+
jag+mozilla
:
superreview-
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7a) Gecko/20040107 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7a) Gecko/20040107 Not all details of a cert are reported, for example the subject-alternative-name(s) in the example URL's cert are *.onnet.cc, *.kensystem.om, & *.interactiveengines.com. However, none of these aliases can be seen by the cert exam tool. When examining the cert served by https://www.kensystem.com, only the *.interactiveengines.com CN can be seen, this may confuse some folks. Reproducible: Always Steps to Reproduce:
Comment 1•21 years ago
|
||
Actually, it DOES show all of them, but a) the extension OIDs are shown in decimal, not as meaningful strings, and b) the extension values are shown in hexadecimal, not ASCII or UTF8. So, unless you can read the hex, and know the OIDs (as some of us do :-) the display is pretty opaque. So, I am changing the summary of this bug report to be slightly more accurate.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P2
Summary: Certificate viewer does not show alternate certificate subjects → Cert viewer does not show readable subject alt names
Assignee | ||
Updated•20 years ago
|
Severity: normal → enhancement
Assignee | ||
Comment 2•20 years ago
|
||
The OIDs were addressed by the fix to bug 97406. Work on the values is blocked by bug 232812, which has been awaiting review for a week.
Assignee: kaie → jgmyers
Depends on: 232812
Assignee | ||
Comment 3•20 years ago
|
||
I believe I need to use CERT_DecodeGeneralName() to implement this. That function is exported in nss.def, but it is declared in genname.h which NSS doesn't export. Nelson?
No longer depends on: 232812
Comment 4•20 years ago
|
||
It seems that we exported CERT_DecodeGeneralName for PSM, but the PSM function that uses it has been commented out. In any case, if CERT_DecodeGeneralName is suitable for being exported, we should move its declaration from genname.h to cert.h.
Assignee | ||
Comment 5•20 years ago
|
||
Assignee | ||
Comment 6•20 years ago
|
||
Assignee | ||
Comment 7•20 years ago
|
||
Requires a newer version of NSS than is currently tagged. Largely cribbed off of nss/cmd/lib/secutil.c
Assignee | ||
Updated•20 years ago
|
Attachment #144285 -
Flags: review?(ddrinan0264)
Assignee | ||
Updated•20 years ago
|
Attachment #144286 -
Flags: review?(ddrinan0264)
Assignee | ||
Updated•20 years ago
|
Attachment #144287 -
Flags: review?(ddrinan0264)
Updated•20 years ago
|
Attachment #144285 -
Flags: review?(ddrinan0264) → review+
Updated•20 years ago
|
Attachment #144286 -
Flags: review?(ddrinan0264) → review+
Updated•20 years ago
|
Attachment #144287 -
Flags: review?(ddrinan0264) → review+
Assignee | ||
Updated•20 years ago
|
Attachment #144285 -
Flags: superreview?(jag)
Assignee | ||
Updated•20 years ago
|
Attachment #144286 -
Flags: superreview?(jag)
Assignee | ||
Updated•20 years ago
|
Attachment #144287 -
Flags: superreview?(jag)
Comment 8•20 years ago
|
||
Comment on attachment 144286 [details] [diff] [review] 2 of 3: Make GetPIPNSSBundleString append instead of replace Instead of doing this, perhaps you could pass in a parameter that specifies whether to replace or append, and make it default to replace?
Comment 9•20 years ago
|
||
Comment on attachment 144287 [details] [diff] [review] 3 of 3: decode subject alt names and simple strings In ProcessRawString and ProcessRawBytes, it looks like you can get into a situation (data->len == 0 or data->data[data->len-1] == '\n') where you'll emit |level| spaces without them being followed by anything. How about: static nsresult ProcessRawString(SECItem *data, int level, nsAString &text) { PRUint32 i; PRBool doIndent = PR_TRUE; ProcessIndent(level, text); for (i=0; i < data->len; i++) { if (doIndent) { ProcessIndent(level, text); doIndent = PR_FALSE; } char c = data->data[i]; if (c == '\r' && i+1 < data->len && data->data[i+1] == '\n') continue; if (c == '\n') { text.Append(PRUnichar('\n')); doIndent = PR_TRUE; continue; } if (c < ' ' || c >= 0x7f) c = '.'; text.Append(PRUnichar(c)); } text.Append(PRUnichar('\n')); return NS_OK; } Also, to aid l10n and RTL languages, instead of manually appending names etc. to CertDumpGNameOther etc., put in a %S and use PIPBundleFormatStringFromName. Looks good otherwise.
Attachment #144287 -
Flags: superreview?(jag) → superreview-
Comment 10•20 years ago
|
||
Comment on attachment 144285 [details] [diff] [review] 1 of 3: Move cert details code to nsNSSCertHelper.cpp Why did you move nsNSSCertificate::CreateTBSCertificateASN1Struct and nsNSSCertificate::CreateASN1Struct? Wouldn't it be better to expose the helper functions they need in nsNSSCertHelper.h and keep the above functions in the nsNSSCertificate.cpp file?
Assignee | ||
Comment 11•20 years ago
|
||
Comment on attachment 144285 [details] [diff] [review] 1 of 3: Move cert details code to nsNSSCertHelper.cpp Moving the functions puts all of the code related to the cert details tab in one file. That allows us to avoid exposing functions across files and gives the compiler better opportunities to make inter-function optimizations. There is no advantage to having CreateTBSCertificateASN1Struct and CreateASN1Struct in nsNSSCedrtificate.cpp
Comment 12•20 years ago
|
||
Comment on attachment 144285 [details] [diff] [review] 1 of 3: Move cert details code to nsNSSCertHelper.cpp sr=jag Could you address my questions/comments on the other two patches?
Attachment #144285 -
Flags: superreview?(jag) → superreview+
Reporter | ||
Comment 13•19 years ago
|
||
Is it too late for any patch for this to make it into Deerpark betas? It SURE would be nice of we could fully inspect certificates in 1.5, especially subject-alt names. Bug 281446 is a duplicate of this; please feel free to flag that, and/or communicate with any independent fixers there. Bug bug 259031 also appears related.
Comment 14•19 years ago
|
||
*** Bug 281446 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 15•19 years ago
|
||
Basically, this patch train got derailed by comment 8. I never found the time to go and re-audit all of the code to implement an arbitrary design change.
Comment 16•19 years ago
|
||
Ken, it is too late for Firefox 1.5. The next release will be Firefox 2.0.
Updated•17 years ago
|
QA Contact: bmartin → ui
Updated•17 years ago
|
Blocks: CVE-2008-2809
Comment 17•16 years ago
|
||
I think this got fixed a while ago. When I use a trunk build and cert viewer to display the cert from the site mentioned in comment 0, cert viewer lists 4 subject alt names.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → WORKSFORME
Updated•16 years ago
|
Attachment #144286 -
Flags: superreview?(jag)
Updated•8 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•