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

Update ssl_engine_ocsp.c #501

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from
Open

Conversation

SirTeggun
Copy link

I have forced the addition of Nonce in OCSP requests to prevent replay attacks, ensuring that each OCSP request is unique, regardless of server configurations. This change increases security when checking certificate status.

I have forced the addition of Nonce in OCSP requests to prevent replay attacks, ensuring that each OCSP request is unique, regardless of server configurations. This change increases security when checking certificate status.
return NULL;
}

OCSP_request_add1_nonce(req, 0, -1);
Copy link
Member

Choose a reason for hiding this comment

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

Why ignore the opt-out config of SSLOCSPUseRequestNonce?

Copy link
Author

Choose a reason for hiding this comment

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

The decision to enforce the inclusion of the nonce in OCSP requests, regardless of the SSLOCSPUseRequestNonce configuration, was made to enhance security. Nonce inclusion helps mitigate replay attacks, where a malicious actor might reuse a previously valid OCSP response to falsely indicate the validity of a revoked certificate.
By ensuring the nonce is always added, the integrity and freshness of the OCSP responses can be guaranteed. While this approach overrides the opt-out feature, it prioritizes preventing potential vulnerabilities.

return req;
}

static int verify_ocsp_status(X509 *cert, X509_STORE_CTX *ctx, conn_rec *c,
Copy link
Member

Choose a reason for hiding this comment

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

This function looks duplicated.

Copy link
Author

Choose a reason for hiding this comment

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

The create_request function is not duplicated. Returns req which is used later in verify_ocsp_status. The separation of the two functions is aimed at maintaining a clear division of responsibilities: create_request is responsible for generating the OCSP request, while verify_ocsp_status is responsible for verifying its status.

Copy link
Member

Choose a reason for hiding this comment

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

I am referring to verify_ocsp_status(). The commit seems to add a second copy.

Copy link
Author

Choose a reason for hiding this comment

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

I checked the code again and found no errors. Tell me if you don't understand something and I'll try to explain better.

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.

2 participants