kubeadm: token must be validated before creation, deletion or usage.

Refs kubernetes/kubeadm#104

cmd/kubeadm/app/cmd/init.go

@@ -21,6 +21,7 @@ import (
 	"io"
 	"io/ioutil"
 	"path"
+	"strconv"
 
 	"github.com/renstrom/dedent"
 	"github.com/spf13/cobra"
@@ -207,9 +208,20 @@ func (i *Init) Run(out io.Writer) error {
 
 	// Exception:
 	if i.cfg.Discovery.Token != nil {
-		if err := kubemaster.PrepareTokenDiscovery(i.cfg.Discovery.Token); err != nil {
+		// Validate token
+		if valid, err := kubeadmutil.ValidateToken(i.cfg.Discovery.Token); valid == false {
 			return err
 		}
+
+		// Make sure there is at least one address
+		if len(i.cfg.Discovery.Token.Addresses) == 0 {
+			ip, err := netutil.ChooseHostInterface()
+			if err != nil {
+				return err
+			}
+			i.cfg.Discovery.Token.Addresses = []string{ip.String() + ":" + strconv.Itoa(kubeadmapiext.DefaultDiscoveryBindPort)}
+		}
+
 		if err := kubemaster.CreateTokenAuthFile(kubeadmutil.BearerToken(i.cfg.Discovery.Token)); err != nil {
 			return err
 		}

cmd/kubeadm/app/cmd/token.go

@@ -31,7 +31,7 @@ import (
 	kubemaster "k8s.io/kubernetes/cmd/kubeadm/app/master"
 	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
 	"k8s.io/kubernetes/pkg/api"
-	v1 "k8s.io/kubernetes/pkg/api/v1"
+	"k8s.io/kubernetes/pkg/api/v1"
 	"k8s.io/kubernetes/pkg/fields"
 	"k8s.io/kubernetes/pkg/kubectl"
 )
@@ -128,25 +128,17 @@ func RunCreateToken(out io.Writer, cmd *cobra.Command, tokenDuration time.Durati
 		return err
 	}
 
-	d := &kubeadmapi.TokenDiscovery{}
-	if token != "" {
-		parsedID, parsedSecret, err := kubeadmutil.ParseToken(token)
-		if err != nil {
-			return err
-		}
-		d.ID = parsedID
-		d.Secret = parsedSecret
-	}
-	err = kubeadmutil.GenerateTokenIfNeeded(d)
+	parsedID, parsedSecret, err := kubeadmutil.ParseToken(token)
 	if err != nil {
 		return err
 	}
+	td := &kubeadmapi.TokenDiscovery{ID: parsedID, Secret: parsedSecret}
 
-	err = kubeadmutil.UpdateOrCreateToken(client, d, tokenDuration)
+	err = kubeadmutil.UpdateOrCreateToken(client, td, tokenDuration)
 	if err != nil {
 		return err
 	}
-	fmt.Fprintln(out, kubeadmutil.BearerToken(d))
+	fmt.Fprintln(out, kubeadmutil.BearerToken(td))
 
 	return nil
 }
@@ -219,6 +211,10 @@ func RunListTokens(out io.Writer, errW io.Writer, cmd *cobra.Command) error {
 
 // RunDeleteToken removes a bootstrap token from the server.
 func RunDeleteToken(out io.Writer, cmd *cobra.Command, tokenId string) error {
+	if err := kubeadmutil.ParseTokenID(tokenId); err != nil {
+		return err
+	}
+
 	client, err := kubemaster.CreateClientFromFile(path.Join(kubeadmapi.GlobalEnvParams.KubernetesDir, "admin.conf"))
 	if err != nil {
 		return err

cmd/kubeadm/app/discovery/BUILD

@@ -21,6 +21,7 @@ go_library(
         "//cmd/kubeadm/app/discovery/https:go_default_library",
         "//cmd/kubeadm/app/discovery/token:go_default_library",
         "//cmd/kubeadm/app/node:go_default_library",
+        "//cmd/kubeadm/app/util:go_default_library",
         "//pkg/client/unversioned/clientcmd:go_default_library",
         "//pkg/client/unversioned/clientcmd/api:go_default_library",
         "//vendor:github.com/spf13/pflag",

cmd/kubeadm/app/discovery/discovery.go

@@ -23,6 +23,7 @@ import (
 
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 	kubenode "k8s.io/kubernetes/cmd/kubeadm/app/node"
+	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
 	"k8s.io/kubernetes/pkg/client/unversioned/clientcmd"
 	clientcmdapi "k8s.io/kubernetes/pkg/client/unversioned/clientcmd/api"
 )
@@ -64,10 +65,15 @@ func runHTTPSDiscovery(hd *kubeadmapi.HTTPSDiscovery) (*clientcmdapi.Config, err
 
 // runTokenDiscovery executes token-based discovery.
 func runTokenDiscovery(td *kubeadmapi.TokenDiscovery) (*clientcmdapi.Config, error) {
+	if valid, err := kubeadmutil.ValidateToken(td); valid == false {
+		return nil, err
+	}
+
 	clusterInfo, err := kubenode.RetrieveTrustedClusterInfo(td)
 	if err != nil {
 		return nil, err
 	}
+
 	cfg, err := kubenode.EstablishMasterConnection(td, clusterInfo)
 	if err != nil {
 		return nil, err

cmd/kubeadm/app/master/BUILD

@@ -38,7 +38,6 @@ go_library(
         "//pkg/util/uuid:go_default_library",
         "//vendor:github.com/blang/semver",
         "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1",
-        "//vendor:k8s.io/apimachinery/pkg/util/net",
         "//vendor:k8s.io/apimachinery/pkg/util/wait",
     ],
 )
@@ -50,13 +49,11 @@ go_test(
         "apiclient_test.go",
         "discovery_test.go",
         "manifests_test.go",
-        "tokens_test.go",
     ],
     library = ":go_default_library",
     tags = ["automanaged"],
     deps = [
         "//cmd/kubeadm/app/apis/kubeadm:go_default_library",
-        "//cmd/kubeadm/app/util:go_default_library",
         "//pkg/api/v1:go_default_library",
         "//pkg/util/intstr:go_default_library",
     ],

cmd/kubeadm/app/master/tokens.go

@@ -21,30 +21,12 @@ import (
 	"fmt"
 	"os"
 	"path"
-	"strconv"
 
-	netutil "k8s.io/apimachinery/pkg/util/net"
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
-	kubeadmapiext "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1alpha1"
-	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
 	"k8s.io/kubernetes/pkg/util/uuid"
 )
 
-func PrepareTokenDiscovery(d *kubeadmapi.TokenDiscovery) error {
-	if len(d.Addresses) == 0 {
-		ip, err := netutil.ChooseHostInterface()
-		if err != nil {
-			return err
-		}
-		d.Addresses = []string{ip.String() + ":" + strconv.Itoa(kubeadmapiext.DefaultDiscoveryBindPort)}
-	}
-	if err := kubeadmutil.GenerateTokenIfNeeded(d); err != nil {
-		return fmt.Errorf("failed to generate token(s) [%v]", err)
-	}
-	return nil
-}
-
 func CreateTokenAuthFile(bt string) error {
 	tokenAuthFilePath := path.Join(kubeadmapi.GlobalEnvParams.HostPKIPath, "tokens.csv")
 	if err := os.MkdirAll(kubeadmapi.GlobalEnvParams.HostPKIPath, 0700); err != nil {

cmd/kubeadm/app/master/tokens_test.go

@@ -1,61 +0,0 @@
-/*
-Copyright 2016 The Kubernetes Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package master
-
-import (
-	"testing"
-
-	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
-	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
-)
-
-func TestValidTokenPopulatesSecrets(t *testing.T) {
-	t.Run("provided", func(t *testing.T) {
-		expectedID := "123456"
-		expectedSecret := "0123456789abcdef"
-		s := &kubeadmapi.TokenDiscovery{
-			ID:     expectedID,
-			Secret: expectedSecret,
-		}
-
-		err := kubeadmutil.GenerateTokenIfNeeded(s)
-		if err != nil {
-			t.Errorf("GenerateTokenIfNeeded gave an error for a valid token: %v", err)
-		}
-		if s.ID != expectedID {
-			t.Errorf("GenerateTokenIfNeeded did not populate the TokenID correctly; expected [%s] but got [%s]", expectedID, s.ID)
-		}
-		if s.Secret != expectedSecret {
-			t.Errorf("GenerateTokenIfNeeded did not populate the Token correctly; expected %v but got %v", expectedSecret, s.Secret)
-		}
-	})
-
-	t.Run("not provided", func(t *testing.T) {
-		s := &kubeadmapi.TokenDiscovery{}
-
-		err := kubeadmutil.GenerateTokenIfNeeded(s)
-		if err != nil {
-			t.Errorf("GenerateTokenIfNeeded gave an error for a valid token: %v", err)
-		}
-		if s.ID == "" {
-			t.Errorf("GenerateTokenIfNeeded did not populate the TokenID correctly; expected ID to be non-empty")
-		}
-		if s.Secret == "" {
-			t.Errorf("GenerateTokenIfNeeded did not populate the Token correctly; expected Secret to be non-empty")
-		}
-	})
-}

cmd/kubeadm/app/util/tokens.go

@@ -42,6 +42,13 @@ const (
 	tokenCreateRetries         = 5
 )
 
+var (
+	tokenIDRegexpString = "^([a-z0-9]{6})$"
+	tokenIDRegexp       = regexp.MustCompile(tokenIDRegexpString)
+	tokenRegexpString   = "^([a-z0-9]{6})\\:([a-z0-9]{16})$"
+	tokenRegexp         = regexp.MustCompile(tokenRegexpString)
+)
+
 func randBytes(length int) (string, error) {
 	b := make([]byte, length)
 	_, err := rand.Read(b)
@@ -70,30 +77,22 @@ func GenerateToken(d *kubeadmapi.TokenDiscovery) error {
 	return nil
 }
 
-var (
-	tokenRegexpString = "^([a-zA-Z0-9]{6})\\:([a-zA-Z0-9]{16})$"
-	tokenRegexp       = regexp.MustCompile(tokenRegexpString)
-)
-
-func GenerateTokenIfNeeded(d *kubeadmapi.TokenDiscovery) error {
-	ok, err := IsTokenValid(d)
-	if err != nil {
-		return err
+// ParseTokenID tries and parse a valid token ID from a string.
+// An error is returned in case of failure.
+func ParseTokenID(s string) error {
+	if !tokenIDRegexp.MatchString(s) {
+		return fmt.Errorf("token ID [%q] was not of form [%q]", s, tokenIDRegexpString)
 	}
-	if ok {
-		return nil
-	}
-	if err := GenerateToken(d); err != nil {
-		return err
-	}
-
 	return nil
+
 }
 
+// ParseToken tries and parse a valid token from a string.
+// A token ID and token secret are returned in case of success, an error otherwise.
 func ParseToken(s string) (string, string, error) {
 	split := tokenRegexp.FindStringSubmatch(s)
 	if len(split) != 3 {
-		return "", "", fmt.Errorf("token %q was not of form %q", s, tokenRegexpString)
+		return "", "", fmt.Errorf("token [%q] was not of form [%q]", s, tokenRegexpString)
 	}
 	return split[1], split[2], nil
 
@@ -104,10 +103,9 @@ func BearerToken(d *kubeadmapi.TokenDiscovery) string {
 	return fmt.Sprintf("%s:%s", d.ID, d.Secret)
 }
 
-func IsTokenValid(d *kubeadmapi.TokenDiscovery) (bool, error) {
-	if len(d.ID)+len(d.Secret) == 0 {
-		return false, nil
-	}
+// ValidateToken validates whether a token is well-formed.
+// In case it's not, the corresponding error is returned as well.
+func ValidateToken(d *kubeadmapi.TokenDiscovery) (bool, error) {
 	if _, _, err := ParseToken(d.ID + ":" + d.Secret); err != nil {
 		return false, err
 	}
@@ -131,8 +129,11 @@ func DiscoveryPort(d *kubeadmapi.TokenDiscovery) int32 {
 // UpdateOrCreateToken attempts to update a token with the given ID, or create if it does
 // not already exist.
 func UpdateOrCreateToken(client *clientset.Clientset, d *kubeadmapi.TokenDiscovery, tokenDuration time.Duration) error {
+	// Let's make sure
+	if valid, err := ValidateToken(d); !valid {
+		return err
+	}
 	secretName := fmt.Sprintf("%s%s", BootstrapTokenSecretPrefix, d.ID)
-
 	var lastErr error
 	for i := 0; i < tokenCreateRetries; i++ {
 		secret, err := client.Secrets(api.NamespaceSystem).Get(secretName, metav1.GetOptions{})
@@ -166,7 +167,11 @@ func UpdateOrCreateToken(client *clientset.Clientset, d *kubeadmapi.TokenDiscove
 		}
 
 	}
-	return fmt.Errorf("<util/tokens> unable to create bootstrap token after %d attempts [%v]", tokenCreateRetries, lastErr)
+	return fmt.Errorf(
+		"unable to create bootstrap token after %d attempts [%v]",
+		tokenCreateRetries,
+		lastErr,
+	)
 }
 
 func encodeTokenSecretData(d *kubeadmapi.TokenDiscovery, duration time.Duration) map[string][]byte {

cmd/kubeadm/app/util/tokens_test.go

@@ -31,6 +31,10 @@ func TestTokenParse(t *testing.T) {
 		// invalid separation
 		"123456:1234567890.123456",
 		"abcdef.1234567890123456",
+		// invalid token id
+		"Abcdef:1234567890123456",
+		// invalid token secret
+		"123456:AABBCCDDEEFFGGHH",
 	}
 
 	for _, token := range invalidTokens {
@@ -38,6 +42,71 @@ func TestTokenParse(t *testing.T) {
 			t.Errorf("ParseToken did not return an error for this invalid token: [%s]", token)
 		}
 	}
+
+	validTokens := []string{
+		"abcdef:1234567890123456",
+		"123456:aabbccddeeffgghh",
+	}
+
+	for _, token := range validTokens {
+		if _, _, err := ParseToken(token); err != nil {
+			t.Errorf("ParseToken returned an error for this valid token: [%s]", token)
+		}
+	}
+}
+
+func TestParseTokenID(t *testing.T) {
+	invalidTokenIDs := []string{
+		"",
+		"1234567890123456789012",
+		"12345",
+		"Abcdef",
+	}
+
+	for _, tokenID := range invalidTokenIDs {
+		if err := ParseTokenID(tokenID); err == nil {
+			t.Errorf("ParseTokenID did not return an error for this invalid token ID: [%q]", tokenID)
+		}
+	}
+
+	validTokens := []string{
+		"abcdef",
+		"123456",
+	}
+
+	for _, tokenID := range validTokens {
+		if err := ParseTokenID(tokenID); err != nil {
+			t.Errorf("ParseTokenID failed for a valid token ID [%q], err: %+v", tokenID, err)
+		}
+	}
+}
+
+func TestValidateToken(t *testing.T) {
+	invalidTokens := []*kubeadmapi.TokenDiscovery{
+		{ID: "", Secret: ""},
+		{ID: "1234567890123456789012", Secret: ""},
+		{ID: "", Secret: "1234567890123456789012"},
+		{ID: "12345", Secret: "1234567890123456"},
+		{ID: "Abcdef", Secret: "1234567890123456"},
+		{ID: "123456", Secret: "AABBCCDDEEFFGGHH"},
+	}
+
+	for _, token := range invalidTokens {
+		if valid, err := ValidateToken(token); valid == true || err == nil {
+			t.Errorf("ValidateToken did not return an error for this invalid token: [%s]", token)
+		}
+	}
+
+	validTokens := []*kubeadmapi.TokenDiscovery{
+		{ID: "abcdef", Secret: "1234567890123456"},
+		{ID: "123456", Secret: "aabbccddeeffgghh"},
+	}
+
+	for _, token := range validTokens {
+		if valid, err := ValidateToken(token); valid == false || err != nil {
+			t.Errorf("ValidateToken failed for a valid token [%s], valid: %t, err: %+v", token, valid, err)
+		}
+	}
 }
 
 func TestGenerateToken(t *testing.T) {