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

PVA over TLS #2

Closed
wants to merge 5 commits into from
Closed

PVA over TLS #2

wants to merge 5 commits into from

Conversation

mdavidsaver
Copy link
Owner

@mdavidsaver mdavidsaver commented Jul 26, 2023

Preliminary work on applying OpenSSL to PVXS.

Client search requests include two "protocol" names: tcp and tls. Server listens on a second TCP port for TLS connections, prefers to respond with tls when both present.

Adds x509 AUTHZ method. Client advertises x509 when configured with a client cert. Server prefers x509 to ca. If selected, uses client cert commonName as account name. eg. CN=foo appears to ACF logic as foo. (note, this really needs an extension to the ACF logic to account for different source of accounts)

TLS features/restrictions

  • OpenSSL >= 3.0
  • TLS protocol version >= 1.3 required
  • ALPN extension negotiates with protocol pva/1. (mandatory?)
  • Client/server reload TLS configuration (eg. replace cert) at runtime. Forces closes connections.

New Configuration:

  • $EPICS_PVAS_TLS_KEYCHAIN $EPICS_PVA_TLS_KEYCHAIN name of a PKCS12 file containing some combination of key, server/client certificate, and/or CA certificates. The file path may optionally be followed by ; and a password. eg. EPICS_PVAS_TLS_KEYCHAIN=/path/to/some.p12;secret.
  • $EPICS_PVAS_TLS_PORT $EPICS_PVA_TLS_PORT default port to listen for TLS connections
  • $OPENSSL_CONF Location of OpenSSL config file. Defaults to /usr/lib/ssl/openssl.cnf on Linux. Application name pvxs is used.
  • $SSLKEYLOGFILE If set, TLS session keys will be written here. eg. for use by wireshark. cf. Decode PVA in TLS via ALPN cashark#14

Building:

libevent must be built with optional openssl support (bundled build will detect). openssl is expected to be installed in the default search path. I have no plans to bundled openssl.

Currently, python3-pyopenssl is also required. This is planned to be temporary during development.

TODO:

  • Configure TLS in $EPICS_PVA_NAME_SERVERS w/ URI-ish pvas://1.2.3.4:5678.
  • Verify interoperability with PKCS12 created by java keytool (OpenSSL parser for PKCS12 is... limited)
    • Can work with files created by openssl pkcs12 or keytool. (although Java can't current work with openssl created files)
    • Handle password protected PKCS12 (which java keytool insists on...)
  • Fix OSX RPATH issue
  • IOC shell commands to manage TLS (re)configuration
  • Test server reconfigure
    • Expose server credentials to client
  • x509 AUTHZ method design
    • Add authority alongside method and account
  • What to do about CMD_BEACON ?
  • OSCP...
  • What to do about certificate expiration? (drop credentials? force reconnect w/o TLS?)
  • Include CA IDs in search request/reply? (more graceful fallback if mis-matched CA certs)
  • Update python module build
  • Investigate openssl provider/engine support. eg tpm2-openssl

@george-mcintyre @kasemir @gregoryraymondwhite fyi. Something to discuss. May be ready for early testing. Definitely not ready for wider testing (no documentation, config. inputs may change).

@mdavidsaver mdavidsaver added the enhancement New feature or request label Jul 26, 2023
@mdavidsaver mdavidsaver self-assigned this Jul 26, 2023
@AppVeyorBot
Copy link

@mdavidsaver
Copy link
Owner Author

Testing recipes:

Run make runtests an look in test/O.Common or explicitly run test/gen_test_certs.py.

EPICS_PVA_TLS_KEYCHAIN=test/O.common/ioc1.p12 ./bin/linux-x86_64/softIocPVA ...

and then one of:

EPICS_PVA_TLS_KEYCHAIN=test/O.common/client1.p12 pvxget ...
EPICS_PVA_CA_FILES=test/O.common/ca.pem pvxget ...

@mdavidsaver
Copy link
Owner Author

mdavidsaver commented Jul 28, 2023

An item on the TODO list Verify interoperability with PKCS12 .... To facilitate interoperability testing, I am hoping to find a recipe to create a set of .p12 files generated with Java keytool, which are analogous to those created by test/gen_test_certs.py.

From what I can tell, Java and OpenSSL have somewhat different interpretations of the PKCS#12 spec.

One difference between the two being that Java keytool will add an extra attribute to mark trusted root CA(s). OpenSSL does not know about this attribute.

Another (potential) difference is in how the end point certificate is distinguished from intermediate/root CAs. This may cause complications with a .p12 files containing only a root CA. A keychain file with only a root CA seems a desirably as a common client configuration.

Also, I am not sure what to do about the possibility of multiple root CAs...

edit: not as different as I initially though. see documentation/pkcs12.md

@mdavidsaver
Copy link
Owner Author

mdavidsaver commented Aug 1, 2023

Updated with changes to:

  • Add API calls, and ioc shell command pvxreconfigure. Which re-reads (only) the TLS related environment variables, then force disconnects any peers.
  • Reading of PKCS#12 files no longer makes assumptions about order. I have tried to be flexible, inspecting attributes of certificate (not PKCS#12 bag attributes), when deciding whether to place in trusted or untrusted lists.
  • Add ServerCredentials to allow clients to inspect server "credentials", which are derived from the server cert. in the same manner as a server will do for a client certificate. Accessed through client Connected exception, which may now also be passed to ConnectBuilder callbacks.

@AppVeyorBot
Copy link

@AppVeyorBot
Copy link

@kasemir
Copy link

kasemir commented Aug 4, 2023

As for interoperability of the PKCS12 files, the ioc1.p12 works for the Java demo server.
ca.pem can be used for the Java client, except it needs to be turned into a PKCS12 file as done with myca.cer in step 3 below.

Here's an example of creating a similar 'ioc' and 'ca' using keytool.

  1. Create our own CA, and make its public certificate available as myca.cer:
keytool -genkeypair -alias myca -keystore ca.p12 -storepass changeit -dname "CN=myca" -keyalg RSA -ext bc=ca:true
keytool -list                   -keystore ca.p12 -storepass changeit
keytool -exportcert -alias myca -keystore ca.p12 -storepass changeit -rfc -file myca.cer
keytool -printcert -file myca.cer 
  1. Now create a server keypair for use by the IOC:
keytool -genkeypair -alias myioc -keystore ioc.p12 -storepass changeit -dname "CN=myioc" -keyalg RSA
keytool -list -v                 -keystore ioc.p12 -storepass changeit

It starts out as a "self-signed certificate" with matching owner and issuer.
Create a certificate signing request. The CSR could be sent to a commercial CA, but we sign it with our own CA.

keytool -certreq -alias myioc -keystore ioc.p12 -storepass changeit -file myioc.csr
keytool -gencert -alias myca  -keystore ca.p12  -storepass changeit -ext san=dns:myioc -infile myioc.csr -outfile myioc.cer
keytool -printcert -file myioc.cer

Import the signed certificate into the ioc keystore. Since ioc.cer is signed by 'myca', which
is not a generally known CA, we will get an error like "Failed to establish chain"
unless we first import myca.cer to trust out local CA.

keytool -importcert -alias myca  -keystore ioc.p12 -storepass changeit -file myca.cer  -noprompt
keytool -importcert -alias myioc -keystore ioc.p12 -storepass changeit -file myioc.cer
keytool -list -v                 -keystore ioc.p12 -storepass changeit
  1. For clients, create a truststore that holds just the CA certificate

A client will trust any IOC certificate signed by 'myca' once it's aware of the 'myca' certificate,
which needs to be imported into the PKCS12 file format:

keytool -importcert -alias myca  -keystore trust_ca.p12 -storepass changeit -file myca.cer  -noprompt

So now an IOC can use ioc.p12, and clients can use trust_ca.p12.
One could create additional ioc*.p12, in the extreme one per IOC, and clients would accept them all because they use the trusted CA.

@kasemir
Copy link

kasemir commented Aug 7, 2023

The tcp vs tls behavior is a little sticky on the server side.

  1. Start softIocPVA with a ..PVAS_KEYCHAIN=ioc1.p12

  2. Read via plain 'pvxget' and server sends a "tcp" search reply. That's good for older clients that don't support tls

  3. Read via 'pvxget' that uses a suitable ..PVA_KEYCHAIN, so it searches for both tcp and tls, and server sends a "tls" search reply. That's good, new setups will prefer tls.

  4. Try the plain pvxget again, it will search for "tcp" as before, but server still sends a "tls" reply.

..So once the server replied with "tls", it keeps doing that, no matter what the seach supports...

@kasemir
Copy link

kasemir commented Aug 7, 2023

For a test, I changed the PKCS12_parse call in ossl.cpp to use "changeit" as a password.
With that, softIocPVX can open the keytool-generated ioc.p12, and it will communicate with a java client that uses trust_ca.p12. The pvxget C++ client will read using ioc.p12, but with the more appropriate trust_ca.p12 it complains ssl/ssl_rsa.c:215:passed a null parameter, SSL_CCTX_use_certificate.

@mdavidsaver
Copy link
Owner Author

... little sticky ...

... ssl/ssl_rsa.c:215:passed a null parameter, SSL_CCTX_use_certificate ...

These should be resolved. fyi. the second actually caused pvxsr 2 to SIGSEGV when a TLS client w/o certificate was connected.

@mdavidsaver
Copy link
Owner Author

$ keytool ... -storepass ""
keytool error: java.lang.Exception: Keystore password must be at least 6 characters

So keytool forces a password. What is a non-interactive daemon supposed to do other than store that password right alongside the pkcs12 file? Ah, the joys of CYA security!

I am tempted to invent some syntax like EPICS_PVA_TLS_KEYCHAIN=/path/file.p12;password.

@mdavidsaver
Copy link
Owner Author

A keytool generated file can be reprocessed with openssl w/o a password. This losses the attributes which openssl doesn't understand. friendlyName (aka -alias) and the 2.16.840.1.113894.746875.1.1 trust marker.

openssl pkcs12 -in some.p12 -out temp -passin pass:changeit -passout pass:
openssl pkcs12 -export -in temp -out another.p12  -passin pass: -passout pass:

@AppVeyorBot
Copy link

@kasemir
Copy link

kasemir commented Aug 8, 2023

some syntax like EPICS_PVA_TLS_KEYCHAIN=/path/file.p12;password

That's reasonable. We do need a password, even "" is a password. We could store it in some "password manager", be it a custom file or the credential store that Windows and Mac OS offer, but that just shifts the issue one level up to providing the manager's password to then obtain the keychain password, and the IOC can't ask for the manager's password on startup.

@kasemir
Copy link

kasemir commented Aug 8, 2023

You indeed fixed the sticky tls behavior and the "null parameter, SSL_CCTX_use_certificate".

I can now read from softIocPVA with both pvxget and java client,
using either the openssl-generated ioc1.p12 (server) and ca.pem (client, imported into a trust_ca.p12),
or keytool-generated ioc.p12 and trust_ca.p12.
Also possible to intersperse plain TCP gets with the TLS gets.

@mdavidsaver
Copy link
Owner Author

Updated to add syntax for TLS in name servers. EPICS_PVA_NAME_SERVERS=pvas://1.2.3.4:5678 Also for PKCS#12 files with password. EPICS_PVAS_TLS_KEYCHAIN=/path/to/some.p12;secret.

@AppVeyorBot
Copy link

@kasemir
Copy link

kasemir commented Aug 11, 2023

The new EPICS_PVAS_TLS_KEYCHAIN=/path/to/some.p12;secret works well for me, so does EPICS_PVA_NAME_SERVERS=pvas://1.2.3.4:5076.

A minor quirk is that EPICS_PVA_NAME_SERVERS=pvas://1.2.3.4 with no port assumes port 5075, the default TCP port, while the TLS socket defaults to 5076, so must list the default TLS port as in EPICS_PVA_NAME_SERVERS=pvas://1.2.3.4:5076. But after users managed to create all those certificates, that's really minor.

@mdavidsaver
Copy link
Owner Author

Another update.

I am confident enough now in my understand of PKCS#12 file interoperability that I have removed $EPICS_PVAS_CA_DIRS and friends. See documentation/pkcs12.md for my attempt to explain. test/gen_test_certs_j.sh is derived form Kay's examples of keytool usage, and is intended to produce a set of .p12 files equivalent to test/gen_test_certs.py. (not identical because keytool insists on a password...)

I have also added an "authority" member to ClientCredentials. For methods "anonymous" and "ca" this is empty. For "x509" this is the Common Name of the root CA. Looking forward, for Kerberos this would be the realm name.

Certificate expiration date is now printed from pvxsr and various other places. (not yet anything more)

I have also made failure to read/parse/setup TLS keychain a non-fatal error. I could be persuaded to change this for the CLI tool, but I think it better that IOCs continue to start since this error can be corrected at runtime (with pvxreconfigure).

... with no port assumes port 5075, the default TCP port ...

Should be fixed.

@mdavidsaver
Copy link
Owner Author

I am also thinking about if/how to configure a "client cert. required" mode, which would make an public facing PVA gateway a less than totally insane proposition.

@kasemir
Copy link

kasemir commented Aug 14, 2023

"client cert. required" mode

Good question. On one hand, an option to require a client certificate sounds the proper thing to do.
On the other hand, I can easily run a client with "ioc.p12" and then it identifies as an IOC.
With "ca authentication" where it gets the user name from the OS I tend to get the actual name of who's running the client.

@mdavidsaver
Copy link
Owner Author

mdavidsaver commented Aug 14, 2023

... the actual name ...

Is system account name actually actual? Group accounts are still to be found at *NLs... Or whatever string a client is made to send.

With this PR, identity as viewed by pvxs would be a triple: method, authority, and account. eg. ("ca", "", "mdavidsaver") vs. ("x509", "myca", "mdavidsaver"). These two "mdavidsaver" accounts should not be seen as equivalent. Distinguishing between them is a question of authorization, which I intend to delegate to the ACF engines in P4P and Base.

I see authorization as (mostly) out of scope for this PR. The in-scope parts are the question of backwards compatibility in how credentials are presented, where existing clients could see these two "mdavidsaver" as identical. Also how to decide to use "ca" vs. "x509". I am not sure what, if anything, to do about these.

Also, it occurs to me that we could support allowing a peer to provide more than one identity. eg. both "ca" and "x509". I can't think of a reason not to do this, but am also not sure if it is worth the effort. Since it would mean a more involved API change, my default position is "no".

@mdavidsaver
Copy link
Owner Author

... an option ...

I also hesitate to have too many configuration options. eg. I don't want to end up with dozens of EPICS_PVA_TLS* variable names (with matching EPICS_PVAS_TLS*). So I don't want eg. EPICS_PVA_TLS_CLIENT_CERT=REQUIRED, but might go with something more open ended like EPICS_PVA_TLS_OPTIONS=client_cert=required ....

@kasemir
Copy link

kasemir commented Aug 14, 2023

EPICS_PVA_NAME_SERVERS=pvas://1.2.3.4:5678 ... with no port assumes port 5075, the default TCP port ...

Should be fixed.

It's now different. What I expected for EPICS_PVA_NAME_SERVERS:

  • localhost uses localhost port 5075 for a plain TCP search.
  • localhost:5075 same with explicit port.

Those are still fine. But these..

  • pvas://localhost uses localhost port 5076 for a TLS search
  • pvas://localhost:5076 same with explicit port

now result in
ERR pvxs.config EPICS_PVA_NAME_SERVERS ignoring invalid 'pvas://...' : URI unsupported in this context

From the plain TCP 5075 search I will get a "tls" reply so the data is then via TLS, but I can no longer start with TLS for the search.

@kasemir
Copy link

kasemir commented Aug 14, 2023

Agree that how to handle authorization is beyond this.

Within this is supporting "ca" and "x509", and some way to select that on the client side (no certificate vs. client certificate with name) --> Done!

On the server side, not sure we need a "client cert required" option. That could be handled in the authorization, implemented later, based on the available information.
In a future authorization configuration file you can then control for example

"anonymous": No access
"ca": Read access
"x509": Read access plus for certain names write access

--> Server side is also done since we know if we get anon, ca or x509?

@mdavidsaver
Copy link
Owner Author

URI unsupported in this context

Sorry, this was a late change. I guess I put it in the wrong place. This error in meant to prevent URIs in contexts with a UDP port.

@mdavidsaver
Copy link
Owner Author

On the server side, not sure we need a "client cert required" option. That could be handled in the authorization, implemented later, based on the available information.

My motivation to think about a special switch is twofold. One technical, and one usability.

"client cert required" exists as a TLS protocol option (SSL_VERIFY_FAIL_IF_NO_PEER_CERT), which is set before any connection is opened, and is acted on before any application messages are exchanged. Setting this flag would prevent anonymous clients from getting to a point of potentially sending malformed PVA messages.

Also, I think of this flag as analogous to the global read-only flags in the CA and PVA gateways. Entirely redundant to a more fully featured ACL mechanism, but simpler to understand. Hopefully also harder to misconfigure.

@kasemir
Copy link

kasemir commented Aug 14, 2023

You're correct that requiring the client cert offers a fail-early option. Yes, like the global read-only.

@mdavidsaver
Copy link
Owner Author

URI unsupported in this context

This should be resolved. Setting pvxs::client::Config::nameServers from code worked. So testtls passed. However, I had not changed the parsing when reading configuration from environment variables. testconfig now covers EPICS_PVA_NAME_SERVERS.

At some point I should remove this duplicate parsing...

@kasemir
Copy link

kasemir commented Aug 15, 2023

..NAME_SERVERS now indeed handles "pvas://.." and that changes the default TCP port from 5075 to 5076, all good.

@AppVeyorBot
Copy link

@mdavidsaver
Copy link
Owner Author

Development moved to main repository with epics-base/pvxs#53 .

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

Successfully merging this pull request may close these issues.

3 participants