diff --git a/acme/api/order.go b/acme/api/order.go index a0f49b78..7f9c30ab 100644 --- a/acme/api/order.go +++ b/acme/api/order.go @@ -10,18 +10,16 @@ import ( "strings" "time" - "go.step.sm/crypto/kms/uri" - "github.com/go-chi/chi/v5" "go.step.sm/crypto/randutil" "go.step.sm/crypto/x509util" "github.com/smallstep/certificates/acme" + "github.com/smallstep/certificates/acme/wire" "github.com/smallstep/certificates/api/render" "github.com/smallstep/certificates/authority/policy" "github.com/smallstep/certificates/authority/provisioner" - "github.com/smallstep/certificates/wire" ) // NewOrderRequest represents the body for a NewOrder request. @@ -52,16 +50,12 @@ func (n *NewOrderRequest) Validate() error { return acme.NewError(acme.ErrorMalformedType, "permanent identifier cannot be empty") } case acme.WireID: - orderValue, err := wire.ParseID([]byte(id.Value)) - if err != nil { - return acme.NewError(acme.ErrorMalformedType, "ID cannot be parsed") - } - clientIDURI, err := uri.Parse(orderValue.ClientID) + wireID, err := wire.ParseID([]byte(id.Value)) if err != nil { - return acme.NewError(acme.ErrorMalformedType, "invalid client ID, it's supposed to be a valid URI") + return acme.WrapError(acme.ErrorMalformedType, err, "failed parsing Wire ID") } - if clientIDURI.Scheme != "wireapp" { - return acme.NewError(acme.ErrorMalformedType, "invalid client ID scheme") + if _, err := wire.ParseClientID(wireID.ClientID); err != nil { + return acme.WrapError(acme.ErrorMalformedType, err, "invalid Wire client ID %q", wireID.ClientID) } default: return acme.NewError(acme.ErrorMalformedType, "identifier type unsupported: %s", id.Type) diff --git a/acme/api/order_test.go b/acme/api/order_test.go index 59c231ab..5f7afce2 100644 --- a/acme/api/order_test.go +++ b/acme/api/order_test.go @@ -96,14 +96,34 @@ func TestNewOrderRequest_Validate(t *testing.T) { err: acme.NewError(acme.ErrorMalformedType, "invalid IP address: %s", "192.168.42.1000"), } }, - "fail/bad-identifier/wireapp-prefix-mismatch": func(t *testing.T) test { + "fail/bad-identifier/wireapp-invalid-uri": func(t *testing.T) test { return test{ nor: &NewOrderRequest{ Identifiers: []acme.Identifier{ {Type: "wireapp-id", Value: "{}"}, }, }, - err: acme.NewError(acme.ErrorMalformedType, "invalid client ID, it's supposed to be a valid URI"), + err: acme.NewError(acme.ErrorMalformedType, `invalid Wire client ID "": invalid Wire client ID URI "": error parsing : scheme is missing`), + } + }, + "fail/bad-identifier/wireapp-wrong-scheme": func(t *testing.T) test { + return test{ + nor: &NewOrderRequest{ + Identifiers: []acme.Identifier{ + {Type: "wireapp-id", Value: `{"name": "Smith, Alice M (QA)", "domain": "example.com", "client-id": "nowireapp://example.com", "handle": "wireapp://%40alice.smith.qa@example.com"}`}, + }, + }, + err: acme.NewError(acme.ErrorMalformedType, `invalid Wire client ID "nowireapp://example.com": invalid Wire client ID scheme "nowireapp"; expected "wireapp"`), + } + }, + "fail/bad-identifier/wireapp-invalid-user-parts": func(t *testing.T) test { + return test{ + nor: &NewOrderRequest{ + Identifiers: []acme.Identifier{ + {Type: "wireapp-id", Value: `{"name": "Smith, Alice M (QA)", "domain": "example.com", "client-id": "wireapp://user-device@example.com", "handle": "wireapp://%40alice.smith.qa@example.com"}`}, + }, + }, + err: acme.NewError(acme.ErrorMalformedType, `invalid Wire client ID "wireapp://user-device@example.com": invalid Wire client ID username "user-device"`), } }, "ok": func(t *testing.T) test { @@ -184,7 +204,7 @@ func TestNewOrderRequest_Validate(t *testing.T) { naf: naf, } }, - "ok/wireapp-prefix": func(t *testing.T) test { + "ok/wireapp-idd": func(t *testing.T) test { nbf := time.Now().UTC().Add(time.Minute) naf := time.Now().UTC().Add(5 * time.Minute) return test{ @@ -780,12 +800,12 @@ func TestHandler_newAuthorization(t *testing.T) { az: az, } }, - "ok/im": func(t *testing.T) test { + "ok/wire": func(t *testing.T) test { az := &acme.Authorization{ AccountID: "accID", Identifier: acme.Identifier{ Type: "wireapp", - Value: "wireapp://user:client@domain", + Value: "wireapp://user!client@domain", }, Status: acme.StatusPending, ExpiresAt: clock.Now(), diff --git a/acme/challenge.go b/acme/challenge.go index 471bb6ba..aa4f46b1 100644 --- a/acme/challenge.go +++ b/acme/challenge.go @@ -33,10 +33,9 @@ import ( "go.step.sm/crypto/pemutil" "go.step.sm/crypto/x509util" "golang.org/x/exp/slices" - "gopkg.in/square/go-jose.v2/jwt" + "github.com/smallstep/certificates/acme/wire" "github.com/smallstep/certificates/authority/provisioner" - "github.com/smallstep/certificates/wire" ) type ChallengeType string @@ -385,8 +384,7 @@ func wireOIDC01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose.JSO GivenName string `json:"given_name,omitempty"` KeyAuth string `json:"keyauth"` // TODO(hs): use this property instead of the one in the payload after https://github.com/wireapp/rusty-jwt-tools/tree/fix/keyauth is done } - err = idToken.Claims(&claims) - if err != nil { + if err := idToken.Claims(&claims); err != nil { return storeError(ctx, db, ch, false, WrapError(ErrorRejectedIdentifierType, err, "error retrieving claims from ID token")) } @@ -418,9 +416,9 @@ func wireOIDC01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose.JSO return WrapErrorISE(err, "error updating challenge") } - parsedIDToken, err := jwt.ParseSigned(oidcPayload.IDToken) + parsedIDToken, err := jose.ParseSigned(oidcPayload.IDToken) if err != nil { - return WrapErrorISE(err, "invalid OIDC id token") + return WrapErrorISE(err, "invalid OIDC ID token") } oidcToken := make(map[string]interface{}) if err := parsedIDToken.UnsafeClaimsWithoutVerification(&oidcToken); err != nil { @@ -432,7 +430,7 @@ func wireOIDC01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose.JSO return WrapErrorISE(err, "could not find current order by account id") } if len(orders) == 0 { - return WrapErrorISE(err, "there are not enough orders for this account for this custom OIDC challenge") + return NewErrorISE("there are not enough orders for this account for this custom OIDC challenge") } order := orders[len(orders)-1] @@ -454,15 +452,14 @@ func wireDPOP01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose.JSO return NewErrorISE("missing provisioner") } - rawKid, thumbprintErr := jwk.Thumbprint(crypto.SHA256) - if thumbprintErr != nil { - return storeError(ctx, db, ch, false, WrapError(ErrorServerInternalType, thumbprintErr, "failed to compute JWK thumbprint")) + rawKid, err := jwk.Thumbprint(crypto.SHA256) + if err != nil { + return storeError(ctx, db, ch, false, WrapError(ErrorServerInternalType, err, "failed to compute JWK thumbprint")) } kid := base64.RawURLEncoding.EncodeToString(rawKid) var dpopPayload wireDpopPayload - err := json.Unmarshal(payload, &dpopPayload) - if err != nil { + if err := json.Unmarshal(payload, &dpopPayload); err != nil { return storeError(ctx, db, ch, false, WrapError(ErrorRejectedIdentifierType, err, "error unmarshalling Wire challenge payload")) } diff --git a/acme/challenge_test.go b/acme/challenge_test.go index e28ecb7e..49b7f347 100644 --- a/acme/challenge_test.go +++ b/acme/challenge_test.go @@ -31,15 +31,16 @@ import ( "time" "github.com/fxamacker/cbor/v2" - "github.com/smallstep/certificates/authority/config" - "github.com/smallstep/certificates/authority/provisioner" - "github.com/smallstep/certificates/wire" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "go.step.sm/crypto/jose" "go.step.sm/crypto/keyutil" "go.step.sm/crypto/minica" "go.step.sm/crypto/x509util" + + "github.com/smallstep/certificates/acme/wire" + "github.com/smallstep/certificates/authority/config" + "github.com/smallstep/certificates/authority/provisioner" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) type mockClient struct { diff --git a/acme/db.go b/acme/db.go index 12a18a9d..d98234a9 100644 --- a/acme/db.go +++ b/acme/db.go @@ -52,9 +52,10 @@ type DB interface { CreateOrder(ctx context.Context, o *Order) error GetOrder(ctx context.Context, id string) (*Order, error) GetOrdersByAccountID(ctx context.Context, accountID string) ([]string, error) - GetAllOrdersByAccountID(ctx context.Context, accountID string) ([]string, error) UpdateOrder(ctx context.Context, o *Order) error + // TODO(hs): put in a different interface + GetAllOrdersByAccountID(ctx context.Context, accountID string) ([]string, error) CreateDpopToken(ctx context.Context, orderID string, dpop map[string]interface{}) error GetDpopToken(ctx context.Context, orderID string) (map[string]interface{}, error) CreateOidcToken(ctx context.Context, orderID string, idToken map[string]interface{}) error diff --git a/acme/order.go b/acme/order.go index cd3c6bac..5c0b3a4a 100644 --- a/acme/order.go +++ b/acme/order.go @@ -17,8 +17,8 @@ import ( "go.step.sm/crypto/keyutil" "go.step.sm/crypto/x509util" + "github.com/smallstep/certificates/acme/wire" "github.com/smallstep/certificates/authority/provisioner" - "github.com/smallstep/certificates/wire" ) type IdentifierType string diff --git a/wire/id.go b/acme/wire/id.go similarity index 73% rename from wire/id.go rename to acme/wire/id.go index 9d57f79b..5e5bd03b 100644 --- a/wire/id.go +++ b/acme/wire/id.go @@ -21,6 +21,7 @@ func ParseID(data []byte) (wireID ID, err error) { } type ClientID struct { + Scheme string Username string DeviceID string Domain string @@ -34,14 +35,18 @@ type ClientID struct { func ParseClientID(clientID string) (ClientID, error) { clientIDURI, err := uri.Parse(clientID) if err != nil { - return ClientID{}, fmt.Errorf("invalid clientID URI %q: %w", clientID, err) + return ClientID{}, fmt.Errorf("invalid Wire client ID URI %q: %w", clientID, err) + } + if clientIDURI.Scheme != "wireapp" { + return ClientID{}, fmt.Errorf("invalid Wire client ID scheme %q; expected \"wireapp\"", clientIDURI.Scheme) } fullUsername := clientIDURI.User.Username() parts := strings.SplitN(fullUsername, "!", 2) if len(parts) != 2 { - return ClientID{}, fmt.Errorf("invalid clientID %q", fullUsername) + return ClientID{}, fmt.Errorf("invalid Wire client ID username %q", fullUsername) } return ClientID{ + Scheme: clientIDURI.Scheme, Username: parts[0], DeviceID: parts[1], Domain: clientIDURI.Host, diff --git a/authority/provisioner/acme.go b/authority/provisioner/acme.go index dd41f5f5..52b24530 100644 --- a/authority/provisioner/acme.go +++ b/authority/provisioner/acme.go @@ -10,8 +10,9 @@ import ( "time" "github.com/pkg/errors" - "github.com/smallstep/certificates/wire" "go.step.sm/linkedca" + + "github.com/smallstep/certificates/acme/wire" ) // ACMEChallenge represents the supported acme challenges. diff --git a/go.mod b/go.mod index fa466025..2360ccbf 100644 --- a/go.mod +++ b/go.mod @@ -148,6 +148,6 @@ require ( google.golang.org/genproto/googleapis/api v0.0.0-20231211222908-989df2bf70f3 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20231212172506-995d672761c0 // indirect gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect - gopkg.in/square/go-jose.v2 v2.6.0 + gopkg.in/square/go-jose.v2 v2.6.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect; indirects )