on_ssl_connectfail logging

[ripienaar]

hello,

Busy testing the new on_ssl_connectfail logging and it looks good, certainly provides all the information you'd need to give useful feedback.

I am wondering if it would be a good idea to give this logger some way to abort, disconnect and raise an exception that bubbles up back to the ruby code that uses the gem?

Imagine the CA file does not exist or is wrong so you get an verification error. You may argue that retrying forever is fine since the user might eventually put the right CA file down. But you might also want to give the user the ability to say when this happens, stop retrying and just fail?

[ripienaar]

Also as a fyi - I don't know if this is by design but I can edit your wiki pages. I fixed something in the SSL examples, this might need tightening up.

[gmallard]

I do not know about putting the logic in the logger, but probably any exception that is a 'Stomp::Error' type should be bubbled up to the caller. I think that is fairly easy to do, and should be done in the 'socket' method's rescue block. Would that work for you?

Re: the wiki - I think that once a github wiki is enabled, it is open to all github users. By all means, make corrections as you see fit. I am sure there are some oddities in what is there now ...... it was write code/write docs back and forth.

[ripienaar]

hmm, if Stomp::Error::* caused hard fails then users would have no way to say those are acceptable for retrying?

Maybe add a new exception type that users can raise in the logger? It does indeed feel odd to put this logic in the logger but I cant think of anywhere else users would be able to influence?

[gmallard]

Of course, 'no exceptions' from callbacks is documented - but nothing prevents one from actually calling raise outside of a begin/rescue. The original intent was that erroneous user code would not/could not prevent the gem from proceeding. No way to enforce it, really.

Right now, if you actually do raise from one of the 'connectfail' callbacks ........ doesn't that get you what you you need?

[gmallard]

After a little thought, you will probably get caught by reliable/retry logic with that.

A new exception type that can me raised in only the 'connect' related callbacks might work.

Let me think about this a little.

[ripienaar]

Yes, this was the conclusion I came too from reading the code, some special exception class is needed

[gmallard]

Take a look at the HEAD of the 'dev' branch.

[ripienaar]

OK that looks like it will do what I want, I will test hopefully tonight or morrow, else I am out rest of the week.

Curious re code style, why:

raise if aex.class.to_s.index("Stomp::Error::LoggerConnectionError")

vs

raise if aex.is_a?(Stomp::Error::LoggerConnectionError)

[gmallard]

Re the style, no particular reason - it just came off my fingers that way. Yours is better actually.

Let me know about the testing.

[ripienaar]

Yes, tested that branch and I think I this approach is good enough to let me bail out in a good way

thanks!

[gmallard]

That is on master now.

Please mull over the question of is there anything obviously missing in the SSL work.

I think this last work needs to go 'out the door', but do not want to cut a new gem, only to soon discover that something else is needed.

[ripienaar]

OK, I'll think a bit and get back to you in the next day or two.

thanks a lot!

[gmallard]

I am contemplating changing all of the File::exists? calls to either:

• File::readable?
• File::readable_real?

Or probably adding that as an additional check.

Do you have any thoughts on this?

[ripienaar]

Yeah those should be better - File::readable? should be enough I'd think

[gmallard]

I am doing some tweaking. Keep watching the HEAD of 'dev'.

[ripienaar]

Sorry was away last week, see you pushed a new gem, I'll test it soon :)

[ripienaar]

Logging works fine,

I have a question about the SSL connection setup - I swear I tested this but maybe i messed up.

It seems the case 4 requires every clients certificate to be stored on the activemq in the trust store. I hoped there was a way where it could be configured to accept any client request from clients using certificates signed using the CA configured on in the activemq keystore.

Did you come across a way to do this? our client list changes regularly - but we do sign them all so we're happy to trust any cert signed by the CA in use on the ActiveMQ but needClientAuth=true in activemq seem to require the clients all to be known

[gmallard]

:-)

For sure, I have not tested every possible combination of CA certs versus individual client certs.

You said 'CA configured in the keystore'. What happens if you put (only) that 'CA cert in the truststore' ? needClientAuth=true does require all clients to be known. I am pretty certain of that. But I would suspect they can all be known by a single CA cert in the truststore. YMMV. I have not tried it. This weekend , maybe.

Your CA .... it signs both your server and client certs, correct? Or are there two private CA's involved? Just thinking ......

Let me know how the experiments go ...... and you could document new insights on the wiki :-)

[ripienaar]

I should have stopped working hours ago, it just works if you only add the CA to the truststore and nothing else, silly me :)

It's only 1 CA :)

This stuff is 1.2.x is looking pretty good so far, I've had one person who complained what looks like characters going missing over the wire with 1.2.1 even without SSL but he couldnt produce test data or any useful information that would help me debug so I am chalking that down to user error

[gmallard]

You are in the UK correct? It is late there, go to bed.

[ripienaar]

heh yeah UK time :)

[gmallard]

I am closing this issue.

If further action is required, please reopen this or create a new issue.