Fix more TODOs

cmd/authwrapper/main.go

@@ -15,7 +15,6 @@ func main() {
 		log.Fatalf(": %v", err)
 	}
 
-	// TODO: Default to port if nothing has been set
 	if config.SSHCaKeyPath != "" && config.SSHSigningServerAddress != "" {
 		caPublickey, err := startSigningServer(
 			config.SSHCaKeyPath,

cmd/authwrapper/setup.go

@@ -39,7 +39,7 @@ func parseEnvironment() (*Config, error) {
 	config := &Config{
 		Command:                 os.Getenv("WRAP_COMMAND"),
 		Args:                    os.Args[1:],
-		RequestedPrincipals:     strings.Split(os.Getenv("PRINCIPALS"), ","),
+		RequestedPrincipals:     strings.Split(os.Getenv("SSH_PRINCIPALS"), ","),
 		SSHKeyPath:              os.Getenv("SSH_KEY_PATH"),
 		SSHKeyPassword:          os.Getenv("SSH_KEY_PASSWORD"),
 		SSHSigningServerURL:     os.Getenv("SSH_SIGNING_SERVER_URL"),
@@ -88,7 +88,12 @@ func parseEnvironment() (*Config, error) {
 		}
 	}
 
-	// TODO: Do more config error validation
+	if config.SSHSigningServerAddress != "" || config.SSHCaAuthorizedKeysPath != "" || config.SSHCaKeyPath != "" {
+		if config.SSHSigningServerAddress == "" || config.SSHCaAuthorizedKeysPath == "" || config.SSHCaKeyPath == "" {
+			return nil, fmt.Errorf("SSH_CA_KEY_PATH, SSH_CA_AUTHORIZED_KEYS_PATH, SSH_SIGNING_SERVER_LISTEN_ADDRESS needs to be provided")
+		}
+
+	}
 
 	if config.SSHSigningServerURL != "" && len(config.RequestedPrincipals) == 0 {
 		return nil, fmt.Errorf("When SSH_SIGNING_SERVER_URL is set a list of principals needs to be provided")
@@ -162,19 +167,25 @@ func setupKeyring(config *Config) (agent.ExtendedAgent, error) {
 	}
 
 	if config.SSHSigningServerURL != "" {
-		// TODO: Do something with the errors
 		var errors []error
 		for _, signer := range signers {
 			userCert, err := fetchUserCert(config.SSHSigningServerURL, signer.Signer, config.Command, config.Args, config.RequestedPrincipals)
 			if err != nil {
-				errors = append(errors, err)
+				errors = append(errors, fmt.Errorf("fetchUserCert for %s failed: %v", signer.Comment, err))
 				continue
 			}
 			certificates = append(certificates, sshagent.SSHCertificate{
 				Certificate: userCert,
-				Comment:     "key " + config.SSHKeyPath,
+				Comment:     "key " + signer.Comment,
 			})
 		}
+		if len(certificates) == 0 {
+			errStr := ""
+			for _, err := range errors {
+				errStr += err.Error()
+			}
+			return nil, fmt.Errorf("Failed to fetch a user cert:\n" + errStr)
+		}
 	}
 
 	return sshagent.NewSSHAlgorithmSignerKeyring(signers, certificates)

kms/google/kms-signer.go

@@ -1,7 +1,5 @@
 package google
 
-// TODO: Move to google kms package instead
-
 import (
 	"context"
 	"crypto"

server/main.go

@@ -85,13 +85,28 @@ func (s *SigningServer) VerifyCertificateRequest(certRequest *CertificateRequest
 	if err != nil {
 		return nil, err
 	}
-	// TODO: Check if challenge expired
 
+	now := time.Now().UTC()
+
+	// Check if challenge expired
+	issueTimeStamp, err := time.Parse(time.RFC3339Nano, value.Timestamp)
+	if err != nil {
+		return nil, err
+	}
+	if issueTimeStamp.After(now.Add(30 * time.Second)) {
+		return nil, fmt.Errorf("challenge expired")
+	}
+
+	// Fetch key from allowed map
 	allowedKey := s.allowedKeysMap[certRequest.PublicKey]
 	if allowedKey == nil {
 		return nil, nil
 	}
-	// TODO: Check if allowedKey is expired?
+
+	// Disallowed if the key expired
+	if allowedKey.ExpiresAt.Before(now) {
+		return nil, fmt.Errorf("key expired")
+	}
 
 	payload := GenerateSigningPayload(certRequest)
 
@@ -149,7 +164,7 @@ func (s *SigningServer) GenerateChallenge() (challenge *Challenge, err error) {
 	}
 
 	jsonBytes, err := json.Marshal(&challengeValue{
-		Timestamp: time.Now().UTC().Format("2006-01-02T15:04:05.999Z"),
+		Timestamp: time.Now().UTC().Format(time.RFC3339Nano),
 		Random:    randomBytes,
 	})
 

server/utils.go

@@ -4,6 +4,7 @@ import (
 	"crypto/rand"
 	"fmt"
 	"strings"
+	"time"
 
 	"golang.org/x/crypto/ssh"
 )
@@ -22,6 +23,7 @@ func GenerateRamdomBytes(length int) (value []byte, err error) {
 type AllowedKey struct {
 	Index       int
 	Key         ssh.PublicKey
+	ExpiresAt   time.Time
 	Comment     string
 	Principals  []string
 	ValidBefore uint64
@@ -36,6 +38,10 @@ func ParseAuthorizedKeys(lines []string) ([]AllowedKey, error) {
 	// http://man7.org/linux/man-pages/man8/sshd.8.html#AUTHORIZED_KEYS_FILE_FORMAT
 	// https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.certkeys
 	for i, line := range lines {
+		if strings.HasPrefix(line, "#") {
+			continue
+		}
+
 		publicKey, comment, options, _, err := ssh.ParseAuthorizedKey([]byte(line))
 		if err != nil {
 			return nil, fmt.Errorf("failed to parse line '%s': %v", line, err)
@@ -44,7 +50,8 @@ func ParseAuthorizedKeys(lines []string) ([]AllowedKey, error) {
 		key := AllowedKey{
 			Index:       i,
 			Key:         publicKey,
-			ValidBefore: ssh.CertTimeInfinity, // TODO: Set a better value  // uint64(time.Now().Add(time.Minute * 60).Unix()),
+			ExpiresAt:   time.Unix(1<<63-62135596801, 999999999), // MaxTime
+			ValidBefore: uint64(time.Now().Add(time.Minute * 60).Unix()),
 			Comment:     comment,
 			Principals:  []string{},
 			Options:     map[string]string{},
@@ -64,10 +71,16 @@ func ParseAuthorizedKeys(lines []string) ([]AllowedKey, error) {
 			case "agent-forwarding":
 				key.Extensions["permit-agent-forwarding"] = value
 			case "command":
-				// TODO: Don't allow empty commands
+				if value == "" {
+					return nil, fmt.Errorf("empty command not allowed")
+				}
 				key.Options["force-command"] = value
 			case "expiry-time":
-				// TODO: Use this to ignore key after date
+				expiresAt, err := time.Parse("2006010215040599", value)
+				if err != nil {
+					return nil, fmt.Errorf("expiry-time not valid format %s", value)
+				}
+				key.ExpiresAt = expiresAt
 			case "from":
 				// TODO: Convert wildcard matching to CIDR address/masklen notation
 				key.Options["source-address"] = value