Skip to content

Comments

Add missing locks in X509_check_issued#6161

Closed
richsalz wants to merge 1 commit intoopenssl:masterfrom
richsalz:gh6121
Closed

Add missing locks in X509_check_issued#6161
richsalz wants to merge 1 commit intoopenssl:masterfrom
richsalz:gh6121

Conversation

@richsalz
Copy link
Contributor

@richsalz richsalz commented May 2, 2018

Thanks to Mingtao Yang for reporting this bug.
Fixes #6121.
Not going to backport if it doesn't cherry-pick cleanly.

Thanks to Mingtao Yang for reporting this bug.
@richsalz richsalz added branch: master Applies to master branch branch: 1.0.2 Applies to OpenSSL_1_0_2-stable branch (EOL) 1.1.0 approval: review pending This pull request needs review by a committer labels May 2, 2018
@richsalz richsalz self-assigned this May 2, 2018
CRYPTO_THREAD_unlock(issuer->lock);
CRYPTO_THREAD_write_lock(subject->lock);
x509v3_cache_extensions(subject);
CRYPTO_THREAD_unlock(subject->lock);
Copy link
Contributor

Choose a reason for hiding this comment

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

If one looks at the other locations where x509v3_cache_extensions() is called, one sees that the EXFLAG_SET flag is used to ensure that the values are cached only once. So either the guards should be added here, too

    if (!(issuer->ex_flags & EXFLAG_SET)) {
        CRYPTO_THREAD_write_lock(issuer->lock);
        x509v3_cache_extensions(issuer);
        CRYPTO_THREAD_unlock(issuer->lock);
    }
    if (!(subject->ex_flags & EXFLAG_SET)) {
        CRYPTO_THREAD_write_lock(subject->lock);
        x509v3_cache_extensions(subject);
        CRYPTO_THREAD_write_lock(subject->lock);
    }

or even better the check could be moved entirely into x509v3_cache_extensions(). See upcoming pull request...

@mspncp
Copy link
Contributor

mspncp commented May 2, 2018

See #6162.

@richsalz
Copy link
Contributor Author

richsalz commented May 3, 2018

Closing this in favor of #6162

@richsalz richsalz closed this May 3, 2018
@richsalz richsalz deleted the gh6121 branch May 5, 2018 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approval: review pending This pull request needs review by a committer branch: master Applies to master branch branch: 1.0.2 Applies to OpenSSL_1_0_2-stable branch (EOL)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants