Closed Bug 510135 Opened 10 years ago Closed 10 years ago

Don't need to decode policy OID in CERTPolicyInfo

Categories

(Core :: Security: PSM, defect, trivial)

defect
Not set
trivial

Tracking

()

RESOLVED FIXED
mozilla1.9.3a1
Tracking Status
status1.9.2 --- beta1-fixed
status1.9.1 --- .4-fixed

People

(Reporter: wtc, Assigned: wtc)

Details

Attachments

(3 files, 1 obsolete file)

Attached patch Proposed patchSplinter Review
After calling CERT_DecodeCertificatePoliciesExtension, the
policyInfos in the returned CERTCertificatePolicies already
contains the decoded policy OID (the 'oid' field):

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/certdb/polcyxtn.c&rev=1.11&mark=183,228#183

so it's not necessary to call SECOID_FindOIDTag again.

Also, use CERT_REV_M_IGNORE_MISSING_FRESH_INFO (the default)
to make it easier to understand the code.
Attachment #394189 - Flags: superreview?(kaie)
Attachment #394189 - Flags: review?(alexei.volkov.bugs)
Attachment #394189 - Flags: review?(alexei.volkov.bugs) → review+
Comment on attachment 394189 [details] [diff] [review]
Proposed patch

r=alexei. I didn't know that we have this code in psm. :)
Comment on attachment 394189 [details] [diff] [review]
Proposed patch

Pushed to mozilla-central in changeset 40ed0ff13420:
http://hg.mozilla.org/mozilla-central/rev/40ed0ff13420
The leak of the certificatePolicies extension is more serious
because it leaks every time we verify a certificate for EV.  This
fix is worth backporting to older Firefox releases.

The leak of the subjectAltName extension occurs on out-of-memory
error paths.
Attachment #394504 - Flags: superreview?(kaie)
Attachment #394504 - Flags: review?(alexei.volkov.bugs)
Comment on attachment 394504 [details] [diff] [review]
Fix leaks of certificatePolicies and subjectAltName extensions

r=alexei
Attachment #394504 - Flags: review?(alexei.volkov.bugs) → review+
Comment on attachment 394504 [details] [diff] [review]
Fix leaks of certificatePolicies and subjectAltName extensions

I pushed this patch to mozilla-central in changeset c4beffbd4540:
http://hg.mozilla.org/mozilla-central/rev/c4beffbd4540
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3
This patch fixes leaks of decoded certificatePolicies
extensions.  This leak occurs whenever we verify a
certificate for EV, so I propose that we fix the leak
in mozilla-1.9.1.
Attachment #394689 - Flags: superreview?(kaie)
Attachment #394689 - Flags: review?(honzab.moz)
Attachment #394689 - Flags: approval1.9.2?
Attachment #394689 - Flags: approval1.9.1.3?
Comment on attachment 394504 [details] [diff] [review]
Fix leaks of certificatePolicies and subjectAltName extensions

The change to nsNSSIOLayer.cpp in this patch breaks
the Linux build because the jump to label 'loser'
crosses initialization of 'CERTGeneralName* current'.
So I backed out that change in changeset ef2e4699b319:
http://hg.mozilla.org/mozilla-central/rev/ef2e4699b319
So only this change to nsIdentityChecking.cpp is still
in mozilla-central.
Attachment #394504 - Attachment is obsolete: true
Attachment #394504 - Flags: superreview?(kaie)
Attachment #394689 - Flags: review?(honzab.moz) → review+
Target Milestone: mozilla1.9.3 → mozilla1.9.3a1
Comment on attachment 394689 [details] [diff] [review]
Fix leaks of certificatePolicies extensions (for mozilla-1.9.1)

r=kaie
Attachment #394689 - Flags: superreview?(kaie) → superreview+
Attachment #394689 - Flags: approval1.9.1.3? → approval1.9.1.4?
Comment on attachment 394689 [details] [diff] [review]
Fix leaks of certificatePolicies extensions (for mozilla-1.9.1)

Needs to land in 1.9.2 first.
This needs approval on 1.9.2 as Dan says. Nominating it for blocking to get it on someone's radar.
Flags: blocking1.9.2?
Comment on attachment 394689 [details] [diff] [review]
Fix leaks of certificatePolicies extensions (for mozilla-1.9.1)

a192=beltzner
Attachment #394689 - Flags: approval1.9.2? → approval1.9.2+
Comment on attachment 394689 [details] [diff] [review]
Fix leaks of certificatePolicies extensions (for mozilla-1.9.1)

I pushed this patch to mozilla-1.9.2 in changeset c4c08b8df2da:
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/c4c08b8df2da
Flags: blocking1.9.2?
Attachment #394689 - Flags: approval1.9.1.4? → approval1.9.1.4+
Comment on attachment 394689 [details] [diff] [review]
Fix leaks of certificatePolicies extensions (for mozilla-1.9.1)

Why is there a hanging sr request on the earlier patch? Is it obsolete? Looks like it got checked in?

anyway, Approved for 1.9.1.4, a=dveditz for release-drivers on this patch.
Comment on attachment 394689 [details] [diff] [review]
Fix leaks of certificatePolicies extensions (for mozilla-1.9.1)

I pushed this patch to mozilla-1.9.1 in changeset 3da4b7e1b598:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/3da4b7e1b598
Attachment #394189 - Flags: superreview?(kaie)
You need to log in before you can comment on or make changes to this bug.