Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ APP_REPO=device-management-toolkit/console
# AES-192 or AES-256) and must not be repetitive — Console refuses to start
# otherwise. Generate one with: openssl rand -base64 24
APP_ENCRYPTION_KEY=
# Offers non-forward-secret RSA cipher suites to older AMT generations, both on
# the CIRA listener (:4433) and on outbound WSMAN connections. Leave false

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a bit too much for folks to understand. Something like for older AMT devices set this to true if they fail to connect. Same for the config comment, folks dont understand what a non-forward-secret RSA has to do with console or AMT.

# unless a device fails to negotiate TLS without them.
APP_ALLOW_INSECURE_CIPHERS=false
APP_COMMON_NAME=console.local
APP_DISABLE_CIRA=true
Expand Down
3 changes: 3 additions & 0 deletions config/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ app:
repo: device-management-toolkit/console
version: DEVELOPMENT
encryption_key: ""
# Offers non-forward-secret RSA cipher suites to older AMT generations, both
# on the CIRA listener (:4433) and on outbound WSMAN connections. Leave false
# unless a device fails to negotiate TLS without them.
allow_insecure_ciphers: false
http:
host: localhost
Expand Down
2 changes: 1 addition & 1 deletion internal/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ func setupCIRAServer(cfg *config.Config, log logger.Interface, closer io.Closer,
ciraCertFile := fmt.Sprintf("config/%s_cert.pem", cfg.CommonName)
ciraKeyFile := fmt.Sprintf("config/%s_key.pem", cfg.CommonName)

ciraServer, err := cira.NewServer(ciraCertFile, ciraKeyFile, usecases.Devices, log)
ciraServer, err := cira.NewServer(ciraCertFile, ciraKeyFile, cfg.AllowInsecureCiphers, usecases.Devices, log)
Comment thread
nbmaiti marked this conversation as resolved.
if err != nil {
_ = closer.Close()

Expand Down
79 changes: 50 additions & 29 deletions internal/controller/tcp/cira/tunnel.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,37 +22,47 @@ import (
)

const (
maxIdleTime = 300 * time.Second
port = "4433"
readBufferSize = 4096
weakCipherSuiteCount = 3
keepAliveInterval = 30
keepAliveTimeout = 90
apfSessionTimeout = 3 * time.Second
maxIdleTime = 300 * time.Second
port = "4433"
readBufferSize = 4096
keepAliveInterval = 30
keepAliveTimeout = 90
apfSessionTimeout = 3 * time.Second
)

// ErrChannelOpenFailed is returned when an APF channel open request fails.
var ErrChannelOpenFailed = errors.New("channel open failed")

// insecureCipherSuites are RSA key-exchange suites that some older AMT
// generations require. They lack perfect forward secrecy, so they are only
// offered when the operator opts in via APP_ALLOW_INSECURE_CIPHERS.
var insecureCipherSuites = []uint16{
tls.TLS_RSA_WITH_AES_128_GCM_SHA256,
tls.TLS_RSA_WITH_AES_128_CBC_SHA,
tls.TLS_RSA_WITH_AES_256_CBC_SHA,
}

type Server struct {
certificates tls.Certificate
notify chan error
listener net.Listener
devices devices.Feature
log logger.Interface
certificates tls.Certificate
notify chan error
listener net.Listener
devices devices.Feature
log logger.Interface
allowInsecureCiphers bool
}

func NewServer(certFile, keyFile string, d devices.Feature, l logger.Interface) (*Server, error) {
func NewServer(certFile, keyFile string, allowInsecureCiphers bool, d devices.Feature, l logger.Interface) (*Server, error) {
cert, err := tls.LoadX509KeyPair(certFile, keyFile)
if err != nil {
return nil, err
}

s := &Server{
certificates: cert,
notify: make(chan error, 1),
devices: d,
log: l,
certificates: cert,
notify: make(chan error, 1),
devices: d,
log: l,
allowInsecureCiphers: allowInsecureCiphers,
}

s.start()
Expand All @@ -73,6 +83,28 @@ func (s *Server) Notify() <-chan error {
return s.notify
}

// cipherSuites returns the TLS 1.2 cipher suites the listener offers to AMT
// devices. Only Go's secure defaults are advertised unless the operator has
// explicitly enabled the non-forward-secret RSA suites needed by older AMT
// firmware. TLS 1.3 suites are not configurable and are always negotiated.
func (s *Server) cipherSuites() []uint16 {
defaultCipherSuites := tls.CipherSuites()
suites := make([]uint16, 0, len(defaultCipherSuites)+len(insecureCipherSuites))

for _, suite := range defaultCipherSuites {
suites = append(suites, suite.ID)
}

if !s.allowInsecureCiphers {
return suites
}

s.log.Warn("CIRA server - insecure cipher suites enabled for AMT compatibility; " +
"these lack perfect forward secrecy. Disable APP_ALLOW_INSECURE_CIPHERS unless required.")

return append(suites, insecureCipherSuites...)
}

func (s *Server) ListenAndServe() error {
config := &tls.Config{
Certificates: []tls.Certificate{s.certificates},
Expand All @@ -81,18 +113,7 @@ func (s *Server) ListenAndServe() error {
MinVersion: tls.VersionTLS12,
}

defaultCipherSuites := tls.CipherSuites()
config.CipherSuites = make([]uint16, 0, len(defaultCipherSuites)+weakCipherSuiteCount)

for _, suite := range defaultCipherSuites {
config.CipherSuites = append(config.CipherSuites, suite.ID)
}
// add the weak cipher suites for AMT device compatibility
config.CipherSuites = append(config.CipherSuites,
tls.TLS_RSA_WITH_AES_128_GCM_SHA256,
tls.TLS_RSA_WITH_AES_128_CBC_SHA,
tls.TLS_RSA_WITH_AES_256_CBC_SHA,
)
config.CipherSuites = s.cipherSuites()

listener, err := tls.Listen("tcp", ":"+port, config)
if err != nil {
Expand Down
51 changes: 51 additions & 0 deletions internal/controller/tcp/cira/tunnel_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cira

import (
"crypto/tls"
"errors"
"net"
"testing"
Expand Down Expand Up @@ -222,6 +223,56 @@ func verifyConnectionRemoved(t *testing.T, authenticated bool, deviceID string)
// fakeConn is a minimal net.Conn implementation for tests.
type fakeConn struct{ net.Conn }

func TestServer_cipherSuites(t *testing.T) {
t.Parallel()

secureIDs := make([]uint16, 0, len(tls.CipherSuites()))
for _, suite := range tls.CipherSuites() {
secureIDs = append(secureIDs, suite.ID)
}

t.Run("insecure suites are not advertised by default", func(t *testing.T) {
t.Parallel()

s := &Server{log: logger.New("error")}

suites := s.cipherSuites()

assert.Equal(t, secureIDs, suites)

for _, insecure := range insecureCipherSuites {
assert.NotContains(t, suites, insecure, "insecure suite must be opt-in")
}
})

t.Run("insecure suites are advertised when explicitly allowed", func(t *testing.T) {
t.Parallel()

s := &Server{log: logger.New("error"), allowInsecureCiphers: true}

suites := s.cipherSuites()

want := make([]uint16, 0, len(secureIDs)+len(insecureCipherSuites))
want = append(want, secureIDs...)
want = append(want, insecureCipherSuites...)

assert.Equal(t, want, suites)
})

t.Run("every opt-in suite is one Go considers insecure", func(t *testing.T) {
t.Parallel()

goInsecure := make([]uint16, 0, len(tls.InsecureCipherSuites()))
for _, suite := range tls.InsecureCipherSuites() {
goInsecure = append(goInsecure, suite.ID)
}

for _, suite := range insecureCipherSuites {
assert.Contains(t, goInsecure, suite)
}
})
}

func TestConnectionContext_registerDevice(t *testing.T) {
t.Parallel()

Expand Down
Loading