-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
X509Chain.Build() returns true for partial chain when AllowUnknownCertificateAuthority flag is present #49615
Comments
Tagging subscribers to this area: @bartonjs, @vcsjones, @krwq, @GrabYourPitchforks Issue DetailsDescriptionAccording this comment by @bartonjs about the
Configuration
ReproRun in bash: create_ca() {
local CA_CN="$1"
openssl genrsa -out ${CA_CN}.key.pem 2048 # Generate private key
openssl req -x509 -new -nodes -key ${CA_CN}.key.pem -sha256 -days 1825 -out ${CA_CN}.cert.pem -subj "/CN=${CA_CN}/O=MyDevices/C=US" # Generate root certificate
}
create_leaf_cert() {
local DEVICE_CN="$1"
openssl genrsa -out ${DEVICE_CN}.key.pem 2048 # new private key
openssl req -new -key ${DEVICE_CN}.key.pem -out ${DEVICE_CN}.csr.pem -subj "/CN=${DEVICE_CN}/O=MyDevices/C=US" # generate signing request for the CA
}
sign_leaf_cert() {
local DEVICE_CN="$1"
local CA_CN="$2"
openssl x509 -req -in ${DEVICE_CN}.csr.pem -CA "${CA_CN}.cert.pem" -CAkey "${CA_CN}.key.pem" -set_serial 01 -out "${DEVICE_CN}.cert.pem" -days 825 -sha256 # sign the CSR
}
# Create one self-issued CA + signed cert
create_ca trust.mydevices.com
create_leaf_cert dev01.mydevices.com
sign_leaf_cert dev01.mydevices.com trust.mydevices.com
# Create another self-issued CA + signed cert
create_ca trust.different.com
create_leaf_cert dev01.different.com
sign_leaf_cert dev01.different.com trust.different.com Expected behaviorI expect chain.Build() to return the boolean equivalent to the OpenSSL validations when paired with the various certificates:
Observed behaviorsuccess=true, even when PartialChain is present in the status because the chain could not be verified
|
Side note, in .NET 5.0 support for certificate pinning was added with CustomRootStore and that mode works as expected:
This, without the
|
The "only" is just because once multiple flags are set the statement of what returns true and what returns false becomes complicated. Ease of scope for discussion, not a technical limitation.
Yeah, apparently that was a goof on my part. It's the way I always expect it to work, and I forget that PartialChain is also suppressed by AllowUnknownCertificateAuthority. On Windows, that behavior is controlled by Windows. On non-Windows we maintain parity with what Windows does since that describes the ".NET Framework behavior" for this type, which is the strong starting point for the ".NET behavior" for this type. |
@bartonjs thanks for the clarification. May I request this be re-opened and track a docs change for these classes then? At the present time, I understand the reason behind keeping the behavior consistent between platforms and therefore why it does so, but that's definitely not intuitive behavior nor described by the docs - potentially with disastrous consequences because if users are unware of this, Given the present behavior, IMO it's crucially important the docs reference the type of verification you describe in the linked GitHub issue from the original post, requiring a manual validation of |
Description
According this comment by @bartonjs in #26449 about the
AllowUnknownCertificateAuthority
flag (emphasis mine):X509Chain.Build()
returningtrue
for any leaf certificate and any CA added to the ExtraStore combination, even the leaf certificate is not signed by the ExtraStore's CA.Configuration
Repro
Run in bash:
Expected behavior
I expect chain.Build() to return the boolean equivalent to the OpenSSL validations when paired with the various certificates:
Observed behavior
success=true, even when PartialChain is present in the status because the chain could not be verified
The text was updated successfully, but these errors were encountered: