425801 - (vfychain-ocsp) vfychain: Add support for OCSP checking

Status: RESOLVED FIXED

(NSS :: Tools, enhancement, P2)

Description

[Kai Engert (:KaiE:)]

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

Comment 1

[Kai Engert (:KaiE:)]

Attachment: Patch v1

Comment 2

[Kai Engert (:KaiE:)]

Attachment: Patch v2

One line change when compared to v1:
Added the new parameter to the "usage" help section.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

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.

Comment 4

[Kai Engert (:KaiE:)]

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.

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

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

Comment 6

[Kai Engert (:KaiE:)]

(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.

Comment 7

[Kai Engert (:KaiE:)]

Attachment: (wrong patch, incomparable to previous one)

Comment 8

[Kai Engert (:KaiE:)]

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

sorry, too much unrelated stuff in that patch

Comment 9

[Kai Engert (:KaiE:)]

Attachment: Patch v4

Comment 10

[Kai Engert (:KaiE:)]

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.

Comment 11

[Nelson Bolyard (seldom reads bugmail)]

(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?

Comment 12

[Kai Engert (:KaiE:)]

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

Comment 13

[Kai Engert (:KaiE:)]

But I guess my arguments were not able to convince you to accept the patch as a reasonable intermediate solution.

Comment 14

[Nelson Bolyard (seldom reads bugmail)]

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.

Comment 15

[Kai Engert (:KaiE:)]

(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.

Comment 16

[Kai Engert (:KaiE:)]

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