Skip to content

Commit

Permalink
log-forwarding: various fixes
Browse files Browse the repository at this point in the history 
 - remove the unnecessary "syslog-server-cert" config
 - drop the "standards/tls" package. It was providing
   little value, and performing incorrect validation
   that hampers use of real certificates
 - stop setting "ServerName" in the TLS config. We will
   validate whatever host is specified in syslog-host
 - simplify parsing/validation of syslog-host
 - drop the message-inspecting layer violation in
   environs/config, where it would depend on the contents
   of error messages from the logfwd/syslog package. The
   error messages are less helpful now, but the code is
   not long for this world
 - rename ClientCACert to CACert, fix comments to match
 - fixes to feature test:
    - Dial "localhost" instead of 127.0.0.1, because the
      test certs don't have an IP SAN
    - Initialise a replica set. Log tailing requires the
      replicaset oplog to work properly. We were getting
      lucky before, because the oplog is only needed if
      the log messages come after the workers start
    - Change the date of the expected message to be in
      the future, not in the past. Having them in the
      past appears to muck with tailing
  • Loading branch information
@axw
axw committed Jul 8, 2016
1 parent 387351e commit 434bb08
Show file tree
Hide file tree
Showing 17 changed files with 169 additions and 1,248 deletions.
38 changes: 3 additions & 35 deletions environs/config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,6 @@ const (
// LogFwdSyslogHost sets the hostname:port of the syslog server.
LogFwdSyslogHost = "syslog-host"

// LogFwdSyslogServerCert sets the expected server certificate for
// syslog forwarding.
LogFwdSyslogServerCert = "syslog-server-cert"

// LogFwdSyslogCACert sets the certificate of the CA that signed the syslog
// server certificate.
LogFwdSyslogCACert = "syslog-ca-cert"
Expand Down Expand Up @@ -448,24 +444,7 @@ func Validate(cfg, old *Config) error {

if lfCfg, ok := cfg.LogFwdSyslog(); ok {
if err := lfCfg.Validate(); err != nil {
// Clean up the error messages a bit.
msg := err.Error()
var field string
switch {
case strings.Contains(msg, "Host"):
field = LogFwdSyslogHost
case strings.Contains(msg, "ExpectedServerCert"):
field = LogFwdSyslogServerCert
case strings.Contains(msg, "ClientCACert"):
field = LogFwdSyslogCACert
case strings.Contains(msg, "ClientCert"):
field = LogFwdSyslogClientCert
case strings.Contains(msg, "ClientKey"):
field = LogFwdSyslogClientKey
default:
return errors.Annotate(err, "invalid syslog forwarding config")
}
return errors.Annotatef(errors.Cause(err), "invalid %q", field)
return errors.Annotate(err, "invalid syslog forwarding config")
}
}

Expand Down Expand Up @@ -699,14 +678,9 @@ func (c *Config) LogFwdSyslog() (*syslog.RawConfig, bool) {
lfCfg.Host = s.(string)
}

if s, ok := c.defined[LogFwdSyslogServerCert]; ok && s != "" {
partial = true
lfCfg.ExpectedServerCert = s.(string)
}

if s, ok := c.defined[LogFwdSyslogCACert]; ok && s != "" {
partial = true
lfCfg.ClientCACert = s.(string)
lfCfg.CACert = s.(string)
}

if s, ok := c.defined[LogFwdSyslogClientCert]; ok && s != "" {
Expand Down Expand Up @@ -1003,7 +977,6 @@ var alwaysOptional = schema.Defaults{
"bootstrap-retry-delay": schema.Omit,
"bootstrap-addresses-delay": schema.Omit,
LogFwdSyslogHost: schema.Omit,
LogFwdSyslogServerCert: schema.Omit,
LogFwdSyslogCACert: schema.Omit,
LogFwdSyslogClientCert: schema.Omit,
LogFwdSyslogClientKey: schema.Omit,
Expand Down Expand Up @@ -1416,13 +1389,8 @@ global or per instance security groups.`,
Type: environschema.Tstring,
Group: environschema.EnvironGroup,
},
LogFwdSyslogServerCert: {
Description: `The expected syslog server certificate in PEM format.`,
Type: environschema.Tstring,
Group: environschema.EnvironGroup,
},
LogFwdSyslogCACert: {
Description: `The certificate of the CA that signed the syslog certificate, in PEM format.`,
Description: `The certificate of the CA that signed the syslog server certificate, in PEM format.`,
Type: environschema.Tstring,
Group: environschema.EnvironGroup,
},
Expand Down
40 changes: 7 additions & 33 deletions environs/config/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -652,86 +652,67 @@ var configTests = []configTest{
"resource-tags": []string{"a"},
}),
err: `resource-tags: expected "key=value", got "a"`,
}, {
about: "Invalid syslog server cert",
useDefaults: config.UseDefaults,
attrs: minimalConfigAttrs.Merge(testing.Attrs{
"type": "my-type",
"name": "my-name",
"syslog-host": "localhost:1234",
"syslog-server-cert": invalidCACert,
"syslog-ca-cert": caCert,
"syslog-client-cert": caCert,
"syslog-client-key": caKey,
}),
err: `invalid "syslog-server-cert": asn1: syntax error: data truncated`,
}, {
about: "Invalid syslog ca cert format",
useDefaults: config.UseDefaults,
attrs: minimalConfigAttrs.Merge(testing.Attrs{
"type": "my-type",
"name": "my-name",
"syslog-host": "localhost:1234",
"syslog-server-cert": caCert2,
"syslog-ca-cert": "abc",
"syslog-client-cert": caCert,
"syslog-client-key": caKey,
}),
err: `invalid "syslog-ca-cert": no certificates found`,
err: `invalid syslog forwarding config: validating TLS config: parsing CA certificate: no certificates found`,
}, {
about: "Invalid syslog ca cert",
useDefaults: config.UseDefaults,
attrs: minimalConfigAttrs.Merge(testing.Attrs{
"type": "my-type",
"name": "my-name",
"syslog-host": "localhost:1234",
"syslog-server-cert": caCert2,
"syslog-ca-cert": invalidCACert,
"syslog-client-cert": caCert,
"syslog-client-key": caKey,
}),
err: `invalid "syslog-ca-cert": asn1: syntax error: data truncated`,
err: `invalid syslog forwarding config: validating TLS config: parsing CA certificate: asn1: syntax error: data truncated`,
}, {
about: "invalid syslog cert",
useDefaults: config.UseDefaults,
attrs: minimalConfigAttrs.Merge(testing.Attrs{
"syslog-host": "10.0.0.1:12345",
"syslog-server-cert": caCert2,
"syslog-ca-cert": caCert,
"syslog-client-cert": invalidCACert,
"syslog-client-key": caKey,
}),
err: `invalid "syslog-client-cert": asn1: syntax error: data truncated`,
err: `invalid syslog forwarding config: validating TLS config: parsing client key pair: asn1: syntax error: data truncated`,
}, {
about: "invalid syslog key",
useDefaults: config.UseDefaults,
attrs: minimalConfigAttrs.Merge(testing.Attrs{
"syslog-host": "10.0.0.1:12345",
"syslog-server-cert": caCert2,
"syslog-ca-cert": caCert,
"syslog-client-cert": caCert,
"syslog-client-key": invalidCAKey,
}),
err: `invalid "syslog-client-key": bad key or key does not match certificate: crypto/tls: failed to parse private key`,
err: `invalid syslog forwarding config: validating TLS config: parsing client key pair: crypto/tls: failed to parse private key`,
}, {
about: "Mismatched syslog cert and key",
useDefaults: config.UseDefaults,
attrs: minimalConfigAttrs.Merge(testing.Attrs{
"syslog-host": "10.0.0.1:12345",
"syslog-server-cert": caCert2,
"syslog-ca-cert": caCert,
"syslog-client-cert": caCert,
"syslog-client-key": caKey2,
}),
err: `invalid "syslog-client-key": bad key or key does not match certificate: crypto/tls: private key does not match public key`,
err: `invalid syslog forwarding config: validating TLS config: parsing client key pair: crypto/tls: private key does not match public key`,
}, {
about: "Valid syslog config values",
useDefaults: config.UseDefaults,
attrs: minimalConfigAttrs.Merge(testing.Attrs{
"type": "my-type",
"name": "my-name",
"syslog-host": "localhost:1234",
"syslog-server-cert": caCert2,
"syslog-ca-cert": testing.CACert,
"syslog-client-cert": testing.ServerCert,
"syslog-client-key": testing.ServerKey,
Expand Down Expand Up @@ -1007,19 +988,12 @@ func (test configTest) check(c *gc.C, home *gitjujutesting.FakeHome) {
}

lfCfg, hasLogCfg := cfg.LogFwdSyslog()
if v, ok := test.attrs["syslog-server-cert"].(string); v != "" {
c.Assert(hasLogCfg, jc.IsTrue)
c.Assert(lfCfg.ExpectedServerCert, gc.Equals, v)
} else if ok {
c.Assert(hasLogCfg, jc.IsTrue)
c.Check(lfCfg.ExpectedServerCert, gc.Equals, "")
}
if v, ok := test.attrs["syslog-ca-cert"].(string); v != "" {
c.Assert(hasLogCfg, jc.IsTrue)
c.Assert(lfCfg.ClientCACert, gc.Equals, v)
c.Assert(lfCfg.CACert, gc.Equals, v)
} else if ok {
c.Assert(hasLogCfg, jc.IsTrue)
c.Check(lfCfg.ClientCACert, gc.Equals, "")
c.Check(lfCfg.CACert, gc.Equals, "")
}
if v, ok := test.attrs["syslog-client-cert"].(string); v != "" {
c.Assert(hasLogCfg, jc.IsTrue)
Expand Down
47 changes: 39 additions & 8 deletions featuretests/syslog_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@ import (
"crypto/tls"
"crypto/x509"
"fmt"
"net"
"regexp"
"runtime"
"time"

"github.com/juju/loggo"
gitjujutesting "github.com/juju/testing"
jc "github.com/juju/testing/checkers"
"github.com/juju/utils/os"
"github.com/juju/utils/series"
Expand All @@ -27,6 +29,7 @@ import (
"github.com/juju/juju/testing/factory"
"github.com/juju/juju/version"
"github.com/juju/juju/worker/logsender"
"github.com/juju/juju/worker/peergrouper"
)

type syslogSuite struct {
Expand All @@ -39,15 +42,37 @@ type syslogSuite struct {

var _ = gc.Suite(&syslogSuite{})

func (s *syslogSuite) SetUpSuite(c *gc.C) {
// Tailing logs requires a replica set. Restart mongo with a
// replicaset before initialising AgentSuite.
mongod := gitjujutesting.MgoServer
mongod.Params = []string{"--replSet", "juju"}
mongod.Restart()

info := mongod.DialInfo()
args := peergrouper.InitiateMongoParams{
DialInfo: info,
MemberHostPort: mongod.Addr(),
}
err := peergrouper.InitiateMongoServer(args)
c.Assert(err, jc.ErrorIsNil)

s.AgentSuite.SetUpSuite(c)
s.AddCleanup(func(*gc.C) {
mongod.Params = nil
mongod.Restart()
})
}

func (s *syslogSuite) SetUpTest(c *gc.C) {
if runtime.GOOS != "linux" {
c.Skip(fmt.Sprintf("this test requires state server, therefore does not support %q", runtime.GOOS))
c.Skip(fmt.Sprintf("this test requires a controller, therefore does not support %q", runtime.GOOS))
}
currentSeries := series.HostSeries()
osFromSeries, err := series.GetOSFromSeries(currentSeries)
c.Assert(err, jc.ErrorIsNil)
if osFromSeries != os.Ubuntu {
c.Skip(fmt.Sprintf("this test requires state server, therefore does not support OS %q only Ubuntu", osFromSeries.String()))
c.Skip(fmt.Sprintf("this test requires a controller, therefore does not support OS %q only Ubuntu", osFromSeries.String()))
}
s.AgentSuite.SetUpTest(c)
// TODO(perrito666) 200160701:
Expand All @@ -56,14 +81,16 @@ func (s *syslogSuite) SetUpTest(c *gc.C) {
// This test should not need JujuConnSuite or AgentSuite.
s.fakeEnsureMongo = agenttest.InstallFakeEnsureMongo(s)

s.received = make(chan rfc5424test.Message, 1)
done := make(chan struct{})
s.received = make(chan rfc5424test.Message)
s.server = rfc5424test.NewServer(rfc5424test.HandlerFunc(func(msg rfc5424test.Message) {
select {
case s.received <- msg:
default:
case <-done:
}
}))
s.AddCleanup(func(*gc.C) { s.server.Close() })
s.AddCleanup(func(*gc.C) { close(done) })

serverCert, err := tls.X509KeyPair(
[]byte(coretesting.ServerCert),
Expand All @@ -80,9 +107,13 @@ func (s *syslogSuite) SetUpTest(c *gc.C) {
}
s.server.StartTLS()

// We must use "localhost", as the certificate does not
// have any IP SANs.
port := s.server.Listener.Addr().(*net.TCPAddr).Port
addr := net.JoinHostPort("localhost", fmt.Sprint(port))

err = s.State.UpdateModelConfig(map[string]interface{}{
"syslog-host": s.server.Listener.Addr().String(),
"syslog-server-cert": coretesting.ServerCert,
"syslog-host": addr,
"syslog-ca-cert": coretesting.CACert,
"syslog-client-cert": coretesting.ServerCert,
"syslog-client-key": coretesting.ServerKey,
Expand Down Expand Up @@ -161,10 +192,10 @@ func (s *syslogSuite) TestLogRecordForwarded(c *gc.C) {

// Ensure that a specific log record gets forwarded.
rec := s.newRecord("something happened!")
rec.Time = time.Date(2015, time.June, 1, 23, 2, 1, 23, time.UTC)
rec.Time = time.Date(2099, time.June, 1, 23, 2, 1, 23, time.UTC)
s.sendRecord(c, rec)
msg := s.popMessagesUntil(c, `something happened!`)
expected := `<11>1 2015-06-01T23:02:01.000000023Z machine-0.%s jujud-machine-agent-%s - - [origin enterpriseID="28978" sofware="jujud-machine-agent" swVersion="%s"][model@28978 controller-uuid="%s" model-uuid="%s"][log@28978 module="juju.featuretests.syslog" source="syslog_test.go:99999"] something happened!`
expected := `<11>1 2099-06-01T23:02:01.000000023Z machine-0.%s jujud-machine-agent-%s - - [origin enterpriseID="28978" sofware="jujud-machine-agent" swVersion="%s"][model@28978 controller-uuid="%s" model-uuid="%s"][log@28978 module="juju.featuretests.syslog" source="syslog_test.go:99999"] something happened!`
modelID := coretesting.ModelTag.Id()
ctlrID := modelID
c.Check(msg.Message, gc.Equals, fmt.Sprintf(expected, modelID, modelID[:28], version.Current, ctlrID, modelID))
Expand Down
18 changes: 7 additions & 11 deletions logfwd/syslog/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package syslog

import (
"crypto/tls"
"fmt"
"io"
"time"
Expand All @@ -14,7 +15,6 @@ import (
"github.com/juju/juju/logfwd"
"github.com/juju/juju/standards/rfc5424"
"github.com/juju/juju/standards/rfc5424/sdelements"
"github.com/juju/juju/standards/tls"
)

// Sender exposes the underlying functionality needed by Client.
Expand All @@ -27,14 +27,14 @@ type Sender interface {

// SenderOpener supports opening a syslog connection.
type SenderOpener interface {
DialFunc(cfg tls.Config, timeout time.Duration) (rfc5424.DialFunc, error)
DialFunc(cfg *tls.Config, timeout time.Duration) (rfc5424.DialFunc, error)

Open(host string, cfg rfc5424.ClientConfig, dial rfc5424.DialFunc) (Sender, error)
}

type senderOpener struct{}

func (senderOpener) DialFunc(cfg tls.Config, timeout time.Duration) (rfc5424.DialFunc, error) {
func (senderOpener) DialFunc(cfg *tls.Config, timeout time.Duration) (rfc5424.DialFunc, error) {
dial, err := rfc5424.TLSDialFunc(cfg, timeout)
return dial, errors.Trace(err)
}
Expand Down Expand Up @@ -76,15 +76,11 @@ func OpenForSender(cfg RawConfig, opener SenderOpener) (*Client, error) {
}

func open(cfg RawConfig, opener SenderOpener) (Sender, error) {
tlsCfg := tls.Config{
RawCert: tls.RawCert{
CertPEM: cfg.ClientCert,
KeyPEM: cfg.ClientKey,
CACertPEM: cfg.ClientCACert,
},
//ServerName: "",
ExpectedServerCertPEM: cfg.ExpectedServerCert,
tlsCfg, err := cfg.tlsConfig()
if err != nil {
return nil, errors.Annotate(err, "constructing TLS config")
}

var timeout time.Duration
dial, err := opener.DialFunc(tlsCfg, timeout)
if err != nil {
Expand Down
Loading

0 comments on commit 434bb08

Please sign in to comment.