[v14] Fix gRPC connections being disconnected regardless of Disconnec…

…tCertExpiry (#43292) * Fix gRPC connections being disconnected regardless of DisconnectCertExpiry * Remove log lines * Add test * back out the changes that added Authorizer
gravitational · Jun 20, 2024 · bb39c9d · bb39c9d
1 parent 7caed92
commit bb39c9d
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 4 deletions.
diff --git a/lib/auth/transport_credentials.go b/lib/auth/transport_credentials.go
@@ -179,8 +179,11 @@ func newTimeoutConn(conn net.Conn, clock clockwork.Clock, expires time.Time) (ne
 	}
 
 	return &timeoutConn{
-		Conn:  conn,
-		timer: clock.AfterFunc(expires.Sub(clock.Now()), func() { conn.Close() }),
+		Conn: conn,
+		timer: clock.AfterFunc(expires.Sub(clock.Now()), func() {
+			log.Debug("Closing gRPC connection due to certificate expiry")
+			conn.Close()
+		}),
 	}, nil
 }
 

diff --git a/lib/authz/permissions.go b/lib/authz/permissions.go
@@ -265,7 +265,12 @@ func (c *Context) GetDisconnectCertExpiry(authPref types.AuthPreference) time.Ti
 	// See https://github.com/gravitational/teleport/issues/18544
 
 	// If the session doesn't need to be disconnected on cert expiry just return the default value.
-	if c.Checker != nil && !c.Checker.AdjustDisconnectExpiredCert(authPref.GetDisconnectExpiredCert()) {
+	disconnectExpiredCert := authPref.GetDisconnectExpiredCert()
+	if c.Checker != nil {
+		disconnectExpiredCert = c.Checker.AdjustDisconnectExpiredCert(disconnectExpiredCert)
+	}
+
+	if !disconnectExpiredCert {
 		return time.Time{}
 	}
 

diff --git a/lib/authz/permissions_test.go b/lib/authz/permissions_test.go
@@ -51,12 +51,14 @@ func TestGetDisconnectExpiredCertFromIdentity(t *testing.T) {
 		name                    string
 		expires                 time.Time
 		previousIdentityExpires time.Time
+		checker                 services.AccessChecker
 		mfaVerified             bool
 		disconnectExpiredCert   bool
 		expected                time.Time
 	}{
 		{
 			name:                    "mfa overrides expires when set",
+			checker:                 &fakeCtxChecker{},
 			expires:                 now,
 			previousIdentityExpires: inAnHour,
 			mfaVerified:             true,
@@ -65,18 +67,36 @@ func TestGetDisconnectExpiredCertFromIdentity(t *testing.T) {
 		},
 		{
 			name:                  "expires returned when mfa unset",
+			checker:               &fakeCtxChecker{},
 			expires:               now,
 			mfaVerified:           false,
 			disconnectExpiredCert: true,
 			expected:              now,
 		},
 		{
 			name:                    "unset when disconnectExpiredCert is false",
+			checker:                 &fakeCtxChecker{},
 			expires:                 now,
 			previousIdentityExpires: inAnHour,
 			mfaVerified:             true,
 			disconnectExpiredCert:   false,
 		},
+		{
+			name:                  "no expiry returned when checker nil and disconnectExpiredCert false",
+			checker:               nil,
+			expires:               now,
+			mfaVerified:           false,
+			disconnectExpiredCert: false,
+			expected:              time.Time{},
+		},
+		{
+			name:                  "expiry returned when checker nil and disconnectExpiredCert true",
+			checker:               nil,
+			expires:               now,
+			mfaVerified:           false,
+			disconnectExpiredCert: true,
+			expected:              now,
+		},
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			var mfaVerified string
@@ -92,7 +112,7 @@ func TestGetDisconnectExpiredCertFromIdentity(t *testing.T) {
 			authPref := types.DefaultAuthPreference()
 			authPref.SetDisconnectExpiredCert(test.disconnectExpiredCert)
 
-			ctx := Context{Checker: &fakeCtxChecker{}, Identity: WrapIdentity(identity)}
+			ctx := Context{Checker: test.checker, Identity: WrapIdentity(identity)}
 
 			got := ctx.GetDisconnectCertExpiry(authPref)
 			require.Equal(t, test.expected, got)