Skip to content

Commit

Permalink
Improve build and fix X509 test failures for Ruby (#1887)
Browse files Browse the repository at this point in the history
1. `bignum_to_string` is called when trying to print out the X509
Extension value. Initially I thought the value wasn't being set
correctly, but it turns out that we were printing the value in hex form,
rather than decimal (which OpenSSL does and Ruby expects). AWS-LC prints
the hex value if the value is more than 32 bits, while OpenSSL has a
much more lax restriction at 128 bits. Tweaking this to align with
OpenSSL gets past the test for `test_x509crl.rb`. Tweaking the value to
align with OpenSSL gets past the test.

2. Great news is I don't think we need to do anything for the test
failure in `test_x509req`. This was a testing gap on Ruby's end,
documented in this commit: ruby/ruby@6b12013.
Only version 1 is available for CSRs and Ruby attempts to set an invalid
version in its tests. OpenSSL 3.3 disallows this behavior now and Ruby
has removed the test in it's mainline branch. We can brush up the patch
to account for this and skip the test with AWS-LC.

3. I also took the chance to add back some of the defines Ruby depends
on as no-ops. The X509 defines aren't actually used in neither OpenSSL
or AWS-LC as found by this commit: 496838a.

### Call-outs:
N/A

### Testing:
N/A

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license and the ISC license.
  • Loading branch information
samuel40791765 authored Oct 2, 2024
1 parent 751fe2a commit 0c846a7
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 3 deletions.
3 changes: 2 additions & 1 deletion crypto/x509/v3_utl.c
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
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",
"0x0123456789012345678901234567890123",
"-0x0123456789012345678901234567890123",
};
for (const char *in : kTests) {
SCOPED_TRACE(in);
Expand Down
7 changes: 7 additions & 0 deletions include/openssl/ocsp.h
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|.
#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
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

0 comments on commit 0c846a7

Please sign in to comment.