PSM must call CERT_PKIXSetDefaults to define default verification flags

RESOLVED INVALID

Status

()

Core
Security: PSM
RESOLVED INVALID
9 years ago
3 years ago

People

(Reporter: kaie, Unassigned)

Tracking

Trunk
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

9 years ago
PSM must call CERT_PKIXSetDefaults to define its desired default verification flags
(Reporter)

Comment 1

9 years ago
Prior to typing a patch, let's simply list the params and the values we want.
I'm copying from the struct definition which includes the parameter comments, and I'm writing my proposed values inline:

   cert_pi_nbioContext     = 1, /* specify a non-blocking IO context used to
			         * resume a session. If this argument is 
				 * specified, no other arguments should be.
				 * Specified in value.pointer.p. If the 
				 * operation completes the context will be 
				 * freed. */

omit, no default



   cert_pi_nbioAbort       = 2, /* specify a non-blocking IO context for an 
				 * existing operation which the caller wants
			         * to abort. If this argument is 
				 * specified, no other arguments should be.
				 * Specified in value.pointer.p. If the 
			         * operation succeeds the context will be 
				 * freed. */

omit, no default



   cert_pi_certList        = 3, /* specify the chain to validate against. If
				 * this value is given, then the path 
				 * construction step in the validation is 
				 * skipped. Specified in value.pointer.chain */

omit, no default



   cert_pi_policyOID       = 4, /* validate certificate for policy OID.
				 * Specified in value.array.oids. Cert must
				 * be good for at least one OID in order
				 * to validate. Default is no policyOID */

omit, no default



   cert_pi_policyFlags     = 5, /* flags for each policy specified in policyOID.
				 * Specified in value.scalar.ul. Policy flags
				 * apply to all specified oids. 
				 * Use CERT_POLICY_FLAG_* macros below. If not
				 * specified policy flags default to 0 */

omit, no default



   cert_pi_keyusage        = 6, /* specify what the keyusages the certificate 
				 * will be evaluated against, specified in
				 * value.scalar.ui. The cert must validate for
				 * at least one of the specified key usages.
				 * Values match the KU_  bit flags defined
				 * in this file. Default is derived from
				 * the 'usages' function argument */

omit, no default



   cert_pi_extendedKeyusage= 7, /* specify what the required extended key 
				 * usage of the certificate. Specified as
				 * an array of oidTags in value.array.oids.
				 * The cert must validate for at least one
				 * of the specified extended key usages.
				 * If not specified, no extended key usages
				 * will be checked. */

omit, no default



   cert_pi_date            = 8, /* validate certificate is valid as of date 
				 * specified in value.scalar.time. A special 
				 * value '0' indicates 'now'. default is '0' */

omit, no default



   cert_pi_revocationFlags = 9, /* Specify what revocation checking to do.
				 * See CERT_REV_FLAG_* macros below
				 * Set in value.pointer.revocation */

For ease of reading I will add the proposed default for revocation checking in a separate comment.
This tells me, each time an application user configures the application to use different OCSP settings (enabled/disabled, network failure = revoked), PSM must call the function to update the defaults.



   cert_pi_certStores      = 10,/* Bitmask of Cert Store flags (see below)
				 * Set in value.scalar.ui */

Possible values are HTTP and LDAP, but I don't know what should be specified here.
What kind of cert stores is this refering to?

For the PSM default we probably want "zero" = don't use remote cert stores?
Please advice.


   cert_pi_trustAnchors    = 11,/* Specify the list of trusted roots to 
				 * validate against. If the list in NULL all
				 * default trusted roots are used.
				 * Specified in value.pointer.chain */

omit, no default



   cert_pi_useAIACertFetch = 12, /* Enables cert fetching using AIA extension.

set of OFF, no fetching
(Reporter)

Comment 2

9 years ago
This enables:
- both CRL and OCSP checking
- on both the leaf and the chain
- currently leaf and chain use the same flags
  (but I have exploded the structs for both independently, so we can easily
   add comments inline for leaf/chain separately)



PSM offers the preference
  if OCSP network fails, treat as invalid

This is currently an OCSP-specific UI preference, but I propose to redefine its meaning into a general "must have fresh revocation info available" (regardless whether it's OCSP or CRL).

I propose this preference should toggle 
  CERT_REV_MI_NO_OVERALL_INFO_REQUIREMENT
and
  CERT_REV_MI_REQUIRE_SOME_FRESH_INFO_AVAILABLE
for both leaf and chain.


  CERTRevocationMethodIndex preferedRevMethods[1] = { 
    cert_revocation_method_ocsp
  };

  PRUint64 revLeafMethodFlags = 
    CERT_REV_M_TEST_USING_THIS_METHOD
    | CERT_REV_M_ALLOW_NETWORK_FETCHING
    | CERT_REV_M_ALLOW_IMPLICIT_DEFAULT_SOURCE
    | CERT_REV_M_SKIP_TEST_ON_MISSING_SOURCE
    | CERT_REV_M_STOP_TESTING_ON_FRESH_INFO;

  PRUint64 revChainMethodFlags = 
    CERT_REV_M_TEST_USING_THIS_METHOD
    | CERT_REV_M_ALLOW_NETWORK_FETCHING
    | CERT_REV_M_ALLOW_IMPLICIT_DEFAULT_SOURCE
    | CERT_REV_M_SKIP_TEST_ON_MISSING_SOURCE
    | CERT_REV_M_STOP_TESTING_ON_FRESH_INFO;

  PRUint64 revMethodIndependentFlags = 
    CERT_REV_MI_TEST_ALL_LOCAL_INFORMATION_FIRST
    | CERT_REV_MI_NO_OVERALL_INFO_REQUIREMENT
    ;

  PRUint64 revChainMethodIndependentFlags = 
    CERT_REV_MI_TEST_ALL_LOCAL_INFORMATION_FIRST
    | CERT_REV_MI_NO_OVERALL_INFO_REQUIREMENT
    ;

  PRUint64 methodFlags[2];
  methodFlags[cert_revocation_method_crl] = revLeafMethodFlags;
  methodFlags[cert_revocation_method_ocsp] = revLeafMethodFlags;

  PRUint64 chain_methodFlags[2];
  methodFlags[cert_revocation_method_crl] = revChainMethodFlags;
  methodFlags[cert_revocation_method_ocsp] = revChainMethodFlags;

  CERTRevocationFlags rev;

  rev.leafTests.number_of_defined_methods = cert_revocation_method_ocsp +1;
  rev.leafTests.cert_rev_flags_per_method = methodFlags;
  rev.leafTests.number_of_preferred_methods = 1;
  rev.leafTests.preferred_methods = preferedRevMethods;
  rev.leafTests.cert_rev_method_independent_flags =
    revMethodIndependentFlags;

  rev.chainTests.number_of_defined_methods = cert_revocation_method_ocsp +1;
  rev.chainTests.cert_rev_flags_per_method = chain_methodFlags;
  rev.chainTests.number_of_preferred_methods = 1;
  rev.chainTests.preferred_methods = preferedRevMethods;
  rev.chainTests.cert_rev_method_independent_flags =
    revChainMethodIndependentFlags;
(Reporter)

Comment 3

9 years ago
Please review the comments.

Comment 4

9 years ago
We need a more user-friendly API for libpkix.  Even I can't
figure out how to use libpkix.  This is why in bug 479393
I ended up only calling CERT_SetUsePKIXForValidation.
(Originally I had a more ambitious goal: replace
CERT_VerifyCert by CERT_PKIXVerifyCert.)

Comment 5

9 years ago
Please consider setting cert_pi_useAIACertFetch to ON, or at least exposing the setting for configuration. This would address the longstanding bug 399324 which hounds anyone who uses PKI in a bridged configuration.

For this to work correctly, it would likely require setting a value for cert_pi_certStores as well, although looking at the NSS source this preference doesn't appear to be used / honored anywhere.
(Reporter)

Comment 6

8 years ago
Nobody has reviewed yet my prior comments, so let me start by asking a simpler question:

If PSM decides to call ONLY 
  CERT_SetUsePKIXForValidation ( PR_TRUE )

but PSM does NOT make any calls to CERT_PKIXSetDefaults

will (question 1) libPKIX use verification settings, that are close to the behaviour of the classic NSS verification functions?


And will (question 2) classic NSS API function
  CERT_SetOCSPFailureMode
also change the behaviour of forwarded
  classic NSS verify -> UsePKIXForValidation -> libPKIX verify
functions?


I assume the answer to both questions is "yes".

If it's really "yes", then we could conclude, this bug doesn't block bug 479393.

Opinions?
(Reporter)

Comment 7

7 years ago
I think it is very important to work on this function,

especially to make sure we use the correct default settings regarding revocation checking.

In comment 1, please read sections

  cert_pi_revocationFlags 

  cert_pi_certStores

  cert_pi_useAIACertFetch


In comment 2, please read my explanations.

Do you have any thoughts?

Comment 8

7 years ago
Kai: I agree with the defaults you proposed.  Note that cert_pi_certStores
is not implemented.

I didn't review the code in comment 2 that sets up the proposed default
for cert_pi_revocationFlags.  I will review the code when you write a
patch.
(Reporter)

Comment 9

7 years ago
(In reply to comment #8)
> I didn't review the code in comment 2 that sets up the proposed default
> for cert_pi_revocationFlags.  I will review the code when you write a
> patch.


I already wrote the patch. It should be complete, except, that I don't yet change the defaults it the users toggles the ocsp.require preference.

Since you proposed in bug 481763 that this whole function may be unnecessary, you probably don't want to review this until we have made the general decision.
(Reporter)

Comment 10

7 years ago
Created attachment 521998 [details] [diff] [review]
Patch v1
(Reporter)

Comment 11

7 years ago
Since we're currently not planning to do bug 481763, we obviously won't be able to do this one either.

However, some of the code I've implemented here will be reused in bug 479393.
No longer blocks: 479393
(Reporter)

Comment 12

6 years ago
reassign bug owner.
mass-update-kaie-20120918
Assignee: kaie → nobody

Comment 13

3 years ago
Firefox now exclusively uses mozilla::pkix, so it doesn't seem like this is necessary anymore. In any case, Bug 481763 still isn't fixed, so there's no CERT_PKIXSetDefaults to call anyways.
=> Resolving as "invalid" in the sense that there's nothing to fix here.

If I've misunderstood, feel free to re-open.
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → INVALID

Comment 14

3 years ago
Cykesiopka: thank you for closing this bug. "WONTFIX" may be
a better resolution because we won't do what was requested.
But "INVALID" is also fine.
You need to log in before you can comment on or make changes to this bug.