Skip to content

Commit 2348149

Browse files
authored
Merge pull request #538 from docker/fix/keychain-null-collection-path
fix(keychain): reject null collection path on linux
2 parents 454441e + 8456689 commit 2348149

2 files changed

Lines changed: 124 additions & 2 deletions

File tree

store/keychain/keychain_linux.go

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,14 @@ const (
3838
// NOTE: do not use this directly, always call [getDefaultCollection]
3939
loginKeychainObjectPath = dbus.ObjectPath("/org/freedesktop/secrets/collection/login")
4040

41+
// the null/root object path returned by the secret service when an alias is
42+
// not assigned to any collection. It is syntactically valid (so
43+
// [dbus.ObjectPath.IsValid] returns true) but does not point at a real
44+
// collection, so it must be rejected explicitly.
45+
//
46+
// https://specifications.freedesktop.org/secret-service-spec/latest/org.freedesktop.Secret.Service.html#org.freedesktop.Secret.Service.ReadAlias
47+
nullObjectPath = dbus.ObjectPath("/")
48+
4149
// used to list all available collections on the secret service API
4250
//
4351
// https://specifications.freedesktop.org/secret-service-spec/latest/org.freedesktop.Secret.Service.html
@@ -54,6 +62,12 @@ const (
5462
secretServiceIsCollectionLockedProperty = "org.freedesktop.Secret.Collection.Locked"
5563
)
5664

65+
// errNoDefaultCollection is returned when the secret service has no usable
66+
// default collection (no 'login' collection and no collection assigned to the
67+
// 'default' alias). This typically happens on headless hosts where the keyring
68+
// has not been initialized.
69+
var errNoDefaultCollection = errors.New("no default keychain collection available")
70+
5771
// getDefaultCollection gets the secret service collection dbus object path.
5872
//
5973
// It prefers the loginKeychainObjectPath, since most users on X11 would have
@@ -84,11 +98,34 @@ func getDefaultCollection(service *kc.SecretService) (dbus.ObjectPath, error) {
8498
return "", err
8599
}
86100

87-
if !defaultKeychainObjectPath.IsValid() {
101+
return resolveDefaultCollection(collections, defaultKeychainObjectPath)
102+
}
103+
104+
// resolveDefaultCollection selects the collection to use given the available
105+
// collections and the object path returned by the 'default' alias lookup.
106+
//
107+
// It is split out from [getDefaultCollection] so the selection logic can be
108+
// unit tested without a live secret service over dbus.
109+
func resolveDefaultCollection(collections []dbus.ObjectPath, aliasPath dbus.ObjectPath) (dbus.ObjectPath, error) {
110+
// choose the 'login' collection if it exists
111+
if slices.Contains(collections, loginKeychainObjectPath) {
112+
return loginKeychainObjectPath, nil
113+
}
114+
115+
if !aliasPath.IsValid() {
88116
return "", errors.New("the default collection object path is invalid")
89117
}
90118

91-
return defaultKeychainObjectPath, nil
119+
// the secret service returns the null path '/' when no collection is
120+
// assigned to the 'default' alias. This is common on headless hosts where
121+
// neither the 'login' collection nor a 'default' alias has been set up.
122+
// The null path is syntactically valid (so IsValid above returns true) but
123+
// does not point at a real collection, so it must be rejected explicitly.
124+
if aliasPath == nullObjectPath {
125+
return "", errNoDefaultCollection
126+
}
127+
128+
return aliasPath, nil
92129
}
93130

94131
var errCollectionLocked = errors.New("collection is locked")
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
// Copyright 2026 Docker, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
//go:build linux
16+
17+
package keychain
18+
19+
import (
20+
"testing"
21+
22+
dbus "github.com/godbus/dbus/v5"
23+
"github.com/stretchr/testify/assert"
24+
"github.com/stretchr/testify/require"
25+
)
26+
27+
func TestResolveDefaultCollection(t *testing.T) {
28+
const customCollection = dbus.ObjectPath("/org/freedesktop/secrets/collection/custom")
29+
30+
tests := []struct {
31+
name string
32+
collections []dbus.ObjectPath
33+
aliasPath dbus.ObjectPath
34+
want dbus.ObjectPath
35+
wantErr error
36+
}{
37+
{
38+
name: "prefers login collection when present",
39+
collections: []dbus.ObjectPath{customCollection, loginKeychainObjectPath},
40+
// even if the alias points elsewhere, login wins
41+
aliasPath: customCollection,
42+
want: loginKeychainObjectPath,
43+
},
44+
{
45+
name: "falls back to default alias when login is absent",
46+
collections: []dbus.ObjectPath{customCollection},
47+
aliasPath: customCollection,
48+
want: customCollection,
49+
},
50+
{
51+
name: "rejects null path from unassigned default alias",
52+
// headless host: no login collection, no default alias set, so
53+
// ReadAlias returns the null object path "/"
54+
collections: []dbus.ObjectPath{},
55+
aliasPath: nullObjectPath,
56+
wantErr: errNoDefaultCollection,
57+
},
58+
{
59+
name: "rejects syntactically invalid alias path",
60+
collections: []dbus.ObjectPath{},
61+
aliasPath: dbus.ObjectPath(""),
62+
// distinct from the null-path case; see resolveDefaultCollection
63+
want: "",
64+
},
65+
}
66+
67+
for _, tt := range tests {
68+
t.Run(tt.name, func(t *testing.T) {
69+
got, err := resolveDefaultCollection(tt.collections, tt.aliasPath)
70+
71+
switch {
72+
case tt.wantErr != nil:
73+
assert.ErrorIs(t, err, tt.wantErr)
74+
assert.Empty(t, got)
75+
case tt.want == "":
76+
// invalid path case: an error is expected, value is empty
77+
require.Error(t, err)
78+
assert.Empty(t, got)
79+
default:
80+
require.NoError(t, err)
81+
assert.Equal(t, tt.want, got)
82+
}
83+
})
84+
}
85+
}

0 commit comments

Comments
 (0)