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.

Sign up for GitHub

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

Jump to bottom

Improve build and fix X509 test failures for Ruby #1887

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Improve build and fix X509 test failures for Ruby #1887

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0fdf13d
align bignum_to_string bit limitation with OpenSSL
samuel40791765 Sep 27, 2024
3e9f334
no-op defines for Ruby
samuel40791765 Sep 27, 2024
a4b97fd
use 33 bit number for testing
samuel40791765 Sep 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion crypto/x509/v3_utl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,8 @@ static char *bignum_to_string(const BIGNUM *bn) {
// Display large numbers in hex and small numbers in decimal. Converting to
// decimal takes quadratic time and is no more useful than hex for large
// numbers.
if (BN_num_bits(bn) < 32) {
// The threshold for large numbers is set at 128 bits to align with OpenSSL.
if (BN_num_bits(bn) < 128) {
return BN_bn2dec(bn);
}

Expand Down
6 changes: 4 additions & 2 deletions crypto/x509/x509_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3190,9 +3190,11 @@ TEST(X509Test, PrettyPrintIntegers) {
"-42",
"256",
"-256",
"4886718345",
"-4886718345",
// Large numbers are pretty-printed in hex to avoid taking quadratic time.
"0x0123456789",
"-0x0123456789",
"0x0123456789012345678901234567890123456789",
"-0x0123456789012345678901234567890123456789",
Copy link
Contributor

Choose a reason for hiding this comment

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

minor nit: you should only need 32 hex digits to hit your threshold of 2^128

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, I just wanted the numbers to align. Will change

};
for (const char *in : kTests) {
SCOPED_TRACE(in);
Expand Down
7 changes: 7 additions & 0 deletions include/openssl/ocsp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ extern "C" {
// aACompromise (10) }
//
// Reason Code RFC: https://www.rfc-editor.org/rfc/rfc5280#section-5.3.1
//
// Note: OCSP_REVOKED_STATUS_NOSTATUS is defined by OpenSSL and is not defined
// within the RFC.
#define OCSP_REVOKED_STATUS_NOSTATUS -1
#define OCSP_REVOKED_STATUS_UNSPECIFIED 0
#define OCSP_REVOKED_STATUS_KEYCOMPROMISE 1
#define OCSP_REVOKED_STATUS_CACOMPROMISE 2
Expand All @@ -58,6 +62,9 @@ extern "C" {
// Certificates included within |bs| or |req| will be included in the
// search for the signing certificate by default, unless |OCSP_NOINTERN| is set.
#define OCSP_NOINTERN 0x2
// OCSP_NOSIGS does nothing. In OpenSSL, this skips signature verification in
// |OCSP_basic_verify| and |OCSP_request_verify|.
Comment on lines +65 to +66
Copy link
Contributor

Choose a reason for hiding this comment

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

so we'll always check the signatures, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the intention was to not do this until it's specifically asked for. There's not really a scenario where the signature shouldn't be verified.

#define OCSP_NOSIGS
// OCSP_NOCHAIN is for |OCSP_basic_verify| and |OCSP_request_verify|.
// For |OCSP_basic_verify|, certificates in both |certs| and in |bs| are
// considered as certificates for the construction of the validation path for
Expand Down
4 changes: 4 additions & 0 deletions include/openssl/x509.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2906,6 +2906,10 @@ OPENSSL_EXPORT int X509_VERIFY_PARAM_set_purpose(X509_VERIFY_PARAM *param,
// X509_TRUST_OBJECT_SIGN evaluates trust with the |NID_code_sign| OID, for
// validating code signing certificates.
#define X509_TRUST_OBJECT_SIGN 5
// X509_TRUST_OCSP_SIGN does nothing. It's unused in OpenSSL and AWS-LC.
#define X509_TRUST_OCSP_SIGN 6
// X509_TRUST_OCSP_REQUEST does nothing. It's unused in OpenSSL and AWS-LC.
#define X509_TRUST_OCSP_REQUEST 7
// X509_TRUST_TSA evaluates trust with the |NID_time_stamp| OID, for validating
// Time Stamping Authority (RFC 3161) certificates.
#define X509_TRUST_TSA 8
Expand Down
Loading