Skip to content

Conversation

@lealem47
Copy link
Contributor

Description

Skip all time checking without having to define NO_ASN_TIME. Originally developed as workaround for the issue addressed by #6181, but still a nice feature to have.

Fixes zd#15790

Testing

Tested with certs in certs/test/expired/

Checklist

  • added tests
  • updated/added doxygen
  • updated appropriate READMEs
  • Updated manual and documentation

@JacobBarthelmeh
Copy link
Contributor

We have the VERIFY_SKIP_DATE option, could that be expanded to OCSP checks, allowing us to avoid adding another macro?

@lealem47
Copy link
Contributor Author

We have the VERIFY_SKIP_DATE option, could that be expanded to OCSP checks, allowing us to avoid adding another macro?

That looks like it's going to involve duplicating wolfSSL_CertManagerLoadCA() and wolfSSL_CertManagerVerifyBuffer() to _ex() versions , only with the an additional flag parameter to pass in WOLFSSL_LOAD_FLAG_DATE_ERR_OKAY. Would that be a better change? @JacobBarthelmeh

Copy link
Contributor

@dgarske dgarske left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the PR. Please add some additional comments in asn.c for NO_ASN_TIME_CHECK to describe the other options for accomplishing this. I'd prefer they use one of the existing supported methods. Also indicate the possible security risk using it.
Over to @SparkiDev for additional feedback.

@dgarske dgarske requested a review from SparkiDev April 20, 2023 20:13
dgarske
dgarske previously approved these changes Apr 20, 2023
@lealem47 lealem47 assigned SparkiDev and unassigned lealem47 Apr 20, 2023
ret = PaseCRL_CheckSignature(dcrl, buff, cm);
}

(void)verify;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already this line above WOLFSSL_MSG.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching that. I opened the PR before that was added.

@SparkiDev SparkiDev merged commit 9230d9c into wolfSSL:master Apr 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants