search

aws / aws-lc

AWS-LC is a general-purpose cryptographic library maintained by the AWS Cryptography team for AWS and their customers. It іs based on code from the Google BoringSSL project and the OpenSSL project.
Other
258 stars 105 forks source link

Add support for NETSCAPE_SPKI_print #1624

Closed samuel40791765 closed 3 weeks ago

samuel40791765 commented 1 month ago

Issues:

Resolves CryptoAlg-1717

Description of changes:

Ruby consumes NETSCAPE_SPKI_print for debugging purposes. This adds support for the symbol for easier integration.

Call-outs:

N/A

Testing:

Expected output test

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.

codecov-commenter commented 1 month ago

Codecov Report

Attention: Patch coverage is 91.66667% with 3 lines in your changes missing coverage. Please review.

Project coverage is 78.16%. Comparing base (37ba0e2) to head (d3bcc10).

Files Patch % Lines
crypto/x509/x509spki.c 89.65% 3 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1624 +/- ## ======================================= Coverage 78.15% 78.16% ======================================= Files 562 562 Lines 94638 94675 +37 Branches 13573 13576 +3 ======================================= + Hits 73969 74006 +37 Misses 20075 20075 Partials 594 594 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

samuel40791765 commented 1 month ago

can we get away with just defining no-op'ing this?

We could consider it, but it did seem harmless to incorporate given that we have support for all the surrounding functions within the module: https://github.com/ruby/ruby/blob/ruby_2_7/ext/openssl/ossl_ns_spki.c#L378-L405

Great question on potential impact, I did some more digging. The only concern would be us failing this Ruby test which asserts that the inputted BIO has at least some contents, but we can pass a short string into BIO to prevent that from happening.

My preference would be supporting the entire module with no caveats, but I'm open to any other concerns with us supporting this.