Skip to content

fix(sdk): DSPX-4607 canonicalize DPoP headers and extract zipstream constants #3973

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dmihalcik-virtru wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(sdk): DSPX-4607 canonicalize DPoP headers and extract zipstream constants #3973

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions sdk/auth/oauth/oauth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func getAccessTokenRequest(tokenEndpoint, dpopNonce string, scopes []string, cli
if err != nil {
return nil, err
}
req.Header.Set("dpop", dpop)
req.Header.Set("DPoP", dpop)
req.Header.Set("Accept", "application/json")
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")

Expand Down Expand Up @@ -163,7 +163,7 @@ func GetAccessToken(client *http.Client, tokenEndpoint string, scopes []string,

defer resp.Body.Close()

if nonceHeader := resp.Header.Get("dpop-nonce"); nonceHeader != "" && resp.StatusCode == http.StatusBadRequest {
if nonceHeader := resp.Header.Get("DPoP-Nonce"); nonceHeader != "" && resp.StatusCode == http.StatusBadRequest {
nonceReq, err := getAccessTokenRequest(tokenEndpoint, nonceHeader, scopes, clientCredentials, &dpopPrivateKey)
if err != nil {
return nil, err
Expand Down Expand Up @@ -272,7 +272,7 @@ func DoTokenExchange(ctx context.Context, client *http.Client, tokenEndpoint str
}
defer resp.Body.Close()

if nonceHeader := resp.Header.Get("dpop-nonce"); nonceHeader != "" && resp.StatusCode == http.StatusBadRequest {
if nonceHeader := resp.Header.Get("DPoP-Nonce"); nonceHeader != "" && resp.StatusCode == http.StatusBadRequest {
nonceReq, err := getTokenExchangeRequest(ctx, tokenEndpoint, nonceHeader, scopes, clientCredentials, tokenExchange, &key)
if err != nil {
return nil, err
Expand Down Expand Up @@ -321,7 +321,7 @@ func getTokenExchangeRequest(ctx context.Context, tokenEndpoint, dpopNonce strin
if err != nil {
return nil, err
}
req.Header.Set("dpop", dpop)
req.Header.Set("DPoP", dpop)
req.Header.Set("Accept", "application/json")
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
err = setClientAuth(clientCredentials, &data, req, tokenEndpoint)
Expand Down Expand Up @@ -371,7 +371,7 @@ func getCertExchangeRequest(ctx context.Context, tokenEndpoint string, clientCre
}
req.Header.Set("Accept", "application/json")
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
req.Header.Set("dpop", dpop)
req.Header.Set("DPoP", dpop)
if err = setClientAuth(clientCredentials, &data, req, tokenEndpoint); err != nil {
return nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions sdk/auth/token_adding_interceptor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,8 +201,8 @@ type FakeAccessServiceServerConnect struct {
}

func (f *FakeAccessServiceServerConnect) PublicKey(ctx context.Context, req *connect.Request[kas.PublicKeyRequest]) (*connect.Response[kas.PublicKeyResponse], error) {
f.accessToken = []string{req.Header().Get("authorization")}
f.dpopToken = []string{req.Header().Get("dpop")}
f.accessToken = []string{req.Header().Get("Authorization")}
f.dpopToken = []string{req.Header().Get("DPoP")}
var ok bool
f.dpopKey, ok = ctx.Value("dpop-jwk").(jwk.Key)
if !ok {
Expand Down
8 changes: 4 additions & 4 deletions sdk/codegen/runner/generate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,12 +177,12 @@ func New%s%s%sConnectWrapper(httpClient connect.HTTPClient, baseURL string, opts
func generateInterfaceType(interfaceName string, methods []string, packageName, prefix, suffix string) string {
// Generate the interface type definition
var builder strings.Builder
builder.WriteString(fmt.Sprintf(`
fmt.Fprintf(&builder, `
type %s%s%s interface {
`, prefix, interfaceName, suffix))
`, prefix, interfaceName, suffix)
for _, method := range methods {
builder.WriteString(fmt.Sprintf(` %s(ctx context.Context, req *%s.%sRequest) (*%s.%sResponse, error)
`, method, packageName, method, packageName, method))
fmt.Fprintf(&builder, ` %s(ctx context.Context, req *%s.%sRequest) (*%s.%sResponse, error)
`, method, packageName, method, packageName, method)
}
builder.WriteString("}\n")
return builder.String()
Expand Down
47 changes: 27 additions & 20 deletions sdk/internal/zipstream/segment_writer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@ import (
"sync"
)

// Error Op and Type values reported by segmentWriter.
const (
opWriteSegment = "write-segment"
opFinalize = "finalize"
errTypeSegment = "segment"
)

// segmentWriter implements the SegmentWriter interface for out-of-order segment writing
type segmentWriter struct {
*baseWriter
Expand Down Expand Up @@ -54,27 +61,27 @@ func (sw *segmentWriter) WriteSegment(ctx context.Context, index int, size uint6

// Check if writer is closed or finalized
if err := sw.checkClosed(); err != nil {
return nil, &Error{Op: "write-segment", Type: "segment", Err: err}
return nil, &Error{Op: opWriteSegment, Type: errTypeSegment, Err: err}
}

if sw.finalized {
return nil, &Error{Op: "write-segment", Type: "segment", Err: ErrWriterClosed}
return nil, &Error{Op: opWriteSegment, Type: errTypeSegment, Err: ErrWriterClosed}
}

// Validate segment index (allow dynamic expansion for streaming use cases)
if index < 0 {
return nil, &Error{Op: "write-segment", Type: "segment", Err: ErrInvalidSegment}
return nil, &Error{Op: opWriteSegment, Type: errTypeSegment, Err: ErrInvalidSegment}
}

// Check for duplicate segment
if _, exists := sw.metadata.Segments[index]; exists {
return nil, &Error{Op: "write-segment", Type: "segment", Err: ErrDuplicateSegment}
return nil, &Error{Op: opWriteSegment, Type: errTypeSegment, Err: ErrDuplicateSegment}
}

// Check context cancellation
select {
case <-ctx.Done():
return nil, &Error{Op: "write-segment", Type: "segment", Err: ctx.Err()}
return nil, &Error{Op: opWriteSegment, Type: errTypeSegment, Err: ctx.Err()}
default:
}

Expand All @@ -87,14 +94,14 @@ func (sw *segmentWriter) WriteSegment(ctx context.Context, index int, size uint6
if index == 0 {
// Segment 0: Write local file header + encrypted data
if err := sw.writeLocalFileHeader(buffer); err != nil {
return nil, &Error{Op: "write-segment", Type: "segment", Err: err}
return nil, &Error{Op: opWriteSegment, Type: errTypeSegment, Err: err}
}
}

// Record segment metadata only (no payload retention). Payload bytes are returned
// to the caller and may be uploaded; we keep only CRC and size for finalize.
if err := sw.metadata.AddSegment(index, size, crc32); err != nil {
return nil, &Error{Op: "write-segment", Type: "segment", Err: err}
return nil, &Error{Op: opWriteSegment, Type: errTypeSegment, Err: err}
}

// Update payload entry metadata
Expand All @@ -112,24 +119,24 @@ func (sw *segmentWriter) Finalize(ctx context.Context, manifest []byte) ([]byte,

// Check if writer is closed or already finalized
if err := sw.checkClosed(); err != nil {
return nil, &Error{Op: "finalize", Type: "segment", Err: err}
return nil, &Error{Op: opFinalize, Type: errTypeSegment, Err: err}
}

if sw.finalized {
return nil, &Error{Op: "finalize", Type: "segment", Err: ErrWriterClosed}
return nil, &Error{Op: opFinalize, Type: errTypeSegment, Err: ErrWriterClosed}
}

// Check context cancellation
select {
case <-ctx.Done():
return nil, &Error{Op: "finalize", Type: "segment", Err: ctx.Err()}
return nil, &Error{Op: opFinalize, Type: errTypeSegment, Err: ctx.Err()}
default:
}

// Nothing arrived at all: report the general incomplete-input error
// rather than the segment-0-specific one below.
if len(sw.metadata.Segments) == 0 {
return nil, &Error{Op: "finalize", Type: "segment", Err: ErrSegmentMissing}
return nil, &Error{Op: opFinalize, Type: errTypeSegment, Err: ErrSegmentMissing}
}

// Only segment 0 emits the payload's local file header, and every offset
Expand All @@ -146,7 +153,7 @@ func (sw *segmentWriter) Finalize(ctx context.Context, manifest []byte) ([]byte,
// absence either, since that derived order is self-consistent by
// construction.
if _, ok := sw.metadata.Segments[0]; !ok {
return nil, &Error{Op: "finalize", Type: "segment", Err: ErrNoSegmentZero}
return nil, &Error{Op: opFinalize, Type: errTypeSegment, Err: ErrNoSegmentZero}
}

// If no explicit order was provided, derive order from present indices (sorted).
Expand All @@ -158,7 +165,7 @@ func (sw *segmentWriter) Finalize(ctx context.Context, manifest []byte) ([]byte,
sort.Ints(order)
if err := sw.metadata.SetOrder(order); err != nil {
// This should be an unreachable state, but handle it defensively.
return nil, &Error{Op: "finalize", Type: "segment", Err: fmt.Errorf("internal error setting segment order: %w", err)}
return nil, &Error{Op: opFinalize, Type: errTypeSegment, Err: fmt.Errorf("internal error setting segment order: %w", err)}
}
}

Expand All @@ -167,7 +174,7 @@ func (sw *segmentWriter) Finalize(ctx context.Context, manifest []byte) ([]byte,
// complete by construction, and the empty set already returned. Kept for
// a future caller that supplies an explicit order.
if !sw.metadata.IsComplete() {
return nil, &Error{Op: "finalize", Type: "segment", Err: ErrSegmentMissing}
return nil, &Error{Op: opFinalize, Type: errTypeSegment, Err: ErrSegmentMissing}
}

// Compute final CRC32 by combining per-segment CRCs now that all are present
Expand Down Expand Up @@ -199,10 +206,10 @@ func (sw *segmentWriter) Finalize(ctx context.Context, manifest []byte) ([]byte,

// 1. Write data descriptor for payload (fail if Zip64Never but required)
if sw.config.Zip64 == Zip64Never && needZip64ForPayload {
return nil, &Error{Op: "finalize", Type: "segment", Err: ErrZip64Required}
return nil, &Error{Op: opFinalize, Type: errTypeSegment, Err: ErrZip64Required}
}
if err := sw.writeDataDescriptor(buffer, needZip64ForPayload); err != nil {
return nil, &Error{Op: "finalize", Type: "segment", Err: err}
return nil, &Error{Op: opFinalize, Type: errTypeSegment, Err: err}
}

// 2. Update payload entry CRC32 and add to central directory
Expand All @@ -221,7 +228,7 @@ func (sw *segmentWriter) Finalize(ctx context.Context, manifest []byte) ([]byte,
}

if err := sw.writeManifestFile(buffer, manifest, manifestEntry); err != nil {
return nil, &Error{Op: "finalize", Type: "segment", Err: err}
return nil, &Error{Op: opFinalize, Type: errTypeSegment, Err: err}
}

// 4. Add manifest entry to central directory
Expand All @@ -232,15 +239,15 @@ func (sw *segmentWriter) Finalize(ctx context.Context, manifest []byte) ([]byte,
// Decide if ZIP64 is needed for central directory/EOCD based on offset or forced mode
needZip64ForCD := needZip64ForPayload || sw.config.Zip64 == Zip64Always || sw.centralDir.Offset > uint64(max32) || len(sw.centralDir.Entries) > int(^uint16(0))
if sw.config.Zip64 == Zip64Never && needZip64ForCD {
return nil, &Error{Op: "finalize", Type: "segment", Err: ErrZip64Required}
return nil, &Error{Op: opFinalize, Type: errTypeSegment, Err: ErrZip64Required}
}
cdBytes, err := sw.centralDir.GenerateBytes(needZip64ForCD)
if err != nil {
return nil, &Error{Op: "finalize", Type: "segment", Err: err}
return nil, &Error{Op: opFinalize, Type: errTypeSegment, Err: err}
}

if _, err := buffer.Write(cdBytes); err != nil {
return nil, &Error{Op: "finalize", Type: "segment", Err: err}
return nil, &Error{Op: opFinalize, Type: errTypeSegment, Err: err}
}

sw.finalized = true
Expand Down
1 change: 1 addition & 0 deletions sdk/kas_client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ type TestUpgradeRewrapRequestV1Suite struct {

func (suite *TestUpgradeRewrapRequestV1Suite) TestUpgradeRewrapRequestV1_Happy() {
response := &kaspb.RewrapResponse{
//nolint:staticcheck // upgradeRewrapRequestV1 exists to translate this deprecated field, so the test must set it
EntityWrappedKey: []byte("wrappedKey"),
}
requests := []*kaspb.UnsignedRewrapRequest_WithPolicyRequest{
Expand Down
Loading