From 9e5762fe063b663e9e852d3a4a15f9cae7adb4b9 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Wed, 11 Aug 2021 11:50:54 -0700 Subject: [PATCH 1/2] Allow the reuse of azure token if DisableTrustOnFirstUse is true Azure caches tokens for 24h and we cannot issue a new certificate for the same instance in that period of time. The meaning of this parameter is to allow the signing of multiple certificate in one instance. This is possible in GCP, because we get a new token, and is possible in AWS because we can generate a new one. On Azure there was no other way to do it unless you wait for 24h. Fixes #656 --- authority/authorize.go | 3 +++ authority/provisioner/azure.go | 5 +++-- authority/provisioner/azure_test.go | 2 +- authority/provisioner/provisioner.go | 12 ++++++++++++ authority/tls.go | 2 +- 5 files changed, 20 insertions(+), 4 deletions(-) diff --git a/authority/authorize.go b/authority/authorize.go index 8d1f878a..69ad2a90 100644 --- a/authority/authorize.go +++ b/authority/authorize.go @@ -173,6 +173,9 @@ func (a *Authority) AuthorizeAdminToken(r *http.Request, token string) (*linkedc } // UseToken stores the token to protect against reuse. +// +// This method currently ignores any error coming from the GetTokenID, but it +// should specifically ignore the error provisioner.ErrAllowTokenReuse. func (a *Authority) UseToken(token string, prov provisioner.Interface) error { if reuseKey, err := prov.GetTokenID(token); err == nil { if reuseKey == "" { diff --git a/authority/provisioner/azure.go b/authority/provisioner/azure.go index 230f246f..fee50658 100644 --- a/authority/provisioner/azure.go +++ b/authority/provisioner/azure.go @@ -131,9 +131,10 @@ func (p *Azure) GetTokenID(token string) (string, error) { return "", errors.Wrap(err, "error verifying claims") } - // If TOFU is disabled create return the token kid + // If TOFU is disabled then allow token re-use. Azure caches the token for + // 24h and without allowing the re-use we cannot use it twice. if p.DisableTrustOnFirstUse { - return claims.ID, nil + return "", ErrAllowTokenReuse } sum := sha256.Sum256([]byte(claims.XMSMirID)) diff --git a/authority/provisioner/azure_test.go b/authority/provisioner/azure_test.go index f21a5676..8033d345 100644 --- a/authority/provisioner/azure_test.go +++ b/authority/provisioner/azure_test.go @@ -72,7 +72,7 @@ func TestAzure_GetTokenID(t *testing.T) { wantErr bool }{ {"ok", p1, args{t1}, w1, false}, - {"ok no TOFU", p2, args{t2}, "the-jti", false}, + {"ok no TOFU", p2, args{t2}, "", true}, {"fail token", p1, args{"bad-token"}, "", true}, {"fail claims", p1, args{"eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.ey.fooo"}, "", true}, } diff --git a/authority/provisioner/provisioner.go b/authority/provisioner/provisioner.go index 83cc6946..75fabed5 100644 --- a/authority/provisioner/provisioner.go +++ b/authority/provisioner/provisioner.go @@ -4,6 +4,7 @@ import ( "context" "crypto/x509" "encoding/json" + stderrors "errors" "net/url" "regexp" "strings" @@ -32,6 +33,17 @@ type Interface interface { AuthorizeSSHRekey(ctx context.Context, token string) (*ssh.Certificate, []SignOption, error) } +// ErrAllowTokenReuse is an error that is returned by provisioners that allows +// the reuse of tokens. +// +// This is for example returned by the Azure provisioner when +// DisableTrustOnFirstUse is set to true. For AWS and GCP DisableTrustOnFirst +// use means that we allow the re-use of a token coming from a specific +// instance, but in these providers we can always get a new token, but because +// Azure caches the token for up to 24h we should add a mechanism to allow the +// re-use. +var ErrAllowTokenReuse = stderrors.New("allow token reuse") + // Audiences stores all supported audiences by request type. type Audiences struct { Sign []string diff --git a/authority/tls.go b/authority/tls.go index 4c3420df..a3dd95d3 100644 --- a/authority/tls.go +++ b/authority/tls.go @@ -366,7 +366,7 @@ func (a *Authority) Revoke(ctx context.Context, revokeOpts *RevokeOptions) error } rci.ProvisionerID = p.GetID() rci.TokenID, err = p.GetTokenID(revokeOpts.OTT) - if err != nil { + if err != nil && !errors.Is(err, provisioner.ErrAllowTokenReuse) { return errs.Wrap(http.StatusInternalServerError, err, "authority.Revoke; could not get ID for token") } From d4ae267addc45ad7b4d9009b218f52f0933a2c89 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Wed, 11 Aug 2021 14:59:26 -0700 Subject: [PATCH 2/2] Fix ErrAllowTokenReuse comment. --- authority/provisioner/provisioner.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/authority/provisioner/provisioner.go b/authority/provisioner/provisioner.go index 75fabed5..652cb888 100644 --- a/authority/provisioner/provisioner.go +++ b/authority/provisioner/provisioner.go @@ -36,12 +36,12 @@ type Interface interface { // ErrAllowTokenReuse is an error that is returned by provisioners that allows // the reuse of tokens. // -// This is for example returned by the Azure provisioner when -// DisableTrustOnFirstUse is set to true. For AWS and GCP DisableTrustOnFirst -// use means that we allow the re-use of a token coming from a specific -// instance, but in these providers we can always get a new token, but because -// Azure caches the token for up to 24h we should add a mechanism to allow the -// re-use. +// This is, for example, returned by the Azure provisioner when +// DisableTrustOnFirstUse is set to true. Azure caches tokens for up to 24hr and +// has no mechanism for getting a different token - this can be an issue when +// rebooting a VM. In contrast, AWS and GCP have facilities for requesting a new +// token. Therefore, for the Azure provisioner we are enabling token reuse, with +// the understanding that we are not following security best practices var ErrAllowTokenReuse = stderrors.New("allow token reuse") // Audiences stores all supported audiences by request type.