Skip to content
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

Deprecate Content-Transfer-Encoding per RFC 8951 #31

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Deprecate Content-Transfer-Encoding per RFC 8951 #31

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c2dee7b
Removed required for Content-Transfer-Encoding headers per RFC 8951 -…
61131 Dec 18, 2023
7d55dfc
Remove all validation of Content-Transfer-Encoding header values per …
61131 Dec 18, 2023
0418aff
Remove unused function
toddgaunt-gs Mar 7, 2024
2094627
Merge branch 'globalsign:master' into master
61131 May 1, 2024
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
8 changes: 0 additions & 8 deletions client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,14 +256,6 @@ func (c *Client) ServerKeyGen(ctx context.Context, r *x509.CertificateRequest) (
return nil, nil, fmt.Errorf("more than %d parts in HTTP response", numParts)
}

// Check content-transfer-encoding is as expected, and read the part
// body.
if ce := part.Header.Get(transferEncodingHeader); ce == "" {
return nil, nil, fmt.Errorf("missing %s header", transferEncodingHeader)
} else if strings.ToUpper(ce) != strings.ToUpper(encodingTypeBase64) {
return nil, nil, fmt.Errorf("unexpected %s: %s", transferEncodingHeader, ce)
}

// Process based on the part's content-type. Per RFC7030 4.4.2, if
// additional encryption is not being employed, the private key data
// must be placed in an application/pkcs8 part. Otherwise, it must
Expand Down
25 changes: 0 additions & 25 deletions est_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -727,31 +727,6 @@ func TestServerErrors(t *testing.T) {
status: http.StatusUnsupportedMediaType,
errText: "415 malformed or missing Content-Type header\n",
},
{
name: "Enroll/BadContentTransferEncoding",
path: enrollEndpoint,
method: http.MethodPost,
headers: http.Header{
typeHeader: []string{mimeTypePKCS10},
encodingHeader: []string{encodingBinary},
authorizationHeader: []string{authorizationValue},
hostHeader: []string{testDomain},
},
status: http.StatusUnsupportedMediaType,
errText: "415 Content-Transfer-Encoding must be base64\n",
},
{
name: "Enroll/MissingContentTransferEncoding",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we actually keep this test but just change the expected status returned? It'd be nice to have test cases that demonstrate that clients can send nothing at all.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, I see now they are in a function that is explicitly meant to check for errors.

path: enrollEndpoint,
method: http.MethodPost,
headers: http.Header{
typeHeader: []string{mimeTypePKCS10},
authorizationHeader: []string{authorizationValue},
hostHeader: []string{testDomain},
},
status: http.StatusUnsupportedMediaType,
errText: "415 missing Content-Transfer-Encoding header\n",
},
{
name: "Enroll/BadBase64",
path: enrollEndpoint,
Expand Down
45 changes: 0 additions & 45 deletions http.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,17 +245,6 @@ func verifyResponseType(r *http.Response, t, e string) error {
return fmt.Errorf("unexpected %s: %s", contentTypeHeader, ctype)
}

cenc := r.Header.Get(transferEncodingHeader)
if cenc == "" {
return fmt.Errorf("missing %s header", transferEncodingHeader)
}

// Content-Transfer-Encoding values are not case sensitive per RFC 2045
// section 6.
if strings.ToUpper(cenc) != strings.ToUpper(e) {
return fmt.Errorf("unexpected %s: %s", transferEncodingHeader, cenc)
}

return nil
}

Expand All @@ -272,17 +261,6 @@ func verifyPartTypeResponse(part *multipart.Part, t, e string) error {
return fmt.Errorf("unexpected %s: %s", contentTypeHeader, ctype)
}

cenc := part.Header.Get(transferEncodingHeader)
if cenc == "" {
return fmt.Errorf("missing %s header", transferEncodingHeader)
}

// Content-Transfer-Encoding values are not case sensitive per RFC 2045
// section 6.
if strings.ToUpper(cenc) != strings.ToUpper(e) {
return fmt.Errorf("unexpected %s: %s", transferEncodingHeader, cenc)
}

return nil
}

Expand All @@ -307,29 +285,6 @@ func verifyRequestType(have, want string) error {
return nil
}

// verifyRequestEncoding verifies if the content-transfer-encoding of an HTTP
// request is as expected. It returns an error implementing Error and is
// intended to be used by server code.
func verifyRequestEncoding(have, want string) error {
if have == "" {
return &estError{
status: http.StatusUnsupportedMediaType,
desc: fmt.Sprintf("missing %s header", transferEncodingHeader),
}
}

// Content-Transfer-Encoding values are not case sensitive per RFC 2045
// section 6.
if strings.ToUpper(have) != strings.ToUpper(want) {
return &estError{
status: http.StatusUnsupportedMediaType,
desc: fmt.Sprintf("%s must be %s", transferEncodingHeader, want),
}
}

return nil
}

// writeResponse writes headers, a status code, and an object containing the
// body to an HTTP response. If encode is true, the object is base64-encoded.
// The appropriate encoding is chosen according to the object's type.
Expand Down
23 changes: 0 additions & 23 deletions http_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,29 +66,6 @@ func TestVerifyResponseType(t *testing.T) {
e: "base64",
err: errors.New("missing or malformed Content-Type header: mime: no media type"),
},
{
name: "WrongEncoding",
r: &http.Response{
Header: http.Header{
"Content-Type": []string{"application/pkcs7; smime-type=certs-only"},
"Content-Transfer-Encoding": []string{"base64"},
},
},
t: "application/pkcs7",
e: "binary",
err: errors.New("unexpected Content-Transfer-Encoding: base64"),
},
{
name: "MissingEncoding",
r: &http.Response{
Header: http.Header{
"Content-Type": []string{"application/pkcs7; smime-type=certs-only"},
},
},
t: "application/pkcs7",
e: "base64",
err: errors.New("missing Content-Transfer-Encoding header"),
},
{
name: "BadTypeParameter",
r: &http.Response{
Expand Down
26 changes: 0 additions & 26 deletions server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,16 +170,12 @@ func NewRouter(cfg *ServerConfig) (http.Handler, error) {

r.With(
requireContentType(mimeTypePKCS10),
).With(
requireTransferEncoding(encodingTypeBase64),
).With(
requireBasicAuth(cfg.CheckBasicAuth, true),
).Post(enrollEndpoint, enroll)

r.With(
requireContentType(mimeTypePKCS10),
).With(
requireTransferEncoding(encodingTypeBase64),
).With(
requireBasicAuth(cfg.CheckBasicAuth, true),
).With(
Expand All @@ -188,8 +184,6 @@ func NewRouter(cfg *ServerConfig) (http.Handler, error) {

r.With(
requireContentType(mimeTypePKCS10),
).With(
requireTransferEncoding(encodingTypeBase64),
).With(
requireBasicAuth(cfg.CheckBasicAuth, true),
).Post(serverkeygenEndpoint, serverkeygen)
Expand All @@ -207,16 +201,12 @@ func NewRouter(cfg *ServerConfig) (http.Handler, error) {

r.With(
requireContentType(mimeTypePKCS10),
).With(
requireTransferEncoding(encodingTypeBase64),
).With(
requireBasicAuth(cfg.CheckBasicAuth, true),
).Post(enrollEndpoint, enroll)

r.With(
requireContentType(mimeTypePKCS10),
).With(
requireTransferEncoding(encodingTypeBase64),
).With(
requireBasicAuth(cfg.CheckBasicAuth, true),
).With(
Expand All @@ -225,8 +215,6 @@ func NewRouter(cfg *ServerConfig) (http.Handler, error) {

r.With(
requireContentType(mimeTypePKCS10),
).With(
requireTransferEncoding(encodingTypeBase64),
).With(
requireBasicAuth(cfg.CheckBasicAuth, true),
).Post(serverkeygenEndpoint, serverkeygen)
Expand Down Expand Up @@ -615,20 +603,6 @@ func requireContentType(t string) func(next http.Handler) http.Handler {
}
}

// requireTransferEncoding is middleware which rejects a request if the content
// transfer encoding is not as stated.
func requireTransferEncoding(e string) func(next http.Handler) http.Handler {
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if err := verifyRequestEncoding(r.Header.Get(transferEncodingHeader), e); err != nil {
writeOnError(r.Context(), w, logMsgTransferEncodingInvalid, err)
return
}
next.ServeHTTP(w, r)
})
}
}

// addServerHeader is middleware which writes to an HTTP response a Server HTTP
// header. Including too much detail (such as an operating system version) in
// this header can be a security risk, but including enough detail can sometimes
Expand Down