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

Adds OCSP support for BoringSSL #7298

Merged
merged 1 commit into from
Jul 20, 2021
Merged

Conversation

randall
Copy link
Contributor

@randall randall commented Oct 27, 2020

No description provided.

@randall randall added the SSL label Oct 27, 2020
@randall randall added this to the 10.0.0 milestone Oct 27, 2020
@randall randall self-assigned this Oct 27, 2020
@maskit
Copy link
Member

maskit commented Oct 27, 2020

Two comments for now.

Maintenance - Having a snapshot of OpenSSL source code sounds a bit scary because it could bring security issues. I think we should have a very easy way to update the snapshot instantly. If there are modifications against the original code we should probably keep them as patch files as well.

License - OpenSSL 3.0 and the code on their master branch are licensed under AL2. Although I didn't check whether the current license is compatible with AL2, AL2 version would be safer option in terms of licensing. The code on master may be unstable so I'm also ok with using non-AL2 version for now as long as the license is compatible.
https://www.openssl.org/source/license.html

@randall
Copy link
Contributor Author

randall commented Oct 27, 2020

License - OpenSSL 3.0 and the code on their master branch are licensed under AL2. Although I didn't check whether the current license is compatible with AL2, AL2 version would be safer option in terms of licensing. The code on master may be unstable so I'm also ok with using non-AL2 version for now as long as the license is compatible.
https://www.openssl.org/source/license.html

The APIs/underlying structures have changed enough to make this not a trivial thing to do. This is not the first openssl code to be added to the project. It looks like parts of BIO_fastopen.cc is copied from the project into our tree.

commit d02e88eb0db871a14279da28af819b5f19a945ac
Author: Leif Hedstrom <leif@ogre.com>
Date:   Fri Feb 17 21:54:45 2017 -0700

    Added the OpenSSL license

@randall
Copy link
Contributor Author

randall commented Oct 27, 2020

Maintenance - Having a snapshot of OpenSSL source code sounds a bit scary because it could bring security issues. I think we should have a very easy way to update the snapshot instantly. If there are modifications against the original code we should probably keep them as patch files as well.

Personally, I'd like the library "ocsp4boring" to not be checked into the tree itself and rather that it was its own project that other projects could easily build against (like nginx).

@shinrich
Copy link
Member

I agree that keeping ocsp4boring out of our source tree seems preferable. And rather user it as a separately installed package.

@randall
Copy link
Contributor Author

randall commented Nov 2, 2020

Seems reasonable to move the ocsp4boring code out of tree. I'll follow up with a change to do this.

@maskit
Copy link
Member

maskit commented Jan 26, 2021

Some of functions that this PR adds are going to be deprecated on OpenSSL 3.0. ATS uses three of those, and will need to use some replacements.
https://github.com/openssl/openssl/blob/master/doc/man3/OCSP_sendreq_new.pod

Deprecated since OpenSSL 3.0, can be hidden entirely by defining OPENSSL_API_COMPAT with a suitable version value, see openssl_user_macros(7):

int OCSP_REQ_CTX_i2d(OCSP_REQ_CT *rctx, const ASN1_ITEM *it, ASN1_VALUE *req);
int OCSP_REQ_CTX_add1_header(OCSP_REQ_CT *rctx,
                             const char *name, const char *value);
void OCSP_REQ_CTX_free(OSSL_HTTP_REQ_CTX *rctx);
void OCSP_set_max_response_length(OCSP_REQ_CT *rctx,
                                  unsigned long len);
int OCSP_REQ_CTX_set1_req(OSSL_HTTP_REQ_CTX *rctx, const OCSP_REQUEST *req);

@randall
Copy link
Contributor Author

randall commented Jun 25, 2021

Randall is still working on this.

@randall randall removed the Stale label Jun 25, 2021
@randall randall force-pushed the ocsp_boringssl branch 2 times, most recently from fada783 to cce9d49 Compare July 5, 2021 23:04
@apache apache deleted a comment from github-actions bot Jul 5, 2021
@apache apache deleted a comment from bryancall Jul 5, 2021
@randall randall requested a review from shinrich July 5, 2021 23:48
@randall randall force-pushed the ocsp_boringssl branch 2 times, most recently from 30d3af9 to 148db52 Compare July 6, 2021 01:57
@randall randall force-pushed the ocsp_boringssl branch 2 times, most recently from 82d8b42 to 39b5ef6 Compare July 19, 2021 22:55
Requires the external library boringocsp
@randall randall merged commit 5ddb462 into apache:master Jul 20, 2021
@randall randall deleted the ocsp_boringssl branch July 20, 2021 17:19
zwoop pushed a commit that referenced this pull request Jul 20, 2021
Requires the external library boringocsp

(cherry picked from commit 5ddb462)
@zwoop
Copy link
Contributor

zwoop commented Jul 20, 2021

Cherry-picked to v9.1.x branch.

@zwoop zwoop modified the milestones: 10.0.0, 9.1.0 Jul 20, 2021
masaori335 pushed a commit to masaori335/trafficserver that referenced this pull request Aug 2, 2021
* asf/9.1.x: (28 commits)
  Updated ChangeLog
  Make the rest of InkAPI allocators Proxy Allocated (apache#8106)
  Added missing milestones and updated slow log report script (apache#8168)
  Cleans up the code bit, including milliseconds consistency (apache#7989)
  Note YAML parser library bug, and work-around, in documentation. (apache#7963)
  Update INSTALL for URLs and version number (apache#8173)
  ESI plugin documentation updates. (apache#7970)
  Add a JSON schema for strategies.yaml (apache#7932)
  ensure hostname_offset is initialized to '0' to indicate null hostname (apache#8162)
  Fixed compile error with Linux AIO unit test (apache#7958)
  Enforce case for well known methods (apache#7886)
  Treat TRACE with body as bad request (apache#7905)
  Close connection after every bad request for HTTP/1.1 (apache#7885)
  use sendmsg and recvmsg (apache#7793)
  Apply log throttling to HTTP/2 session error rate messages (apache#7772)
  limit m_current_range to max value in RangeTransform (apache#4843)
  Fix HPACK eviction iterator manipulation (apache#8004)
  Replace fix assert in error event processing (apache#8058)
  Adds OCSP support for BoringSSL (apache#7298)
  .gitignore rules for gcov generated files (apache#8099)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants