Merge branch 'main' into vault-token-local-paths-ce

builtin/logical/pki/acme_authorizations.go

@@ -20,6 +20,7 @@ type ACMEIdentifier struct {
 	Value         string             `json:"value"`
 	OriginalValue string             `json:"original_value"`
 	IsWildcard    bool               `json:"is_wildcard"`
+	IsV6IP        bool               `json:"is_v6_ip"`
 }
 
 func (ai *ACMEIdentifier) MaybeParseWildcard() (bool, string, error) {

builtin/logical/pki/acme_challenge_engine.go

@@ -7,6 +7,7 @@ import (
 	"container/list"
 	"context"
 	"fmt"
+	"strings"
 	"sync"
 	"time"
 
@@ -435,7 +436,9 @@ func (ace *ACMEChallengeEngine) _verifyChallenge(sc *storageContext, id string,
 			return ace._verifyChallengeCleanup(sc, err, id)
 		}
 
-		valid, err = ValidateHTTP01Challenge(authz.Identifier.Value, cv.Token, cv.Thumbprint, config)
+		domain := encodeIdentifierForHTTP01Challenge(authz.Identifier)
+
+		valid, err = ValidateHTTP01Challenge(domain, cv.Token, cv.Thumbprint, config)
 		if err != nil {
 			err = fmt.Errorf("%w: error validating http-01 challenge %v: %v; %v", ErrIncorrectResponse, id, err, ChallengeAttemptFailedMsg)
 			return ace._verifyChallengeRetry(sc, cv, authzPath, authz, challenge, err, id)
@@ -494,6 +497,27 @@ func (ace *ACMEChallengeEngine) _verifyChallenge(sc *storageContext, id string,
 	return ace._verifyChallengeCleanup(sc, nil, id)
 }
 
+func encodeIdentifierForHTTP01Challenge(identifier *ACMEIdentifier) string {
+	if !(identifier.Type == ACMEIPIdentifier && identifier.IsV6IP) {
+		return identifier.Value
+	}
+
+	// If our IPv6 identifier has a zone we need to encode the % to %25 otherwise
+	// the http.Client won't properly use it. RFC6874 specifies how the zone
+	// identifier in the URI should be handled. In section 2:
+	//
+	//    According to URI syntax [RFC3986], "%" is always treated as
+	//    an escape character in a URI, so, according to the established URI
+	//    syntax [RFC3986] any occurrences of literal "%" symbols in a URI MUST
+	//    be percent-encoded and represented in the form "%25". Thus, the
+	//    scoped address fe80::a%en1 would appear in a URI as
+	//    http://[fe80::a%25en1].
+	escapedIPv6 := strings.Replace(identifier.Value, "%", "%25", 1)
+
+	// IPv6 addresses need to be surrounded by [] within URLs
+	return fmt.Sprintf("[%s]", escapedIPv6)
+}
+
 func (ace *ACMEChallengeEngine) _verifyChallengeRetry(sc *storageContext, cv *ChallengeValidation, authzPath string, auth *ACMEAuthorization, challenge *ACMEChallenge, verificationErr error, id string) (bool, time.Time, error) {
 	now := time.Now()
 	path := acmeValidationPrefix + id

builtin/logical/pki/acme_challenge_engine_test.go

@@ -0,0 +1,47 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: BUSL-1.1
+
+package pki
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// Test_encodeIdentifierForHTTP01Challenge validates the encoding behaviors of our identifiers
+// for the HTTP01 challenge. Basically properly encode the identifier for an HTTP request.
+func Test_encodeIdentifierForHTTP01Challenge(t *testing.T) {
+	tests := []struct {
+		name string
+		arg  *ACMEIdentifier
+		want string
+	}{
+		{
+			name: "dns",
+			arg:  &ACMEIdentifier{Type: ACMEDNSIdentifier, Value: "www.dadgarcorp.com"},
+			want: "www.dadgarcorp.com",
+		},
+		{
+			name: "ipv4",
+			arg:  &ACMEIdentifier{Type: ACMEIPIdentifier, Value: "192.168.1.1"},
+			want: "192.168.1.1",
+		},
+		{
+			name: "ipv6",
+			arg:  &ACMEIdentifier{Type: ACMEIPIdentifier, Value: "2001:0db8:0000:0000:0000:0000:0000:0068", IsV6IP: true},
+			want: "[2001:0db8:0000:0000:0000:0000:0000:0068]",
+		},
+		{
+			name: "ipv6-zoned",
+			arg:  &ACMEIdentifier{Type: ACMEIPIdentifier, Value: "fe80::1cc0:3e8c:119f:c2e1%ens18", IsV6IP: true},
+			want: "[fe80::1cc0:3e8c:119f:c2e1%25ens18]",
+		},
+	}
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equalf(t, tt.want, encodeIdentifierForHTTP01Challenge(tt.arg), "encodeIdentifierForHTTP01Challenge(%v)", tt.arg)
+		})
+	}
+}

builtin/logical/pki/path_acme_order.go

@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"net"
 	"net/http"
+	"net/netip"
 	"sort"
 	"strings"
 	"time"
@@ -960,10 +961,24 @@ func parseOrderIdentifiers(data map[string]interface{}) ([]*ACMEIdentifier, erro
 		switch typeStr {
 		case string(ACMEIPIdentifier):
 			identifier.Type = ACMEIPIdentifier
-			ip := net.ParseIP(valueStr)
-			if ip == nil {
+			ip, err := netip.ParseAddr(valueStr)
+			if err != nil {
 				return nil, fmt.Errorf("value argument (%s) failed validation: failed parsing as IP: %w", valueStr, ErrMalformed)
 			}
+			if ip.Is6() {
+				if len(ip.Zone()) > 0 {
+					// If we are given an identifier with a local zone that doesn't make much sense
+					// as zone's are specific to the sender not us. For now disallow, perhaps in the
+					// future we should simply drop the zone?
+					return nil, fmt.Errorf("value argument (%s) failed validation: IPv6 identifiers with zone information are not allowed: %w", valueStr, ErrMalformed)
+				}
+
+				// We should keep whatever formatting of the IPv6 address that came in according
+				// to RFC8738 Section 2:
+				// An identifier for the IPv6 address 2001:db8::1 would be formatted like so:
+				//   {"type": "ip", "value": "2001:db8::1"}
+				identifier.IsV6IP = true
+			}
 		case string(ACMEDNSIdentifier):
 			identifier.Type = ACMEDNSIdentifier
 
@@ -1010,6 +1025,10 @@ func parseOrderIdentifiers(data map[string]interface{}) ([]*ACMEIdentifier, erro
 		identifiers = append(identifiers, identifier)
 	}
 
+	if len(identifiers) == 0 {
+		return nil, fmt.Errorf("no parsed identifiers were found: %w", ErrMalformed)
+	}
+
 	return identifiers, nil
 }
 

builtin/logical/pki/path_acme_order_test.go

@@ -4,12 +4,15 @@
 package pki
 
 import (
+	"fmt"
 	"net"
+	"strings"
 	"testing"
 
 	"github.com/hashicorp/vault/builtin/logical/pki/issuing"
 	"github.com/hashicorp/vault/sdk/framework"
 	"github.com/hashicorp/vault/sdk/logical"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -96,6 +99,135 @@ func TestACME_ValidateIdentifiersAgainstRole(t *testing.T) {
 	}
 }
 
+// Test_parseOrderIdentifiers validates we convert ACME requests into proper ACMEIdentifiers
+func Test_parseOrderIdentifiers(t *testing.T) {
+	tests := []struct {
+		name    string
+		data    map[string]interface{}
+		want    *ACMEIdentifier
+		wantErr assert.ErrorAssertionFunc
+	}{
+		{
+			name: "ipv4",
+			data: map[string]interface{}{"type": "ip", "value": "192.168.1.1"},
+			want: &ACMEIdentifier{
+				Type:          ACMEIPIdentifier,
+				Value:         "192.168.1.1",
+				OriginalValue: "192.168.1.1",
+				IsWildcard:    false,
+				IsV6IP:        false,
+			},
+			wantErr: assert.NoError,
+		},
+		{
+			name: "ipv6",
+			data: map[string]interface{}{"type": "ip", "value": "2001:0:130F::9C0:876A:130B"},
+			want: &ACMEIdentifier{
+				Type:          ACMEIPIdentifier,
+				Value:         "2001:0:130F::9C0:876A:130B",
+				OriginalValue: "2001:0:130F::9C0:876A:130B",
+				IsWildcard:    false,
+				IsV6IP:        true,
+			},
+			wantErr: assert.NoError,
+		},
+		{
+			name: "ipv4-in-ipv6",
+			data: map[string]interface{}{"type": "ip", "value": "::ffff:192.168.1.1"},
+			want: &ACMEIdentifier{
+				Type:          ACMEIPIdentifier,
+				Value:         "::ffff:192.168.1.1",
+				OriginalValue: "::ffff:192.168.1.1",
+				IsWildcard:    false,
+				IsV6IP:        true,
+			},
+			wantErr: assert.NoError,
+		},
+		{
+			name: "dns",
+			data: map[string]interface{}{"type": "dns", "value": "dadgarcorp.com"},
+			want: &ACMEIdentifier{
+				Type:          ACMEDNSIdentifier,
+				Value:         "dadgarcorp.com",
+				OriginalValue: "dadgarcorp.com",
+				IsWildcard:    false,
+				IsV6IP:        false,
+			},
+			wantErr: assert.NoError,
+		},
+		{
+			name: "wildcard-dns",
+			data: map[string]interface{}{"type": "dns", "value": "*.dadgarcorp.com"},
+			want: &ACMEIdentifier{
+				Type:          ACMEDNSIdentifier,
+				Value:         "dadgarcorp.com",
+				OriginalValue: "*.dadgarcorp.com",
+				IsWildcard:    true,
+				IsV6IP:        false,
+			},
+			wantErr: assert.NoError,
+		},
+		{
+			name:    "ipv6-with-zone", // This is debatable if we should strip or fail
+			data:    map[string]interface{}{"type": "ip", "value": "fe80::1cc0:3e8c:119f:c2e1%ens18"},
+			wantErr: ErrorContains("IPv6 identifiers with zone information are not allowed"),
+		},
+		{
+			name:    "bad-dns-wildcard",
+			data:    map[string]interface{}{"type": "dns", "value": "*192.168.1.1"},
+			wantErr: ErrorContains("invalid wildcard"),
+		},
+		{
+			name:    "ip-in-dns",
+			data:    map[string]interface{}{"type": "dns", "value": "192.168.1.1"},
+			wantErr: ErrorContains("parsed OK as IP address"),
+		},
+		{
+			name:    "empty-identifiers",
+			data:    nil,
+			wantErr: ErrorContains("no parsed identifiers were found"),
+		},
+	}
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			identifiers := map[string]interface{}{"identifiers": []interface{}{}}
+			if tt.data != nil {
+				identifiers["identifiers"] = append(identifiers["identifiers"].([]interface{}), tt.data)
+			}
+			got, err := parseOrderIdentifiers(identifiers)
+			if !tt.wantErr(t, err, fmt.Sprintf("parseOrderIdentifiers(%v)", tt.data)) {
+				return
+			} else if err != nil {
+				// If we passed the test above and an error was set no point in testing below
+				return
+			}
+
+			require.Len(t, got, 1, "expected a single return value")
+			acmeId := got[0]
+			require.Equal(t, tt.want.Type, acmeId.Type)
+			require.Equal(t, tt.want.Value, acmeId.Value)
+			require.Equal(t, tt.want.OriginalValue, acmeId.OriginalValue)
+			require.Equal(t, tt.want.IsWildcard, acmeId.IsWildcard)
+			require.Equal(t, tt.want.IsV6IP, acmeId.IsV6IP)
+		})
+	}
+}
+
+func ErrorContains(errMsg string) assert.ErrorAssertionFunc {
+	return func(t assert.TestingT, err error, i ...interface{}) bool {
+		if err == nil {
+			return assert.Fail(t, "expected error got none", i...)
+		}
+
+		if !strings.Contains(err.Error(), errMsg) {
+			return assert.Fail(t, fmt.Sprintf("error did not contain '%s':\n%+v", errMsg, err), i...)
+		}
+
+		return true
+	}
+}
+
 func _buildACMEIdentifiers(values ...string) []*ACMEIdentifier {
 	var identifiers []*ACMEIdentifier
 

changelog/28698.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+ui: No longer running decodeURIComponent on KVv2 list view allowing percent encoded data-octets in path name.
+```

changelog/28718.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+secrets/pki: Address issue with ACME HTTP-01 challenges failing for IPv6 IPs due to improperly formatted URLs
+```

ui/app/app.js

@@ -21,6 +21,7 @@ export default class App extends Application {
           'namespace',
           { 'app-router': 'router' },
           'store',
+          'pagination',
           'version',
           'custom-messages',
         ],
@@ -60,6 +61,7 @@ export default class App extends Application {
           'path-help',
           { 'app-router': 'router' },
           'store',
+          'pagination',
           'version',
           'secret-mount-path',
         ],
@@ -78,7 +80,14 @@ export default class App extends Application {
     },
     ldap: {
       dependencies: {
-        services: [{ 'app-router': 'router' }, 'store', 'secret-mount-path', 'flash-messages', 'auth'],
+        services: [
+          { 'app-router': 'router' },
+          'store',
+          'pagination',
+          'secret-mount-path',
+          'flash-messages',
+          'auth',
+        ],
         externalRoutes: {
           secrets: 'vault.cluster.secrets.backends',
         },
@@ -95,6 +104,7 @@ export default class App extends Application {
           { 'app-router': 'router' },
           'secret-mount-path',
           'store',
+          'pagination',
           'version',
         ],
         externalRoutes: {
@@ -114,6 +124,7 @@ export default class App extends Application {
           { 'app-router': 'router' },
           'secret-mount-path',
           'store',
+          'pagination',
           'version',
         ],
         externalRoutes: {
@@ -125,7 +136,7 @@ export default class App extends Application {
     },
     sync: {
       dependencies: {
-        services: ['flash-messages', 'flags', { 'app-router': 'router' }, 'store', 'version'],
+        services: ['flash-messages', 'flags', { 'app-router': 'router' }, 'store', 'pagination', 'version'],
         externalRoutes: {
           kvSecretOverview: 'vault.cluster.secrets.backend.kv.secret.index',
           clientCountOverview: 'vault.cluster.clients',

ui/app/components/console/ui-panel.js

@@ -28,7 +28,7 @@ export default Component.extend({
   console: service(),
   router: service(),
   controlGroup: service(),
-  store: service(),
+  pagination: service(),
   'data-test-component': 'console/ui-panel',
   attributeBindings: ['data-test-component'],
 
@@ -108,7 +108,7 @@ export default Component.extend({
     const currentRoute = owner.lookup(`router:main`).currentRouteName;
 
     try {
-      this.store.clearDataset();
+      this.pagination.clearDataset();
       yield this.router.transitionTo(currentRoute);
       this.logAndOutput(null, { type: 'success', content: 'The current screen has been refreshed!' });
     } catch (error) {

ui/app/components/generated-item-list.js

@@ -26,13 +26,13 @@ import { tracked } from '@glimmer/tracking';
 
 export default class GeneratedItemList extends Component {
   @service router;
-  @service store;
+  @service pagination;
   @tracked itemToDelete = null;
 
   @action
   refreshItemList() {
     const route = getOwner(this).lookup(`route:${this.router.currentRouteName}`);
-    this.store.clearDataset();
+    this.pagination.clearDataset();
     route.refresh();
   }
 }