feat(kuma-cp) remove provided ca cert validation

[nikita15p]

Summary

With these changes sub ca be used as provided ca

Full changelog

• [Implement ...] subca
• [Fix ...] support subca as provided ca for mtls #2063

Issues resolved

Fix #2063

Documentation NA

• Link to the website [documentation PR] NA

Testing

• Unit tests
• E2E tests
• Manual testing on Universal
• Manual testing on Kubernetes

Backwards compatibility

• Add backport-to-stable label if the code is backwards compatible. Otherwise, list breaking changes.

[jpeach]

Thanks @nikita15p. Before reviewing, could you please rebase the branch and squash the commits either into a single commit, or a small series of logicall-separated commits. Please include a git commit message that explains what the changes are in each commit and why they are there.

Thanks!

[jpeach]

Need to clarify the relationship between this PR and #2575.

[nikita15p]

@jpeach Actually I raised this PR yesterday and was not aware of already existing intermediate CA PR.
However verified it with provided CA.

I am getting errors in tls handshake between the dataplanes when I pass the subca in mesh config without root cert. This mistake can be done by many users.

Debug log level of dp 1:

[2021-08-23 16:11:37.056][24371][debug][router] [source/common/router/router.cc:1614] [C15][S6866189660563418670] performing retry
[2021-08-23 16:11:37.057][24371][debug][pool] [source/common/http/conn_pool_base.cc:74] queueing stream due to no available connections
[2021-08-23 16:11:37.057][24371][debug][pool] [source/common/conn_pool/conn_pool_base.cc:241] trying to create new connection
[2021-08-23 16:11:37.057][24371][debug][pool] [source/common/conn_pool/conn_pool_base.cc:143] creating a new connection
[2021-08-23 16:11:37.057][24371][debug][client] [source/common/http/codec_client.cc:43] [C21] connecting
[2021-08-23 16:11:37.057][24371][debug][connection] [source/common/network/connection_impl.cc:860] [C21] connecting to 127.0.0.1:9090
[2021-08-23 16:11:37.058][24371][debug][connection] [source/common/network/connection_impl.cc:876] [C21] connection in progress
[2021-08-23 16:11:37.058][24371][debug][http2] [source/common/http/http2/codec_impl.cc:1222] [C21] updating connection-level initial window size to 268435456
[2021-08-23 16:11:37.058][24371][debug][connection] [source/common/network/connection_impl.cc:665] [C21] connected
[2021-08-23 16:11:37.063][24371][debug][connection] [source/extensions/transport_sockets/tls/ssl_socket.cc:219] [C21] TLS error: 268435581:SSL routines:OPENSSL_internal:CERTIFICATE_VERIFY_FAILED
[2021-08-23 16:11:37.063][24371][debug][connection] [source/common/network/connection_impl.cc:243] [C21] closing socket: 0
[2021-08-23 16:11:37.063][24371][debug][connection] [source/extensions/transport_sockets/tls/ssl_socket.cc:219] [C21] TLS error: 268435581:SSL routines:OPENSSL_internal:CERTIFICATE_VERIFY_FAILED
[2021-08-23 16:11:37.064][24371][debug][client] [source/common/http/codec_client.cc:101] [C21] disconnect. resetting 0 pending requests
[2021-08-23 16:11:37.064][24371][debug][pool] [source/common/conn_pool/conn_pool_base.cc:393] [C21] client disconnected, failure reason: TLS error: 268435581:SSL routines:OPENSSL_internal:CERTIFICATE_VERIFY_FAILED
[2021-08-23 16:11:37.064][24371][debug][router] [source/common/router/router.cc:1091] [C15][S6866189660563418670] upstream reset: reset reason: connection failure, transport failure reason: TLS error: 268435581:SSL routines:OPENSSL_internal:CERTIFICATE_VERIFY_FAILED
[2021-08-23 16:11:37.064][24371][debug][http] [source/common/http/filter_manager.cc:883] [C15][S6866189660563418670] Sending local reply with details upstream_reset_before_response_started{connection failure,TLS error: 268435581:SSL routines:OPENSSL_internal:CERTIFICATE_VERIFY_FAILED}

Debug log level of dp2:

[2021-08-23 16:11:37.056][24371][debug][router] [source/common/router/router.cc:1614] [C15][S6866189660563418670] performing retry
[2021-08-23 16:11:37.057][24371][debug][pool] [source/common/http/conn_pool_base.cc:74] queueing stream due to no available connections
[2021-08-23 16:11:37.057][24371][debug][pool] [source/common/conn_pool/conn_pool_base.cc:241] trying to create new connection
[2021-08-23 16:11:37.057][24371][debug][pool] [source/common/conn_pool/conn_pool_base.cc:143] creating a new connection
[2021-08-23 16:11:37.057][24371][debug][client] [source/common/http/codec_client.cc:43] [C21] connecting
[2021-08-23 16:11:37.057][24371][debug][connection] [source/common/network/connection_impl.cc:860] [C21] connecting to 127.0.0.1:9090
[2021-08-23 16:11:37.058][24371][debug][connection] [source/common/network/connection_impl.cc:876] [C21] connection in progress
[2021-08-23 16:11:37.058][24371][debug][http2] [source/common/http/http2/codec_impl.cc:1222] [C21] updating connection-level initial window size to 268435456
[2021-08-23 16:11:37.058][24371][debug][connection] [source/common/network/connection_impl.cc:665] [C21] connected
[2021-08-23 16:11:37.063][24371][debug][connection] [source/extensions/transport_sockets/tls/ssl_socket.cc:219] [C21] TLS error: 268435581:SSL routines:OPENSSL_internal:CERTIFICATE_VERIFY_FAILED
[2021-08-23 16:11:37.063][24371][debug][connection] [source/common/network/connection_impl.cc:243] [C21] closing socket: 0
[2021-08-23 16:11:37.063][24371][debug][connection] [source/extensions/transport_sockets/tls/ssl_socket.cc:219] [C21] TLS error: 268435581:SSL routines:OPENSSL_internal:CERTIFICATE_VERIFY_FAILED
[2021-08-23 16:11:37.064][24371][debug][client] [source/common/http/codec_client.cc:101] [C21] disconnect. resetting 0 pending requests
[2021-08-23 16:11:37.064][24371][debug][pool] [source/common/conn_pool/conn_pool_base.cc:393] [C21] client disconnected, failure reason: TLS error: 268435581:SSL routines:OPENSSL_internal:CERTIFICATE_VERIFY_FAILED
[2021-08-23 16:11:37.064][24371][debug][router] [source/common/router/router.cc:1091] [C15][S6866189660563418670] upstream reset: reset reason: connection failure, transport failure reason: TLS error: 268435581:SSL routines:OPENSSL_internal:CERTIFICATE_VERIFY_FAILED
[2021-08-23 16:11:37.064][24371][debug][http] [source/common/http/filter_manager.cc:883] [C15][S6866189660563418670] Sending local reply with details upstream_reset_before_response_started{connection failure,TLS error: 268435581:SSL routines:OPENSSL_internal:CERTIFICATE_VERIFY_FAILED}

This PR simplifies commonly occuring user issue
Also for simplicity for users, I have introduced root cert and chain cert additionally:

{
  "name": "mesh-1",
  "type": "Mesh",
  "mtls": {
    "backends": [
      {
        "name": "ca-1",
        "type": "builtin"
      },
      {
        "name": "ca-2",
        "type": "provided",
        "conf": {
          "cert": {
            "inline": "XXXXXXXXXXXXXXX"
          },
          "key": {
            "inline": "XXXX"
          },
          "rootCert": {
            "inline": "XXXXX"
          },
          "chainCert": {
            "inline": "XXXXXXXX"
          }
        }
      }
    ],
    "enabledBackend": "ca-2"
  },
  "logging": {
  	"defaultBackend": "file",
    "backends": [
      {
        "name": "file",
        "format": "{ \"destination\": \"%KUMA_DESTINATION_SERVICE%\", \"destinationAddress\": \"%UPSTREAM_LOCAL_ADDRESS%\", \"source\": \"%KUMA_SOURCE_SERVICE%\", \"sourceAddress\": \"%KUMA_SOURCE_ADDRESS%\", \"bytesReceived\": \"%BYTES_RECEIVED%\", \"bytesSent\": \"%BYTES_SENT%\"}",
        "type": "file",
        "conf": {
          "path": "/tmp/nw.log"
        }
      }
    ]
  }
}

So if the user is using self signed cert, it will work as expected as it was till now. And user need not provide rootCert and chainCert.

This PR also removes following validations which blocks using enterprise grade certs.

    "title": "Could not create a resource",
    "details": "Resource is not valid",
    "causes": [
        {
            "field": "mtls.backends[1].config.cert[0]",
            "message": "key usage extension 'digitalSignature' must NOT be set (see X509-SVID: Appendix A. X.509 Field Reference)"
        },
        {
            "field": "mtls.backends[1].config.cert[0]",
            "message": "key usage extension 'keyEncipherment' must NOT be set (see X509-SVID: Appendix A. X.509 Field Reference)"
        }
    ]
}

[jakubdyszkiewicz]

I'm revisiting SPIFFE requirements https://github.com/spiffe/spiffe/blob/main/standards/X509-SVID.md

It seems that digitalSignature and keyEncipherment can only be used in leaf certs.
I think we could remove this validation and add information in docs that you are violating SPIFFE by using certs with it, but I'd like to understand why.

[jakubdyszkiewicz]

On splitting fields into cert, key, rootCert and chainCert. I may be biased since I've added intermediate CA support, but I don't think it's simpler.

Scenario 1

I want to just use Root CA without any intermediate CAs. Which fields should I fill? Just cert and key? Or should I repeat a cert in rootCert?

Scenario 2

I want to just Root CA with Intermediate CA. Which fields should I fill? cert is probably leaf CA, rootCert is root CA (but without any certs in between)? and chainCert is everything in between leaf CA and root CA or the whole chain?

With the current solution, you have just cert and key and you provide the whole "public" chain (regardless how many certs are there) in cert and key of leaf cert in key. I agree that cert could be called certChain or certBundle, I did not want to introduce breaking change.

@kumahq/kuma-maintainers please give feedback.

[nikita15p]

Digital signature is something which is commonly used in an enterprise subordinate ca. So if we disable this flag, it will block using those certs.
And for key encipherment, I saw some examples of generating intermediate ca where this is set. So wanted to remove this constraint.
Following snippet from in https://github.com/istio/istio/blob/da6178604559bdf2c707a57f452d16bee0de90c8/tools/certs/common.mk

%/intermediate.conf:
	@echo "[ req ]" > $@
	@echo "encrypt_key = no" >> $@
	@echo "prompt = no" >> $@
	@echo "utf8 = yes" >> $@
	@echo "default_md = sha256" >> $@
	@echo "default_bits = $(INTERMEDIATE_KEYSZ)" >> $@
	@echo "req_extensions = req_ext" >> $@
	@echo "x509_extensions = req_ext" >> $@
	@echo "distinguished_name = req_dn" >> $@
	@echo "[ req_ext ]" >> $@
	@echo "subjectKeyIdentifier = hash" >> $@
	@echo "basicConstraints = critical, CA:true, pathlen:0" >> $@
	@echo "keyUsage = critical, digitalSignature, nonRepudiation, keyEncipherment, keyCertSign" >> $@
	@echo "subjectAltName=@san" >> $@
	@echo "[ san ]" >> $@
	@echo "DNS.1 = $(INTERMEDIATE_SAN_DNS)" >> $@
	@echo "[ req_dn ]" >> $@
	@echo "O = $(INTERMEDIATE_ORG)" >> $@
	@echo "CN = $(INTERMEDIATE_CN)" >> $@
	@echo "L = $(L:/=)" >> $@

[jakubdyszkiewicz]

Ok, I'm fine with dropping validation for digitalSignature and keyEncipherment as long as we add information in docs that to be spiffe compliment, you cannot have this in cert you provide.

[codecov-commenter]

Codecov Report

Merging #2623 (9c3e640) into master (8e7bc99) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2623      +/-   ##
==========================================
- Coverage   51.97%   51.96%   -0.01%     
==========================================
  Files         871      871              
  Lines       49513    49509       -4     
==========================================
- Hits        25734    25727       -7     
+ Misses      21699    21697       -2     
- Partials     2080     2085       +5     

Impacted Files	Coverage Δ
pkg/plugins/ca/provided/ca_cert_validator.go	85.71% <ø> (-2.29%)	⬇️
pkg/core/resources/store/customizable_store.go	77.77% <0.00%> (-11.12%)	⬇️
pkg/plugins/resources/memory/store.go	77.71% <0.00%> (-4.35%)	⬇️
app/kumactl/cmd/root.go	70.17% <0.00%> (-3.51%)	⬇️
pkg/core/resources/manager/cache.go	81.81% <0.00%> (ø)
pkg/core/bootstrap/autoconfig.go	54.46% <0.00%> (+8.03%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8e7bc99...9c3e640. Read the comment docs.

[bartsmykla]

Ok, if the tests will pass in the next 20 minutes, we'll be able to ship it with the next release (1.3.0)

[jpeach]

With the current solution, you have just cert and key and you provide the whole "public" chain (regardless how many certs are there) in cert and key of leaf cert in key. I agree that cert could be called certChain or certBundle, I did not want to introduce breaking change.

+1 on this. The CA cert is a chain and we ought to accept a bundle of PEM certificates in that field.