From f3656e80f072b7ef9335e2cb84bc2cbd7017d8fb Mon Sep 17 00:00:00 2001
From: Jeff Mitchell <jeff@hashicorp.com>
Date: Sat, 17 Mar 2018 21:29:17 -0400
Subject: [PATCH] Properly forward (or specifically don't) sys calls that
 result in read only errors (#4129)

Prior to this policy writes against a performance secondary would not
succeed because the read-only error was swallowed by handleError. In
addition to fixing this, it adds a similar function that explicitly
doesn't trigger forwarding. This is useful for things that are local to
the secondary such as raw operations and lease management.
---
 vault/logical_system.go | 36 +++++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/vault/logical_system.go b/vault/logical_system.go
index c632c8805bc5..9fd884db2baf 100644
--- a/vault/logical_system.go
+++ b/vault/logical_system.go
@@ -1075,7 +1075,7 @@ func (b *SystemBackend) handleTidyLeases(ctx context.Context, req *logical.Reque
 	err := b.Core.expiration.Tidy()
 	if err != nil {
 		b.Backend.Logger().Error("sys: failed to tidy leases", "error", err)
-		return handleError(err)
+		return handleErrorNoReadOnlyForward(err)
 	}
 	return nil, err
 }
@@ -1591,6 +1591,24 @@ func (b *SystemBackend) handleMount(ctx context.Context, req *logical.Request, d
 // used to intercept an HTTPCodedError so it goes back to callee
 func handleError(
 	err error) (*logical.Response, error) {
+	if strings.Contains(err.Error(), logical.ErrReadOnly.Error()) {
+		return logical.ErrorResponse(err.Error()), err
+	}
+	switch err.(type) {
+	case logical.HTTPCodedError:
+		return logical.ErrorResponse(err.Error()), err
+	default:
+		return logical.ErrorResponse(err.Error()), logical.ErrInvalidRequest
+	}
+}
+
+// Performs a similar function to handleError, but upon seeing a ReadOnlyError
+// will actually strip it out to prevent forwarding
+func handleErrorNoReadOnlyForward(
+	err error) (*logical.Response, error) {
+	if strings.Contains(err.Error(), logical.ErrReadOnly.Error()) {
+		return nil, fmt.Errorf("operation could not be completed as storage is read-only")
+	}
 	switch err.(type) {
 	case logical.HTTPCodedError:
 		return logical.ErrorResponse(err.Error()), err
@@ -1950,7 +1968,7 @@ func (b *SystemBackend) handleLeaseLookupList(ctx context.Context, req *logical.
 	keys, err := b.Core.expiration.idView.List(ctx, prefix)
 	if err != nil {
 		b.Backend.Logger().Error("sys: error listing leases", "prefix", prefix, "error", err)
-		return handleError(err)
+		return handleErrorNoReadOnlyForward(err)
 	}
 	return logical.ListResponse(keys), nil
 }
@@ -1975,7 +1993,7 @@ func (b *SystemBackend) handleRenew(ctx context.Context, req *logical.Request, d
 	resp, err := b.Core.expiration.Renew(leaseID, increment)
 	if err != nil {
 		b.Backend.Logger().Error("sys: lease renewal failed", "lease_id", leaseID, "error", err)
-		return handleError(err)
+		return handleErrorNoReadOnlyForward(err)
 	}
 	return resp, err
 }
@@ -1995,7 +2013,7 @@ func (b *SystemBackend) handleRevoke(ctx context.Context, req *logical.Request,
 	// Invoke the expiration manager directly
 	if err := b.Core.expiration.Revoke(leaseID); err != nil {
 		b.Backend.Logger().Error("sys: lease revocation failed", "lease_id", leaseID, "error", err)
-		return handleError(err)
+		return handleErrorNoReadOnlyForward(err)
 	}
 	return nil, nil
 }
@@ -2025,7 +2043,7 @@ func (b *SystemBackend) handleRevokePrefixCommon(
 	}
 	if err != nil {
 		b.Backend.Logger().Error("sys: revoke prefix failed", "prefix", prefix, "error", err)
-		return handleError(err)
+		return handleErrorNoReadOnlyForward(err)
 	}
 	return nil, nil
 }
@@ -2499,7 +2517,7 @@ func (b *SystemBackend) handleRawRead(ctx context.Context, req *logical.Request,
 
 	entry, err := b.Core.barrier.Get(ctx, path)
 	if err != nil {
-		return handleError(err)
+		return handleErrorNoReadOnlyForward(err)
 	}
 	if entry == nil {
 		return nil, nil
@@ -2511,7 +2529,7 @@ func (b *SystemBackend) handleRawRead(ctx context.Context, req *logical.Request,
 	// will be nil.
 	outputBytes, _, err := compressutil.Decompress(entry.Value)
 	if err != nil {
-		return handleError(err)
+		return handleErrorNoReadOnlyForward(err)
 	}
 
 	// `outputBytes` is nil if the input is uncompressed. In that case set it to the original input.
@@ -2563,7 +2581,7 @@ func (b *SystemBackend) handleRawDelete(ctx context.Context, req *logical.Reques
 	}
 
 	if err := b.Core.barrier.Delete(ctx, path); err != nil {
-		return handleError(err)
+		return handleErrorNoReadOnlyForward(err)
 	}
 	return nil, nil
 }
@@ -2585,7 +2603,7 @@ func (b *SystemBackend) handleRawList(ctx context.Context, req *logical.Request,
 
 	keys, err := b.Core.barrier.List(ctx, path)
 	if err != nil {
-		return handleError(err)
+		return handleErrorNoReadOnlyForward(err)
 	}
 	return logical.ListResponse(keys), nil
 }