Fix url encoding for ocsp requests #7184
Conversation
| // encodeOCSPRequest encodes the OCSP request in base64 and URL-encodes it. | ||
| // This is needed to fulfill the OCSP responder's requirements for the request format. (X.690) | ||
| func encodeOCSPRequest(reqDER []byte) string { | ||
| reqEnc := base64.StdEncoding.EncodeToString(reqDER) |
There was a problem hiding this comment.
Should this be base64.RawURLEncoding.EncodeToString(...) and then you can get rid of url.QueryEscape?
There was a problem hiding this comment.
I was hesitant to remove the padding characters because I don't know how the decoder is implemented. It should be able to determine the correct size by measuring and rounding up, though.
However, using the URLEncoding functions definitely makes sense.
There was a problem hiding this comment.
Good point about the padding, in which case URLEncoding should be fine indeed.
There was a problem hiding this comment.
I will test this against our PKI first and report back.
There was a problem hiding this comment.
Not well. The PKI we use can't handle some of the options, but I'm still waiting for a response.
I also checked back with the RFC 4648 and the URLEncode is not the same like normal base64.
See https://datatracker.ietf.org/doc/html/rfc4648#section-5 for reference.
Right now I think that my first approach of using the StdEncoding (without removing padding characters) and then url.Encoding is safer. Will update this PR as soon as I get feedback from my colleagues.
There was a problem hiding this comment.
OK, I've tested all the combinations against our PKI system.
Only the 'normal' Base64 encoding, including the padding characters, worked.
Therefore, I would stick with my initial proposal to fix the bug and achieve the greatest possible compatibility with external PKI systems.
azerbe
force-pushed
the
fix-ocsp-url-encoding
branch
from
be33cea to
b947f4c
Compare
Signed-off-by: Alexander Zerbe <[email protected]>
azerbe
force-pushed
the
fix-ocsp-url-encoding
branch
from
51283e8 to
6e5d3e8
Compare
Includes the following: - #7184 Signed-off-by: Neil Twigg <[email protected]>
Like described in issue #7183 , this PR adds the encoding to the base64 encoded certificates.
Resolves #7183
Signed-off-by: Alexander Zerbe [email protected]