Skip to content

Commit 928d977

Browse files
Change username format, enforce identity format (#802)
* Change username format, enforce identity format This updates the username type to avoid the username subject format looking like an email. Fulcio will now specify the subject in the OtherName SAN, and the format will use a ! instead of @. This required some custom ASN.1 marshalling and unmarshalling, since crypto/x509 does not support the OtherName SAN. This also adds enforcement that email subjects match a basic email regex format, and that other types do not look like emails. Fixes #716 Signed-off-by: Hayden Blauzvern <[email protected]>
1 parent aeee899 commit 928d977

File tree

14 files changed

+496
-44
lines changed

14 files changed

+496
-44
lines changed

docs/oid-info.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,11 @@ This contains the `ref` claim from the GitHub OIDC Identity token that contains
5252
the git ref that the workflow run was based upon.
5353
[(docs)][github-oidc-doc]
5454

55+
### 1.3.6.1.4.1.57264.1.7 | OtherName SAN
56+
57+
This specifies the username identity in the OtherName Subject Alternative Name, as
58+
defined by [RFC5280 4.2.1.6](https://datatracker.ietf.org/doc/html/rfc5280#section-4.2.1.6).
59+
5560
<!-- References -->
5661
[github-oidc-doc]: https://docs.github.com/en/actions/deployment/security-hardening-your-deployments/about-security-hardening-with-openid-connect#understanding-the-oidc-token
5762
[oid-link]: http://oid-info.com/get/1.3.6.1.4.1.57264

docs/oidc.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,4 +163,4 @@ Additionally, the configuration must include `SubjectDomain`, for example `examp
163163

164164
* The issuer in the configuration must partially match the domain in the configuration. The top level domain and second level domain must match. The user who updates the Fulcio configuration must also have control over both the issuer and domain configuration fields (Verified either manually or through an ACME-style challenge).
165165

166-
`SubjectDomain` is appended to `sub` to form an email, `sub@SubjectDomain`, and included as a SAN email address.
166+
`SubjectDomain` is appended to `sub` to form an identity, `sub!SubjectDomain`, and included as an OtherName SAN.

go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ require (
66
cloud.google.com/go/security v1.8.0
77
github.com/PaesslerAG/jsonpath v0.1.1
88
github.com/ThalesIgnite/crypto11 v1.2.5
9+
github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d
910
github.com/coreos/go-oidc/v3 v3.4.0
1011
github.com/fsnotify/fsnotify v1.5.4
1112
github.com/goadesign/goa v2.2.5+incompatible

go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,8 @@ github.com/armon/go-radix v1.0.0 h1:F4z6KzEeeQIMeLFa97iZU6vupzoecKdU5TX24SNppXI=
181181
github.com/armon/go-radix v1.0.0/go.mod h1:ufUuZ+zHj4x4TnLV4JWEpy2hxWSpsRywHrMgIH9cCH8=
182182
github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs=
183183
github.com/aryann/difflib v0.0.0-20170710044230-e206f873d14a/go.mod h1:DAHtR1m6lCRdSC2Tm3DSWRPvIPr6xNKyeHdqDQSQT+A=
184+
github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d h1:Byv0BzEl3/e6D5CLfI0j/7hiIEtvGVFPCZ7Ei2oq8iQ=
185+
github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d/go.mod h1:WaHUgvxTVq04UNunO+XhnAqY/wQc+bxr74GqbsZ/Jqw=
184186
github.com/aws/aws-lambda-go v1.13.3/go.mod h1:4UKl9IzQMoD+QF79YdCuzCwp8VbmG4VAQwij/eHl5CU=
185187
github.com/aws/aws-sdk-go v1.15.27/go.mod h1:mFuSZ37Z9YOHbQEwBWztmVzqXrEkub65tZoCYDt7FT0=
186188
github.com/aws/aws-sdk-go v1.19.18/go.mod h1:KmX6BPdI08NWTb3/sm4ZGu5ShLoqVDhKgpiN924inxo=

pkg/certificate/extensions.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ var (
2727
OIDGitHubWorkflowName = asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 4}
2828
OIDGitHubWorkflowRepository = asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 5}
2929
OIDGitHubWorkflowRef = asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 6}
30+
OIDOtherName = asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 7}
3031
)
3132

3233
// Extensions contains all custom x509 extensions defined by Fulcio

pkg/identity/email/principal.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@ import (
1818
"context"
1919
"crypto/x509"
2020
"errors"
21+
"fmt"
2122

23+
"github.com/asaskevich/govalidator"
2224
"github.com/coreos/go-oidc/v3/oidc"
2325
"github.com/sigstore/fulcio/pkg/certificate"
2426
"github.com/sigstore/fulcio/pkg/config"
@@ -40,6 +42,10 @@ func PrincipalFromIDToken(ctx context.Context, token *oidc.IDToken) (identity.Pr
4042
return nil, errors.New("email_verified claim was false")
4143
}
4244

45+
if !govalidator.IsEmail(emailAddress) {
46+
return nil, fmt.Errorf("email address is not valid")
47+
}
48+
4349
cfg, ok := config.FromContext(ctx).GetIssuer(token.Issuer)
4450
if !ok {
4551
return nil, errors.New("invalid configuration for OIDC ID Token issuer")

pkg/identity/email/principal_test.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,25 @@ func TestPrincipalFromIDToken(t *testing.T) {
144144
},
145145
WantErr: true,
146146
},
147+
`Invalid email address should error`: {
148+
Claims: map[string]interface{}{
149+
"aud": "sigstore",
150+
"iss": "https://iss.example.com",
151+
"sub": "doesntmatter",
152+
"email": "foo.com",
153+
"email_verified": true,
154+
},
155+
Config: config.FulcioConfig{
156+
OIDCIssuers: map[string]config.OIDCIssuer{
157+
"https://iss.example.com": {
158+
IssuerURL: "https://iss.example.com",
159+
Type: config.IssuerTypeEmail,
160+
ClientID: "sigstore",
161+
},
162+
},
163+
},
164+
WantErr: true,
165+
},
147166
`No issuer configured for token`: {
148167
Claims: map[string]interface{}{
149168
"aud": "sigstore",

pkg/identity/uri/principal.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"fmt"
2222
"net/url"
2323

24+
"github.com/asaskevich/govalidator"
2425
"github.com/coreos/go-oidc/v3/oidc"
2526
"github.com/sigstore/fulcio/pkg/certificate"
2627
"github.com/sigstore/fulcio/pkg/config"
@@ -40,6 +41,10 @@ func PrincipalFromIDToken(ctx context.Context, token *oidc.IDToken) (identity.Pr
4041
return nil, errors.New("invalid configuration for OIDC ID Token issuer")
4142
}
4243

44+
if govalidator.IsEmail(uriWithSubject) {
45+
return nil, fmt.Errorf("uri subject should not be an email address")
46+
}
47+
4348
// The subject hostname must exactly match the subject domain from the configuration
4449
uSubject, err := url.Parse(uriWithSubject)
4550
if err != nil {

pkg/identity/uri/principal_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@ func TestPrincipalFromIDToken(t *testing.T) {
4747
Token: &oidc.IDToken{Issuer: "https://notaccounts.example.com", Subject: "https://example.com/users/1"},
4848
WantErr: true,
4949
},
50+
`Subject as an email address should error`: {
51+
Token: &oidc.IDToken{Issuer: "https://accounts.example.com", Subject: "[email protected]"},
52+
WantErr: true,
53+
},
5054
`Incorrect subject domain hostname should error`: {
5155
Token: &oidc.IDToken{Issuer: "https://accounts.example.com", Subject: "https://notexample.com/users/1"},
5256
WantErr: true,

pkg/identity/username/othername.go

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
// Copyright 2022 The Sigstore Authors.
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+
package username
16+
17+
import (
18+
"crypto/x509/pkix"
19+
"encoding/asn1"
20+
"errors"
21+
"fmt"
22+
23+
"github.com/sigstore/fulcio/pkg/certificate"
24+
)
25+
26+
// OtherName describes a name related to a certificate which is not in one
27+
// of the standard name formats. RFC 5280, 4.2.1.6:
28+
//
29+
// OtherName ::= SEQUENCE {
30+
// type-id OBJECT IDENTIFIER,
31+
// value [0] EXPLICIT ANY DEFINED BY type-id }
32+
//
33+
// OtherName for Fulcio-issued certificates only supports UTF-8 strings as values.
34+
type OtherName struct {
35+
ID asn1.ObjectIdentifier
36+
Value string `asn1:"utf8,explicit,tag:0"`
37+
}
38+
39+
// MarshalSANS creates a Subject Alternative Name extension
40+
// with an OtherName sequence. RFC 5280, 4.2.1.6:
41+
//
42+
// SubjectAltName ::= GeneralNames
43+
// GeneralNames ::= SEQUENCE SIZE (1..MAX) OF GeneralName
44+
// GeneralName ::= CHOICE {
45+
//
46+
// otherName [0] OtherName,
47+
// ... }
48+
func MarshalSANS(name string, critical bool) (*pkix.Extension, error) {
49+
var rawValues []asn1.RawValue
50+
o := OtherName{
51+
ID: certificate.OIDOtherName,
52+
Value: name,
53+
}
54+
bytes, err := asn1.MarshalWithParams(o, "tag:0")
55+
if err != nil {
56+
return nil, err
57+
}
58+
rawValues = append(rawValues, asn1.RawValue{FullBytes: bytes})
59+
60+
sans, err := asn1.Marshal(rawValues)
61+
if err != nil {
62+
return nil, err
63+
}
64+
return &pkix.Extension{
65+
Id: asn1.ObjectIdentifier{2, 5, 29, 17},
66+
Critical: critical,
67+
Value: sans,
68+
}, nil
69+
}
70+
71+
// UnmarshalSANs extracts a UTF-8 string from the OtherName
72+
// field in the Subject Alternative Name extension.
73+
func UnmarshalSANS(exts []pkix.Extension) (string, error) {
74+
var otherNames []string
75+
76+
for _, e := range exts {
77+
if !e.Id.Equal(asn1.ObjectIdentifier{2, 5, 29, 17}) {
78+
continue
79+
}
80+
81+
var seq asn1.RawValue
82+
rest, err := asn1.Unmarshal(e.Value, &seq)
83+
if err != nil {
84+
return "", err
85+
} else if len(rest) != 0 {
86+
return "", fmt.Errorf("trailing data after X.509 extension")
87+
}
88+
if !seq.IsCompound || seq.Tag != 16 || seq.Class != 0 {
89+
return "", asn1.StructuralError{Msg: "bad SAN sequence"}
90+
}
91+
92+
rest = seq.Bytes
93+
for len(rest) > 0 {
94+
var v asn1.RawValue
95+
rest, err = asn1.Unmarshal(rest, &v)
96+
if err != nil {
97+
return "", err
98+
}
99+
100+
// skip all GeneralName fields except OtherName
101+
if v.Tag != 0 {
102+
continue
103+
}
104+
105+
var other OtherName
106+
_, err := asn1.UnmarshalWithParams(v.FullBytes, &other, "tag:0")
107+
if err != nil {
108+
return "", fmt.Errorf("could not parse requested OtherName SAN: %v", err)
109+
}
110+
if !other.ID.Equal(certificate.OIDOtherName) {
111+
return "", fmt.Errorf("unexpected OID for OtherName, expected %v, got %v", certificate.OIDOtherName, other.ID)
112+
}
113+
otherNames = append(otherNames, other.Value)
114+
}
115+
}
116+
117+
if len(otherNames) == 0 {
118+
return "", errors.New("no OtherName found")
119+
}
120+
if len(otherNames) != 1 {
121+
return "", errors.New("expected only one OtherName")
122+
}
123+
124+
return otherNames[0], nil
125+
}

0 commit comments

Comments
 (0)