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

Cert validation behaviour change in 2.2.0 #59

Closed
will118 opened this issue Dec 13, 2024 · 3 comments
Closed

Cert validation behaviour change in 2.2.0 #59

will118 opened this issue Dec 13, 2024 · 3 comments

Comments

@will118
Copy link

will118 commented Dec 13, 2024

I've noticed a change in behaviour with the following:

fn main() {
    let cert = "-----BEGIN CERTIFICATE-----\nINVALID\n-----END CERTIFICATE-----";
    let cert = rustls_pemfile::certs(&mut cert.as_bytes())
        .next()
        .unwrap()
        .unwrap();
    dbg!(cert);
}
[package]
name = "bad-cert"
version = "0.1.0"
edition = "2021"

[dependencies]
rustls-pemfile = "=2.1.2"
# rustls-pemfile = "=2.2.0"

In 2.1.2 I see:

Custom { kind: InvalidData, error: "InvalidLastSymbol(6, 68)" }

but in 2.2.0 I get:

[src/main.rs:7:5] cert = CertificateDer(
    0x20d5402c80,
)

I'm not confident what the docs mean by:

Filters out any PEM sections that are not certificates and yields errors if a problem occurs while trying to extract a certificate.

...but regardless the change surprised me and was caught by a test.

Is it expected?

@ctz
Copy link
Member

ctz commented Dec 13, 2024

Thanks for reporting this!

INVALID is actually valid base64, ignoring padding. The base64 crate has a behaviour where it requires the last codepoint has unused bits that are cleared (this is important for users where encodings must be canonical, but I don't think PEM is one of these -- see https://datatracker.ietf.org/doc/html/rfc4648#section-3.5 for a description). Whereas, other implementations don't do that:

$ cat huh.go
package main

import (
	"encoding/base64"
	"fmt"
)

func main() {
	decoded, err := base64.StdEncoding.WithPadding(base64.NoPadding).DecodeString("INVALID")
	if err != nil {
		fmt.Println("decode error:", err)
		return
	}
	fmt.Printf("%x\n", string(decoded))
}

$ go run huh.go 
20d5402c80

I would recommend using a more overtly wrong base64 encoding in your test, such as * which will not be accepted by any implementation.

edit: altered to language to remove implication that base64 crate does something non-standard; it is just optional according to RFC4648.

@djc
Copy link
Member

djc commented Dec 13, 2024

(Sorry, read your issue report wrong.)

@will118
Copy link
Author

will118 commented Dec 13, 2024

OK, thank you for the explanation.

I think there was a misunderstanding (on our part) about the validation performed by certs(..) (valid PEM vs valid "certificate").

Now I've taken the time to understand, I don't think the change in behaviour is that big a deal...

I'm happy to close this.

@will118 will118 closed this as completed Dec 15, 2024
@djc djc reopened this Dec 15, 2024
@djc djc closed this as not planned Won't fix, can't repro, duplicate, stale Dec 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants