Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Duplicate Time Validator #106

Closed
zzzsz opened this issue Sep 4, 2024 · 1 comment · Fixed by #112
Closed

Duplicate Time Validator #106

zzzsz opened this issue Sep 4, 2024 · 1 comment · Fixed by #112
Labels
bug Something isn't working

Comments

@zzzsz
Copy link

zzzsz commented Sep 4, 2024

The function of time.TimeCorrectEncodingValidator() in create_validity_validator_container() is equal to time.UtcTimeCorrectSyntaxValidator() plus time.GeneralizedTimeCorrectSyntaxValidator().
And in the end, these three are all in doc_validator of crl.

def create_validity_validator_container(additional_validators=None):
    if additional_validators is None:
        additional_validators = []
    return validation.ValidatorContainer(
        validators=[
                       crl_validity.CrlSaneValidityPeriodValidator(),
                       time.TimeCorrectEncodingValidator(),
                   ] + additional_validators,
        path='certificateList.tbsCertList'
    )
@CBonnell CBonnell added the bug Something isn't working label Sep 4, 2024
@CBonnell
Copy link
Collaborator

CBonnell commented Sep 4, 2024

Thanks, I think the ValidatorContainer's path should be scoped to certificateList.tbsCertList.thisUpdate and certificateList.tbsCertList.nextUpdate to ensure that GeneralizedTime values appearing CRL extensions are not incorrectly flagged if they contain a year between 1950 and 2049.

Also consider having the TimeCorrectEncodingValidator merely return upon encountering an invalid syntax value instead of raising a finding that will be duplicated by the Syntax validators.

@CBonnell CBonnell linked a pull request Sep 27, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants