add OCSP support #124

lampad · 2017-03-08T22:40:29Z

An attempt to resolve #24

Finally found enough random time to finish this up.

Some assertions in test_ocsp.rb (ripped straight from 2.4.0 CRuby) are commented out because of some odd behavior I noticed with decoding objects using ASN1.decode. A cursory glance seems to suggest that bits of data are left out of the decoded objects or aren't put back in to the der string when calling to_der.

I decided to TODO them since it felt like scope creep, and my mind just wasn't ready to dive into that side of things.

Let me know if you have questions or anything. This is my first real foray into JRuby, so if I did stupid stuff, please let me know.

TODO: - Replace manual construction of Request in #sign with OCSPReqBuilder - Fix verification

* There seems to be an issue with decoding objects using ASN1.decode and then trying to reconstitute decoded objects by calling to_der on them. Some data seems to be left off. Not covering it here because of scope creep.

headius · 2017-03-09T01:21:23Z

Wow!

kares · 2017-03-09T06:27:12Z

src/main/java/org/jruby/ext/openssl/OCSP.java

+        return Utils.newError(runtime, _OCSP(runtime).getClass("OCSPError"), ex);
+    }
+
+    public static RaiseException newSSLError(Ruby runtime, String message) {


📛 seems wrong

Yeah, not sure why I added this in the first place. Thanks for catching.

kares · 2017-03-09T06:29:31Z

src/main/java/org/jruby/ext/openssl/OCSPBasicResponse.java

+* ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+* DEALINGS IN THE SOFTWARE.
+*
+*  Copyright (C) 2007-2009 Ola Bini <ola.bini@gmail.com>


Haha, ok. I realized I need to add the license to the other files too.

kares · 2017-03-09T06:33:22Z

src/main/java/org/jruby/ext/openssl/OCSPBasicResponse.java

+
+    private RubyString der;
+    private byte[] nonce;
+    private SecureRandom random = new SecureRandom();


we should delay the SecureRandom creation - and use the helper from SecurityHelper (this is minor, really)

The final version ended up not needing SecureRandom anyway

kares · 2017-03-09T06:34:47Z

src/main/java/org/jruby/ext/openssl/OCSPBasicResponse.java

+
+    @JRubyMethod(name = "add_status")
+    public OCSPBasicResponse add_status(final ThreadContext context) {
+        //TODO: Implement


shouldn't we also print a warning on these stub impls? (am not sure - thus asking, there's helpers for that)

Sorry, those were notes for myself to implement them before pushing. The final version should have everything implemented. I could rebase/squash my commits if that would help so people wouldn't even see this. Thoughts?

kares · 2017-03-09T06:39:20Z

src/main/java/org/jruby/ext/openssl/OCSPRequest.java

+
+    @JRubyMethod(name = "to_der")
+    public IRubyObject to_der() {
+        //TODO implement


it definitely makes sense to have warnings on these e.g.
OpenSSL.warn(context, "WARNING: unimplemented method called: OCSP::Request#to_der");

kares · 2017-03-09T06:40:06Z

src/main/java/org/jruby/ext/openssl/OCSPRequest.java

+    public IRubyObject verify(IRubyObject certificates, IRubyObject store, IRubyObject flags) {
+        //TODO implement
+
+        return RubyBoolean.newBoolean(getRuntime(), true);


safer to return false if its unimplemented for users relying on this feature, right?

kares · 2017-03-09T06:41:20Z

src/main/java/org/jruby/ext/openssl/StringHelper.java

@@ -29,6 +29,8 @@
 import java.util.ArrayList;
 import java.util.Locale;

+import org.bouncycastle.asn1.ASN1InputStream;
+import org.bouncycastle.asn1.ASN1Primitive;


kares · 2017-03-09T07:12:14Z

CI also catched a (minor) issue :

[INFO] java.lang.ClassCastException: org.bouncycastle.jcajce.provider.asymmetric.x509.X509CertificateObject cannot be cast to org.bouncycastle.jce.provider.X509CertificateObject
[INFO] 	at org.jruby.ext.openssl.OCSPRequest.findCertByName(OCSPRequest.java:389)
[INFO] 	at org.jruby.ext.openssl.OCSPRequest.verify(OCSPRequest.java:307)

.. otherwise this is looking 🍇 as far as I can tell 💘

lampad · 2017-03-09T15:22:30Z

I'll update based on comments and try to figure out the CI failure.

lampad · 2017-03-09T17:53:22Z

@kares I'm not sure what's going on with the build failures now.

One appears to have timed out: https://travis-ci.org/jruby/jruby-openssl/jobs/209400174

The others seem to be having trouble getting their environment/dependencies set up to run the ruby tests in their respective environments: https://travis-ci.org/jruby/jruby-openssl/jobs/209400177

I can't tell if I caused this problem or not... When I look at the file(s) in the stack in my checked out copy (jruby-openssl/lib/openssl.rb and jruby-openssl/lib/jopenssl/load.rb) i don't see the relevant lines... Am I missing something?

kares · 2017-03-09T18:06:07Z

@lampad no worries. thanks for all the code!

HoneyryderChuck · 2017-03-28T19:01:52Z

I've bundled the master branch, and I'm still seeing the same error:

NameError: uninitialized constant OpenSSL::OCSP

HoneyryderChuck · 2017-03-28T19:03:19Z

@lampad , your issues might have to do with #119

lampad · 2017-03-28T20:52:09Z

@HoneyryderChuck I'm not sure what "bundling" the master branch does exactly, but you'll probably need to package/repackage the new jopenssl.jar and the jruby-openssl gem and install the gem in your local gem env. There are instructions on how to do the packaging in the BUILDING.md file which should spit out a gem in the pkg dir.

lampad · 2017-03-28T20:55:58Z

@HoneyryderChuck and yeah, I think you're correct re: #119. I didn't dig too deep into it there, but I did notice some functionality missing in the ASN1 class. If I find some time, I'll take a deep dive.

HoneyryderChuck · 2017-03-29T09:25:34Z

@lampad I think bundler already takes care of that. $LOAD_PATH is at least pointing to a local bundled jruby-openssl directory (.../jruby-openssl-580ea57f016b/lib, which hash-key is the current last commit from master).

HoneyryderChuck · 2017-03-29T09:48:15Z

here's the relevant bundler log run in verbose mode:

Fetching https://github.com/jruby/jruby-openssl.git
...
Using jruby-openssl 0.9.21.dev (java) from https://github.com/jruby/jruby-openssl.git (at master@580ea57)
0:  jruby-openssl (0.9.21.dev) from /Users/tiagocardoso/Projects/jaguar/.bundle/jruby/2.3.0/bundler/gems/jruby-openssl-580ea57f016b/jruby-openssl.gemspec
...

kares · 2017-03-29T09:59:58Z

@HoneyryderChuck jruby-openssl does not work when using :git => ... ext compilation won't happen.

UPDATE: here's a SNAPSHOT build that should do.

HoneyryderChuck · 2017-03-29T11:25:29Z

@kares thx for the info. That's unfortunate, my expectation for bundler would be definitely something else. Is this a pending/in-progress feature, and does it concern bundler or jruby?

After downloading the package, is there a way to automate installing the local package with bundler, or do I resort to following the instructions in BUILDING.md, as suggested by @lampad ?

kares · 2017-03-29T11:46:01Z

that's a usual issue with native gems as they require building (e.g. same with nokogiri), this could be hacked to get working in an 'un-official' way. it does not concern JRuby nor Bundler atm.

you just gem install the downloaded gem and than use it (specify its version in Gemfile). this is a bit out-of-scope here. just do the same thing as you would if you had a third-party gem not on RubyGems.

HoneyryderChuck · 2017-03-29T13:17:09Z

@kares, doh, you're right :) managed to do it, and on to the next issue.

lampad added 7 commits January 27, 2017 17:19

Basic structure of everything. Lots more to do.

119dc42

Move toward more BC-centric processing

14eadfd

Working on successful build and test completion

2d89857

Most tests passing.

796f7dd

TODO: - Replace manual construction of Request in #sign with OCSPReqBuilder - Fix verification

Verification of BasicResponses working

579cd72

All tests passing*

420c068

* There seems to be an issue with decoding objects using ASN1.decode and then trying to reconstitute decoded objects by calling to_der on them. Some data seems to be left off. Not covering it here because of scope creep.

Import cleanup

fa967c5

kares reviewed Mar 9, 2017

View reviewed changes

Review comments, build fix(hopefully), licenses

3379bc3

kares changed the title ~~Add ocsp~~ add OCSP support Mar 9, 2017

kares merged commit 96955ce into jruby:master Mar 9, 2017

lampad deleted the add_ocsp branch March 9, 2017 18:06

kares added a commit that referenced this pull request Mar 13, 2017

post review #124 - mostly to follow JRuby ext best-practices + dry-out

3336fb8

HoneyryderChuck mentioned this pull request Mar 29, 2017

Can't require in jruby cchandler/certificate_authority#50

Open

VimCommando mentioned this pull request Feb 13, 2020

Add support for TLS/SSL certificate revocations elastic/logstash#11602

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add OCSP support #124

add OCSP support #124

lampad commented Mar 8, 2017

headius commented Mar 9, 2017

kares Mar 9, 2017

lampad Mar 9, 2017

kares Mar 9, 2017

lampad Mar 9, 2017

kares Mar 9, 2017

lampad Mar 9, 2017

kares Mar 9, 2017

lampad Mar 9, 2017

kares Mar 9, 2017

kares Mar 9, 2017

kares Mar 9, 2017

lampad Mar 9, 2017

kares commented Mar 9, 2017

lampad commented Mar 9, 2017

lampad commented Mar 9, 2017

kares commented Mar 9, 2017

HoneyryderChuck commented Mar 28, 2017

HoneyryderChuck commented Mar 28, 2017

lampad commented Mar 28, 2017

lampad commented Mar 28, 2017

HoneyryderChuck commented Mar 29, 2017

HoneyryderChuck commented Mar 29, 2017

kares commented Mar 29, 2017 •

edited

Loading

HoneyryderChuck commented Mar 29, 2017

kares commented Mar 29, 2017

HoneyryderChuck commented Mar 29, 2017

add OCSP support #124

add OCSP support #124

Conversation

lampad commented Mar 8, 2017

headius commented Mar 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kares commented Mar 9, 2017

lampad commented Mar 9, 2017

lampad commented Mar 9, 2017

kares commented Mar 9, 2017

HoneyryderChuck commented Mar 28, 2017

HoneyryderChuck commented Mar 28, 2017

lampad commented Mar 28, 2017

lampad commented Mar 28, 2017

HoneyryderChuck commented Mar 29, 2017

HoneyryderChuck commented Mar 29, 2017

kares commented Mar 29, 2017 • edited Loading

HoneyryderChuck commented Mar 29, 2017

kares commented Mar 29, 2017

HoneyryderChuck commented Mar 29, 2017

kares commented Mar 29, 2017 •

edited

Loading