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

verify cert run long time when given one faked cert with large intermediate_certs that can't be verified by any anchors #276

Open
stanal opened this issue Sep 1, 2023 · 10 comments

Comments

@stanal
Copy link

stanal commented Sep 1, 2023

give one cert that can't be verified by any ca in all anchors, the check_signatures can nerver be called
the verify logical will take long time to ended (may be expotent or quradic time)
`

  match loop_while_non_fatal_error(trust_anchors, |trust_anchor: &TrustAnchor| {
    let trust_anchor_subject = untrusted::Input::from(trust_anchor.subject);
    if cert.issuer != trust_anchor_subject { // always emitted
        println!("cert's issuer is not anchor's subject, return err");
        return Err(Error::UnknownIssuer.into());
    }
   check_signatures(supported_sig_algs, cert, trust_anchor_spki, signatures)?;  // will never do 

`

@stanal stanal changed the title verify cert will loop forerver when check_signatures nerver called and cert.issuer != trust_anchor_subject satisfied all the time verify cert loop nerver end when given one faked cert that can't be verified by any anchors Sep 1, 2023
@stanal stanal changed the title verify cert loop nerver end when given one faked cert that can't be verified by any anchors verify cert logical nerver end when given one faked cert that can't be verified by any anchors Sep 1, 2023
@stanal
Copy link
Author

stanal commented Sep 1, 2023

example: use self-signed cert for server-side, and client-side not load root cert which signed the server cert, client-side just use webpki-roots as anchors.
then client-side will nerver can verify the server certificate, and will loop to do build_chain again and again for long long time to end

@stanal stanal changed the title verify cert logical nerver end when given one faked cert that can't be verified by any anchors verify cert run long time when given one faked cert with large intermediate_certs that can't be verified by any anchors Sep 1, 2023
@briansmith
Copy link
Owner

Which version of webpki are you using? If 0.22.1, did it work with 0.22.0?

@stanal
Copy link
Author

stanal commented Sep 2, 2023

Which version of webpki are you using? If 0.22.1, did it work with 0.22.0?

0.22.1

@briansmith
Copy link
Owner

Thanks. What happens if you use 0.22.0?

@stanal
Copy link
Author

stanal commented Sep 3, 2023

Thanks. What happens if you use 0.22.0?

it has the same problem
i think the rustls-webpki also has this problem, the verify_cert.rs are same to each other
i test that when given 10 intermidiates certs, the build_chain_inner function will be called 2060312 times and elp 22s to end ,
i guess the time is releative to n! where n is num of intermidiates

@stanal
Copy link
Author

stanal commented Sep 3, 2023

the counter signatures just limit the called times of verify_signed_data, but not limit the total recursion times of call build_chain_inner

@stanal
Copy link
Author

stanal commented Sep 3, 2023

I'm not sure whether need to limit the total recursion times, because when given one right chians that has large number of intermidiates certs, although it will take long time to verify, but it will verify successfully finnally

@ctz
Copy link
Contributor

ctz commented Sep 4, 2023

i test that when given 10 intermidiates certs, the build_chain_inner function will be called 2060312 times and elp 22s to end ,

could you clarify exactly what this test case looks like? because trying to reproduce with a ten-deep untrusted chain doesn't do that for me. though i can reproduce this issue with other shapes of chains.

It seems like a mistake to allow the same certificate to appear multiple times in the intermediates list (AFAIK that cannot, by definition, ever make an invalid chain become valid?)

@briansmith
Copy link
Owner

It seems like a mistake to allow the same certificate to appear multiple times in the intermediates list (AFAIK that cannot, by definition, ever make an invalid chain become valid?)

That's right. See https://github.com/nss-dev/nss/blob/bb4a1d38dd9e92923525ac6b5ed0288479f3f3fc/lib/mozpkix/lib/pkixbuild.cpp#L160.

See also buildForwardCallBudget in the same code base.

@stanal
Copy link
Author

stanal commented Sep 4, 2023

i test that when given 10 intermidiates certs, the build_chain_inner function will be called 2060312 times and elp 22s to end ,

could you clarify exactly what this test case looks like? because trying to reproduce with a ten-deep untrusted chain doesn't do that for me. though i can reproduce this issue with other shapes of chains.

see https://github.com/stanal/tlsserver/tree/main

It seems like a mistake to allow the same certificate to appear multiple times in the intermediates list (AFAIK that cannot, by definition, ever make an invalid chain become valid?)

cpu added a commit to cpu/brianwebpki that referenced this issue Sep 8, 2023
This is intended to be complementary to the signature validation limit
fix and addresses briansmith#276 in the same manner as NSS
libmozpkix.
cpu added a commit to cpu/brianwebpki that referenced this issue Sep 8, 2023
This is intended to be complementary to the signature validation limit
fix and addresses briansmith#276 in the same manner as NSS
libmozpkix.
cpu added a commit to cpu/brianwebpki that referenced this issue Sep 8, 2023
This is intended to be complementary to the signature validation limit
fix and addresses briansmith#276 in the same manner as NSS
libmozpkix.
cpu added a commit to cpu/brianwebpki that referenced this issue Sep 8, 2023
This is intended to be complementary to the signature validation limit
fix and addresses briansmith#276 in the same manner as NSS
libmozpkix.
cpu added a commit to cpu/brianwebpki that referenced this issue Sep 8, 2023
This is intended to be complementary to the signature validation limit
fix and addresses briansmith#276 in the same manner as NSS
libmozpkix.
cpu added a commit to cpu/brianwebpki that referenced this issue Sep 8, 2023
This is intended to be complementary to the signature validation limit
fix and addresses briansmith#276 in the same manner as NSS
libmozpkix.
cpu added a commit to cpu/brianwebpki that referenced this issue Sep 8, 2023
This is intended to be complementary to the signature validation limit
fix and addresses briansmith#276 in the same manner as NSS
libmozpkix.
cpu added a commit to cpu/brianwebpki that referenced this issue Sep 15, 2023
This is intended to be complementary to the signature validation limit
fix and addresses briansmith#276 in the same manner as NSS
libmozpkix.
briansmith pushed a commit that referenced this issue Sep 30, 2023
This is intended to be complementary to the signature validation limit
fix and addresses #276 in the same manner as NSS
libmozpkix.
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