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

recreate certs for SSL example configs #866

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gbirchmeier
Copy link
Member

This doesn't work yet

@gbirchmeier
Copy link
Member Author

@dckorben - Since you know a lot about SSL certs, do you see anything obviously wrong here?

I've included shell scripts which list the commands used to recreate these certs. I've gotten something wrong, but I'm not sure what.

@dckorben
Copy link
Contributor

dckorben commented Mar 4, 2025

Is the intent for these re-create on every build? If so, there are .NET native things that can be done similarly to the tests I added for SSLStreamFactory that would work nicely.

I don't see the passphrase being set in the openssl commandline for the certs -nodes.
I'm not sure off the top my head what would happen if you don't have passphrase protection on the key but then provide one in the quickfix config SSLCertificatePassword=. I'm assuming it would fail but...

That's my cursory review but I'll try to test this more definitively.

@dckorben
Copy link
Contributor

dckorben commented Mar 4, 2025

I take it you are getting RemoteCertificateNameMismatch?

@gbirchmeier
Copy link
Member Author

Yes, that's what I get.

The reason for this PR is that it turns out the old SSL certs never worked on my Mac. They came from a submission years ago, and I think I vetted them on Windows at the time. Recently I discovered they didn't work on Mac, and decided to try to fix that. And here we are.

@dckorben
Copy link
Contributor

dckorben commented Mar 4, 2025

I reserve my right to change my mind but it looks like the root cause is: Those certs are very old and lacking SANs but then the SSLServerName= is being compared to the new certs CN and resulting in RemoteCertificateNameMismatch. It looks like you updated the names in both places though, so.

I'm gonna do a little more testing to confirm a fix.

SSLServerName=QuickFixn-TestServer
SSLCACertificate=../QuickFixn-TestCA.cer
SSLServerName=qfn-server.demo
SSLCACertificate=../quickfixn-CA.demo.cer

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need SSLCheckCertificateRevocation=N in this config

@gbirchmeier
Copy link
Member Author

The pre-PR certs are very old, of course. I have no idea what commands were used to create them.

The certs in this PR are much less important than the process used to create them. My bash scripts are just a record of what I tried. If there is a better way to create the certs (such as the .NET native way you mentioned), I'm all for it. The true goal is to have a repeatable cert-creation procedure that produces certs that work on all platforms.

@dckorben
Copy link
Contributor

dckorben commented Mar 4, 2025

My bash scripts are just a record of what I tried. If there is a better way to create the certs (such as the .NET native way you mentioned), I'm all for it. The true goal is to have a repeatable cert-creation procedure that produces certs that work on all platforms.

I threw together a powershell which generated the 3 certs and successfully handshaked but it was getting late so I need to verify my work. Since there are other ps scripts in the repo, I assume those would be viable?

@gbirchmeier
Copy link
Member Author

Yeah, powershell is my preference, actually, since it is cross-platform.

Actually, when I made this PR, I don't think I planned on including scripts -- just the corrected certs. The bash scripts were just to document what I tried. But now with the benefit of memory-fogged hindsight, I think documenting the procedure with scripts is an important thing to do.

@dckorben
Copy link
Contributor

dckorben commented Mar 5, 2025

https://github.com/dckorben/quickfixn/tree/develop-demo-certs
The certs included in my branch only live for 5 days, I suspect you'd prefer to regenerate them on your side and commit those to in the final version to master. The ps is a little more verbose than it necessarily needs to be but I was trying to get it as close to the old certs as you can get in the modern world.

I dropped SSLServerName=qfn-server.demo and added SSLCheckCertificateRevocation=N
I can submit this as a separate pull request if you would prefer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants