Bug 425801 (vfychain-ocsp)

vfychain: Add support for OCSP checking

RESOLVED FIXED in 3.12

Status

NSS
Tools
P2
enhancement
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: kaie, Assigned: kaie)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

10 years ago
It should be possible to request that vfychain performs an OCSP check.

There are several flavors that could be done using libpkix etc., but I propose to start with the most general choice:
- enable OCSP "if a default responder is available"
- allow OCSP to be performed when doing libpkix verification
(Assignee)

Comment 1

10 years ago
Created attachment 312358 [details] [diff] [review]
Patch v1
Attachment #312358 - Flags: review?(nelson)
(Assignee)

Comment 2

10 years ago
Created attachment 312366 [details] [diff] [review]
Patch v2

One line change when compared to v1:
Added the new parameter to the "usage" help section.
Attachment #312358 - Attachment is obsolete: true
Attachment #312366 - Flags: review?(nelson)
Attachment #312358 - Flags: review?(nelson)
Comment on attachment 312366 [details] [diff] [review]
Patch v2

This patch is expedient for the immediate testing issue, 
and I think it does what it's trying to do correctly.

But I'm sure we don't want to support this defintion of the -s option 
for the long term.  And since, unlike PSM, we must support new features
forever, we don't want to add a new command line option that will quickly
be found to be insufficient.

There are now 10 flags for each of CRL and OCSP revocation.
This implementation of the -s option sets one particular combination
out of the 1024 possible combinations for those flags, and offers no
way for the user to specify another combination.  Our testing will 
require that we have a way to specify other combinations.

So, I suggest that we define the -s option as taking an argument by 
which the user specifies the desired combination.

Here are two suggestions for the definitions of that argument string:

a) That argument will be a string of letters, each corresponding to one of 
the 10 bits now defined.  An example might be -s abfg

b) That argument will be a hexadecimal value expressing the flags directly
by their defined bit values, e.g. -s 2c  

Perhaps we can define it so that if the argument string is missing,, some
default value will be used.
Attachment #312366 - Flags: review?(nelson) → review-
(Assignee)

Comment 4

10 years ago
I think specifying bit combinations is difficult to implement, if at the same time you wish to be flexible wrt future added bits.

I'd like to proposed an approach that is a mix of your and my approach.

Let's use "named" configurations.
The existing vfychain uses "allow-crl" and could be the default.

 -s allow-crl

The combination I've implemented is "allow-crl-and-ocsp".

 -s allow-crl-and-ocsp

This is easy to implement, and will allow us to define whatever is necessary is the future. If we ever require to implement something based on bits, we can define that a certain prefix is used for refering to bit combinations. For example, we could define that the prefix "01:" means: What follows is a bitfield.

 -s 01:whatever


Here is my offering: I have time to implement the very simple proposal I make above, using strings that map to our current 2 options.

I'm afraid I do not have time to work on the bitfield stuff.
> I think specifying bit combinations is difficult to implement, if at 
> the same time you wish to be flexible wrt future added bits.

I agree completely, which is why I do NOT want to name combinations.
We'd need 1024 names! 

Rather, I proposed that we name bits, that is, provide a way to specify 
the values of the individual bits separately.  Using hex is an obvious way 
to do it, but not very mnemonic.  Assigning a single letter name to each 
bit _might_ be more mnemonic, provided that you can find 10 separate letters
that meaningfully identify the 10 existing bits.
(Assignee)

Comment 6

10 years ago
(In reply to comment #5)
> I agree completely, which is why I do NOT want to name combinations.
> We'd need 1024 names! 

Nobody says we'd have to implement them all.


> Rather, I proposed that we name bits, that is, provide a way to specify 
> the values of the individual bits separately.  Using hex is an obvious way 
> to do it, but not very mnemonic.  Assigning a single letter name to each 
> bit _might_ be more mnemonic, provided that you can find 10 separate letters
> that meaningfully identify the 10 existing bits.

I wouldn't do it with single letters.
I would do it with strings.
I would require that "-s bitname" is specified once for each bit you want to set or not set. Probably with prefixes to specify which group you mean, like global, leaf, chain, etc.

But again, I don't have the time to work on it and get it right.

I ask that we use something as a resonable default.
IMHO my proposal to allow both named-completed-configurations (or call it policy), and in the future, add support for full bitfields, is a salomoic proposal.

I've implemented it, and will attach a new patch.
I will address review requests about correctness of that patch.

But sorry, I'm too swamped to work on the bit fields.
(Assignee)

Comment 7

10 years ago
Created attachment 312391 [details] [diff] [review]
(wrong patch, incomparable to previous one)
(Assignee)

Updated

10 years ago
Attachment #312391 - Flags: review?(nelson)
(Assignee)

Comment 8

10 years ago
Comment on attachment 312391 [details] [diff] [review]
(wrong patch, incomparable to previous one)

sorry, too much unrelated stuff in that patch
Attachment #312391 - Attachment is obsolete: true
Attachment #312391 - Flags: review?(nelson)
(Assignee)

Comment 9

10 years ago
Created attachment 312392 [details] [diff] [review]
Patch v4
Attachment #312366 - Attachment is obsolete: true
Attachment #312392 - Flags: review?(nelson)
(Assignee)

Updated

10 years ago
Alias: vfychain-ocsp
(Assignee)

Comment 10

10 years ago
Also, I would like to remind you that we had categorized vfychain as one of our unsupported tools. This is one more argument that this patch is acceptable. It helps, and it was little work. So let's take it.
(In reply to comment #10)
> Also, I would like to remind you that we had categorized vfychain as one 
> of our unsupported tools.

Thanks for that reminder.  Do we have any public-facing web page that 
documents which commands are considered supported and which are not?
(Assignee)

Comment 12

10 years ago
http://wiki.mozilla.org/NSS:ToolsToShip is the page we used a while ago.

Based on that page is the categorization we use for the tools that we currently ship in Fedora.

/usr/bin/certutil
/usr/bin/cmsutil
/usr/bin/crlutil
/usr/bin/modutil
/usr/bin/pk12util
/usr/bin/signtool
/usr/bin/signver
/usr/bin/ssltap
/usr/lib/nss/unsupported-tools/atob
/usr/lib/nss/unsupported-tools/btoa
/usr/lib/nss/unsupported-tools/derdump
/usr/lib/nss/unsupported-tools/ocspclnt
/usr/lib/nss/unsupported-tools/pp
/usr/lib/nss/unsupported-tools/selfserv
/usr/lib/nss/unsupported-tools/strsclnt
/usr/lib/nss/unsupported-tools/symkeyutil
/usr/lib/nss/unsupported-tools/tstclnt
/usr/lib/nss/unsupported-tools/vfychain
/usr/lib/nss/unsupported-tools/vfyserv
(Assignee)

Comment 13

10 years ago
But I guess my arguments were not able to convince you to accept the patch as a reasonable intermediate solution.
Attachment #312391 - Attachment description: Patch v3 → (wrong patch, incomparable to previous one)
Comment on attachment 312392 [details] [diff] [review]
Patch v4

Sorry, Kai, this just fell off my radar.
In today's conference call, we agreed that 
a) this command is not a "supported" one, and therefore we are under no obligation to maintain backwards compatibility in the command line syntax, and 
b) option arguments that name preselected combinations of revocation flags are not mutually exclusive with option arguments that specify individual bits, so the new command line option could eventually accept arguments of either style.  Some sort of argument prefix might be used to signify that the argument string is a bit-by-bit representation rather than the name of a combination of revocation flags.

So, I'm approving this patch for 3.12 on that basis.
I *think* that the behavior of this program, in the absence of the new -s command line option, is the same as before, and that's really the only requirement for this patch.
Attachment #312392 - Flags: review?(nelson) → review+
(Assignee)

Comment 15

10 years ago
(In reply to comment #14)
> So, I'm approving this patch for 3.12 on that basis.

Nelson, thanks a lot!

> I *think* that the behavior of this program, in the absence of the new -s
> command line option, is the same as before, and that's really the only
> requirement for this patch.  

Correct.
(Assignee)

Comment 16

10 years ago
checked in, marking fixed.

Checking in vfychain.c;
/cvsroot/mozilla/security/nss/cmd/vfychain/vfychain.c,v  <--  vfychain.c
new revision: 1.20; previous revision: 1.19
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee: nobody → kaie
Severity: normal → enhancement
Status: REOPENED → NEW
Priority: -- → P2
Target Milestone: --- → 3.12
Status: NEW → RESOLVED
Last Resolved: 10 years ago10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.