481764 - PSM must call CERT_PKIXSetDefaults to define default verification flags

Status: RESOLVED INVALID

(Core :: Security: PSM, defect)

Description

[Kai Engert (:KaiE:)]

PSM must call CERT_PKIXSetDefaults to define its desired default verification flags

Comment 1

[Kai Engert (:KaiE:)]

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

Comment 2

[Kai Engert (:KaiE:)]

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;

Comment 3

[Kai Engert (:KaiE:)]

Please review the comments.

Comment 4

[Wan-Teh Chang]

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

[Andrew Prout]

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.

Comment 6

[Kai Engert (:KaiE:)]

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?

Comment 7

[Kai Engert (:KaiE:)]

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

[Wan-Teh Chang]

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.

Comment 9

[Kai Engert (:KaiE:)]

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

Comment 10

[Kai Engert (:KaiE:)]

Attachment: Patch v1

Comment 11

[Kai Engert (:KaiE:)]

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.

Comment 12

[Kai Engert (:KaiE:)]

reassign bug owner.
mass-update-kaie-20120918

Comment 13

[:Cykesiopka]

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.

Comment 14

[Wan-Teh Chang]

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.