Add a transport library. #358

kisom · 2015-10-01T21:07:00Z

The transport library provides tools for building TLS-secured client
and server connections. It is designed to minimise the number of knobs
and switches that are presented to end users, and supports features
such as auto-updating of certificates.

0xhaven · 2015-10-01T22:07:26Z

transport/ca/cfssl_provider.go

+	}
+
+	hosts := make([]string, 0, len(csr.DNSNames)+len(csr.IPAddresses))
+	for i := range csr.DNSNames {


copy(hosts, csr.DNSNames)

0xhaven · 2015-10-05T19:37:02Z

It's possible this should be under cfssl/helpers/transport

j-delaney · 2015-10-09T20:19:51Z

transport/kp/key_provider_test.go

+		t.Fatal("key provider shouldn't have a key yet")
+	}
+
+	err = kp.Generate("ecdsa", 256)


For better test coverage you could add a test for Generate with "rsa" as well

0xhaven · 2015-10-21T23:59:42Z

Rebase off master to fix CI issues...

0xhaven · 2015-10-27T18:22:43Z

transport/client.go

+
+// Dial initiates a TLS connection to an outbound server. It returns a
+// TLS connection to the server.
+func Dial(address string, tr *Transport) (*tls.Conn, error) {


Why not:

func (tr *Transport) Dial(address string) (*tls.Conn, error)

The `crypto/tls` library does it this way: it's not ``` func (cfg *Config) Dial... ``` I just tried to keep that style to cut down on having to remember what to use where.

The transport library provides tools for building TLS-secured client and server connections. It is designed to minimise the number of knobs and switches that are presented to end users, and supports features such as auto-updating of certificates.

The SHA384 GCM ciphersuites aren't supported in Go 1.4; instead of having version-specified suites, include them as manually-specified suites. The TLS package won't send ciphersuites it doesn't actually support, and there is no compile-time generation of the ciphersuite list.

TrustStore provides a mechanism for obtaining trusted roots. By default, it will use the system roots; otherwise, it will attempt to load certificates from a set of specified roots.

This commit simplifies the listener structure, and ensures that it satisfies the net.Listener interface.

* Move common functions to an example library. * Various clean ups: splitting things into separate functions and making it easier to extend the examples later. * Client sends a few messages this time, and the server will acknowledge them. In the future, a more extensive example might be useful.

This README explains how to bootstrap a CFSSL for use in the examples, how to run the server, and how to run the clients.

Add configurations to demonstrate the case where the remote CFSSL requires authentication.

This is mostly commentary adjustments via https://travis-ci.org/cloudflare/cfssl/jobs/87769654.

grittygrease · 2015-10-28T00:03:10Z

transport/ca/cert_provider.go

@@ -0,0 +1,16 @@
+// Package ca provides the CertificateAuthority interface for the
+// transport package, which provide an interface to get a CSR signed


* Remove unused core/config.go file. Users should just use the `Dial` and `Listen` functions in the `transport` package. * Fix spelling and grammar issues. * Clean up formatting on JSON file.

lmb · 2015-10-29T18:55:02Z

transport/ca/cfssl_provider.go

+var ErrNoAuth = errors.New("transport: authentication is required for non-local remotes")
+
+var v4Loopback = net.IPNet{
+	IP:   net.IP{127, 0, 0, 1},


Threw me for a second. Maybe make it net.IP{127, 0, 0, 0}?

* Notation change in specifying local IPv4 network. * Explicitly mark AutoUpdate channels as write-only. * Add a recovery and associated critical log message in the event an autoupdate goroutine panics.

* Use "client" as the profile name for consistency with the authenticated version. * Add an error channel to server to demonstrate its use.

@jkroll

This was requested by @jkroll. Also adds package documentation.

lmb · 2015-10-29T21:30:15Z

transport/client.go

+// New builds a new transport from an identity and a before time. The
+// before time tells the transport how long before the certificate
+// expires to start attempting to update when auto-updating.
+func New(before time.Duration, identity *core.Identity) (*Transport, error) {


What happens if before > lifetime of a freshly issued cert?

If the auto updater is running, every time it checks the time (e.g. every PollInterval seconds) it will request a new certificate.

lmb · 2015-10-29T21:47:01Z

Questions on certificate rollover, all in the context of very long running connections:

Does a server have to react to an updated cert by resetting connections?
Does a client have to AutoUpdate?
If yes, should a client restart connections as well on a new certificate?

Otherwise a pleasure to read!

grittygrease · 2015-10-29T22:33:22Z

Re: certificate rollover questions, the certificate is only used at the establishment of a connection so long-running connections can have the certificate swapped out from under them with no danger. Clients should auto-update when their certificate is close to expiration. The same logic applies, no restart needed.

Question #1: "What happens if before > lifetime of a freshly issued cert?" Question #2: "Questions on certificate rollover, all in the context of very long running connections: * Does a server have to react to an updated cert by resetting connections? * Does a client have to AutoUpdate? * If yes, should a client restart connections as well on a new certificate?" Rekey was renamed to RefreshKeys, and the documentation now reflects this.

kisom · 2015-10-29T22:40:31Z

@lmb Answered these questions in the latest commit (so that they're also in the documentation now), but the gist is:

If AutoUpdate is running, no additional actions have to be taken. Existing connections are fine, new connections will use the new certificate.
Clients don't need AutoUpdate if they're only making a single connection, but otherwise they should run it.
If before > the issuance life time of a certificate, every time an update check is done, the package will request a new certificate.

grittygrease · 2015-10-30T18:58:38Z

Looks good to merge.

lziest · 2015-11-02T22:02:04Z

LGTM

ashishgandhi · 2015-11-02T22:23:57Z

transport/client.go

+				errChan <- err
+			}
+
+			<-time.After(5 * time.Minute)


Exponential back-off?

Added in transport/core/backoff.go

ashishgandhi · 2015-11-03T21:25:46Z

👍

Add a transport library.

kisom force-pushed the kyle/transport branch from c1d0d85 to 2e688a3 Compare October 1, 2015 21:51

0xhaven reviewed Oct 1, 2015
View reviewed changes

kisom force-pushed the kyle/transport branch from 2e688a3 to 0f8e034 Compare October 1, 2015 22:33

j-delaney reviewed Oct 9, 2015
View reviewed changes

kisom force-pushed the kyle/transport branch from 4a697df to f4a346e Compare October 23, 2015 23:50

0xhaven reviewed Oct 27, 2015
View reviewed changes

kisom force-pushed the kyle/transport branch from 2dac1e9 to e2b2ed5 Compare October 27, 2015 19:55

kisom and others added 11 commits October 27, 2015 14:28

Add a transport library.

01e8715

The transport library provides tools for building TLS-secured client and server connections. It is designed to minimise the number of knobs and switches that are presented to end users, and supports features such as auto-updating of certificates.

Fix ignored file issue.

503d06f

Implement TrustStore.

0c8d6d3

TrustStore provides a mechanism for obtaining trusted roots. By default, it will use the system roots; otherwise, it will attempt to load certificates from a set of specified roots.

Update test to match new TrustStore.

dde1e36

Initial transport examples.

acae57a

Update Listener struct

cc1eb64

This commit simplifies the listener structure, and ensures that it satisfies the net.Listener interface.

Add a transport example README.

6b3d51b

This README explains how to bootstrap a CFSSL for use in the examples, how to run the server, and how to run the clients.

Add authenticated CFSSL example.

ff27854

Add configurations to demonstrate the case where the remote CFSSL requires authentication.

Address Zi's comment.

65c12ae

kisom force-pushed the kyle/transport branch from 5219a37 to 65c12ae Compare October 27, 2015 21:28

kisom force-pushed the kyle/transport branch from 9000070 to 113d742 Compare October 27, 2015 21:39

golint changes.

1209e80

This is mostly commentary adjustments via https://travis-ci.org/cloudflare/cfssl/jobs/87769654.

kisom force-pushed the kyle/transport branch from 113d742 to 1209e80 Compare October 27, 2015 21:46

grittygrease reviewed Oct 28, 2015
View reviewed changes

Fix notes from Nick.

b035c5e

* Remove unused core/config.go file. Users should just use the `Dial` and `Listen` functions in the `transport` package. * Fix spelling and grammar issues. * Clean up formatting on JSON file.

lmb reviewed Oct 29, 2015
View reviewed changes

kisom added 3 commits October 29, 2015 13:03

Address @lmb's comments.

04733ab

* Notation change in specifying local IPv4 network. * Explicitly mark AutoUpdate channels as write-only. * Add a recovery and associated critical log message in the event an autoupdate goroutine panics.

Clean up example code.

b910c79

* Use "client" as the profile name for consistency with the authenticated version. * Add an error channel to server to demonstrate its use.

Add support for loading roots from file.

08fb74a

This was requested by @jkroll. Also adds package documentation.

lmb reviewed Oct 29, 2015
View reviewed changes

ashishgandhi reviewed Nov 2, 2015
View reviewed changes

Add exponential backoff with jitter to AutoUpdate.

51e1ac7

grittygrease added a commit that referenced this pull request Nov 3, 2015

Merge pull request #358 from cloudflare/kyle/transport

f0a7d05

Add a transport library.

grittygrease merged commit f0a7d05 into master Nov 3, 2015

0xhaven deleted the kyle/transport branch November 10, 2015 22:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a transport library. #358

Add a transport library. #358

kisom commented Oct 1, 2015

0xhaven Oct 1, 2015

0xhaven commented Oct 5, 2015

j-delaney Oct 9, 2015

0xhaven commented Oct 21, 2015

0xhaven Oct 27, 2015

kisom Oct 27, 2015 via email

grittygrease Oct 28, 2015

lmb Oct 29, 2015

lmb Oct 29, 2015

kisom Oct 29, 2015

lmb commented Oct 29, 2015

grittygrease commented Oct 29, 2015

kisom commented Oct 29, 2015

grittygrease commented Oct 30, 2015

lziest commented Nov 2, 2015

ashishgandhi Nov 2, 2015

kisom Nov 3, 2015

ashishgandhi commented Nov 3, 2015

		@@ -0,0 +1,16 @@
		// Package ca provides the CertificateAuthority interface for the
		// transport package, which provide an interface to get a CSR signed

Add a transport library. #358

Add a transport library. #358

Conversation

kisom commented Oct 1, 2015

Choose a reason for hiding this comment

0xhaven commented Oct 5, 2015

Choose a reason for hiding this comment

0xhaven commented Oct 21, 2015

Choose a reason for hiding this comment

kisom Oct 27, 2015 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lmb commented Oct 29, 2015

grittygrease commented Oct 29, 2015

kisom commented Oct 29, 2015

grittygrease commented Oct 30, 2015

lziest commented Nov 2, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashishgandhi commented Nov 3, 2015