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

Add support for --tls-chain #60

Merged
merged 1 commit into from
Nov 7, 2023

Conversation

wolfgangkarall
Copy link
Contributor

--tls-cert alone is not enough if an intermediate cert has to be supplied to the server as well.

--tls-cert alone is not enough if an intermediate cert has to be
supplied to the server as well.
@jetmore
Copy link
Owner

jetmore commented Nov 4, 2023

@wolfgangkarall Can you provide a scenario where this option would be needed? I would like to understand it better and add it to the test suite before merging

@jetmore jetmore added this to the next milestone Nov 4, 2023
@jetmore jetmore added the enhancement New functionality slated to be implemented someday label Nov 5, 2023
@wolfgangkarall
Copy link
Contributor Author

Having to use an intermediate CA with client certificates is a common scenario, see e.g. https://www.postfix.org/TLS_README.html#client_cert_key and the example there, and it would be nice to test these scenarios with swaks.

In our case this came up during a pentest of the SMTP infrastructure at work, where the guy was using swaks (good for him!) with only --tls-key and --tls-cert and then said our Postfix server certificate setup wasn't correct, when openssl s_client with -key, -cert and -cert_chain was able to set up the secure connection successfully (as do real clients on the same setup).

@jetmore jetmore changed the base branch from develop to add-tls-chain November 7, 2023 20:23
@jetmore jetmore merged commit 4e20cff into jetmore:add-tls-chain Nov 7, 2023
@jetmore
Copy link
Owner

jetmore commented Nov 7, 2023

Thanks @wolfgangkarall. I need to play with it a bit before it gets merged into the main branch but it looks like it belongs

@jetmore
Copy link
Owner

jetmore commented Nov 7, 2023

@wolfgangkarall Can you help me understand this a little bit more?

So you have SWAKS connecting as a client to SERVER

SWAKS is configured with CLIENT-CRT
SERVER is configured with SERVER-CRT

In the scenario you describe, which of those certs is signed by CA-CHAIN?

I think it must be the signing CHAIN for SERVER-CERT, but if that's true I'm unsure why --tls-chain requires --tls-cert/--tls-key. Swaks can verify SERVER-CRT without CLIENT-CRT being present.

@wolfgangkarall
Copy link
Contributor Author

The server is configured to accept client certificates and trusts some root CA "foo".

The client is configured to use a client certificate signed by an intermediate CA "bar" which in turn is issued by root CA "foo".

Now if the client only sends its client cert without also sending intermediate CA "bar", the server will not be able to validate the chain.

Hence --tls-chain, it allows sending the intermediate CA. I tried to supply a file with concatenated cert+intermediate to --tls-cert but that's not how it works with Net::SSLeay, you have to use CTX_use_certificate_chain_file.

The server cert is of no concern here, it can be issued by some whole different chain of CAs, but there it's also required that the intermediate is sent along with the server cert so the client can validate it against it's own list of trusted root CAs.

@jetmore
Copy link
Owner

jetmore commented Nov 8, 2023

Thanks for the explanation. That's what I originally thought was supposed to happen but I was having trouble configuring Exim to accept a chain from the client. Maybe I'll change to postfix since you say that's what you were working with

@jetmore
Copy link
Owner

jetmore commented Nov 10, 2023

@wolfgangkarall Did you actually test this change? I finally got a test scenario configured and I can authenticate with s_client but not swaks. I think I understand why and can make it work, but if you verified your code worked I'm curious what the difference is between your test and my test

# works/verifies
openssl s_client -starttls smtp -connect 192.168.0.4:1025 \
    -cert testing/certs/signed-intermediate.example.com.crt \
    -key testing/certs/signed-intermediate.example.com.key \
    -cert_chain testing/certs/ca-chain.pem

# fails to verify
./swaks --to [email protected] -q mail --server 192.168.0.4 --tls -p 1025 \
  --tls-cert testing/certs/signed-intermediate.example.com.crt  \
  --tls-key testing/certs/signed-intermediate.example.com.key \
  --tls-chain testing/certs/ca-chain.pem

@wolfgangkarall
Copy link
Contributor Author

openssl and Net:SSLeay need different files. The former is happy with just the intermediates in -cert_chain, the latter needs a 'fullchain' file, i.e. both the cert and the intermediates, see the "The certificates must be in PEM format and must be sorted starting with..." in the Net::SSLeay man page.

I'm not at work any more, so I'd need to check more specific details on Monday.

@@ -545,6 +545,10 @@ Provide a path to a file containing the local certificate Swaks should use if TL

Provide a path to a file containing the local private key Swaks should use if TLS is negotiated. The file path argument is required. As currently implemented the certificate in the file must be in PEM format. Contact the author if there's a compelling need for ASN1. If this option is set, C<--tls-cert> is also required. (Arg-Required)

=item --tls-chain <chain-file>

Provide a path to a file containing the local certificate chain (starting with the certificate followed by the necessary intermediate CA certificates) Swaks should use if TLS is negotiated. The file path argument is required. As currently implemented the certificate in the file must be in PEM format. Contact the author if there's a compelling need for ASN1. If this option is set, C<--tls-cert> and C<--tls-key> are also required. (Arg-Required)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jetmore See here for my attempt at explaining the file format needed. Of course, you could assemble some temporary file from the ones passed with --tls-cert and --tls-chain instead and pass that to Net::SSLeay

@jetmore
Copy link
Owner

jetmore commented Nov 12, 2023

Thanks for your patience on all the back and forth, Wolfgang. When you are able, will you test this swaks (https://github.com/jetmore/swaks/blob/add-tls-chain/swaks) and see if you're happy with how --tls-chain is implemented? In this implementation the entire chain can be in --tls-cert or the client cert can be in --tls-cert and the chain can be in --tls-chain. Thanks.

@wolfgangkarall
Copy link
Contributor Author

Yes, works fine, and not having to add the certificate to --tls-chain makes more sense this way, too, very nice!

@jetmore jetmore mentioned this pull request Nov 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New functionality slated to be implemented someday
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants