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.

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

GET requests are not decoded with base64 #98

Closed
ghost opened this issue Nov 29, 2018 · 5 comments
Closed

GET requests are not decoded with base64 #98

ghost opened this issue Nov 29, 2018 · 5 comments

Comments

@ghost
Copy link

ghost commented Nov 29, 2018

When sending a GET SCEP request, the parameter given in the URL is encoded in base64 (as per https://tools.ietf.org/html/draft-gutmann-scep-10#section-4.1):
"When using GET messages to communicate binary data, base64 encoding
as specified in [2] MUST be used. The base64 encoded data is
distinct from "base64url" and may contain URI reserved characters,
thus it MUST be escaped as specified in [8] in addition to being
base64 encoded. Finally, the encoded data is inserted into the
MESSAGE portion of the HTTP GET request."
However in the implementation the message is not decoded.
Could you please have a look at it?
Thank you!

@groob
Copy link
Member

groob commented Dec 14, 2018

I'll point out that the SCEP says if the server advertises POST, the client shouldn't be using GET.

If the remote CA supports POST, the CMS-encoded SCEP messages
MUST be sent via HTTP POST instead of HTTP GET.

Now onto the actual bug, I think I see it.
The client encodes the message as base64:

msg := base64.URLEncoding.EncodeToString(req.Message)

but the server does not decode it,

scep/server/transport.go

Lines 74 to 87 in 434638f

func decodeSCEPRequest(ctx context.Context, r *http.Request) (interface{}, error) {
msg, err := message(r)
if err != nil {
return nil, err
}
defer r.Body.Close()
request := SCEPRequest{
Message: msg,
Operation: r.URL.Query().Get("operation"),
}
return request, nil
}

@aduez I'll except a PR to get this fixed if you can verify it.

@ghost
Copy link
Author

ghost commented Dec 17, 2018

Hi,
You are indeed right about POST. My goal is to have both GET and POST working.
For the fix itself, here's what I did to make it work for me:
https://github.com/aduez/scep/commit/6bfb2cfcc3df0ae29fc7c810598a420dba77e26a
However I think this bugfix is a dirty workaround, therefore I did not create a PU.
Best regards,

@groob
Copy link
Member

groob commented Dec 19, 2018

if you added it to the decodeSCEPRequest instead of ParsePKIMessage it wouldn't be a hack.
you can use GET vs POST there to determine if the request should be base64 decoded.

@ghost
Copy link
Author

ghost commented Dec 20, 2018

I think you are right, however I am not sure how to implement this. If I put the base64 decoding in decodeSCEPRequest, won't it try to decode as well on POST requests? (plus I'm not familiar with golang)

@wrcgator
Copy link

wrcgator commented Sep 4, 2020

doesnt seem to be fixed yet. I changed message() as it was already decoding by GET/POST switch...

func message(r *http.Request) ([]byte, error) {
	switch r.Method {
	case "GET":
		var msg string
		q := r.URL.Query()
		if _, ok := q["message"]; ok {
			msg = q.Get("message")
		}

		bmsg, err := base64.StdEncoding.DecodeString(msg)
		if err != nil {
                        return nil, error
		}
                // return decoded base 64 - work with other scep clients (sscep, sceptools, probably more...)
		return bmsg, nil
	case "POST":
		return ioutil.ReadAll(io.LimitReader(r.Body, maxPayloadSize))
	default:
		return nil, errors.New("method not supported")
	}
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants