From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 7EECC620AB for ; Wed, 19 Jan 2022 13:54:05 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 74A6518C7D for ; Wed, 19 Jan 2022 13:54:05 +0100 (CET) Received: from office.oderland.com (office.oderland.com [91.201.60.5]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id AA98318C71 for ; Wed, 19 Jan 2022 13:54:03 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=oderland.se ; s=default; h=Content-Transfer-Encoding:Content-Type:In-Reply-To:Subject: From:References:To:MIME-Version:Date:Message-ID:Sender:Reply-To:Cc:Content-ID :Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To: Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe :List-Post:List-Owner:List-Archive; bh=xuzicfMDW6G8xGJOKVkPvHLKDgzhltmWL5gZ/F0PbfY=; b=mpeqsdkxEuYh0JvUSFz5NvGE49 8TgVO2iuWvrQSL7bi55FoiEGs0Uk/ISsSjCG5tnVG1lWdpzY+nSMHhu4U5geokvNZn7Zgf+R35hIH LB9zpfPhV83bzKxW0uOHmJo71PwwsT5TJqvN9raV4qdyuuaP8Ur51Jvl2DYguoGS/8yZZ0jDAmKKb JG9GyTL22PUwIlu1+SXQ2/HQJi2wIwrcbDPXAKCuW3pgGakh+Ug90lAvlUqlbS3vxPD5fI3TNvvRX YxD4Fhj/vYsAs1UYGzvfXnIrqMcG+XMGoVxAp8Fe7AiI0LRQCxZuxiyYecWHxBvxsX1tK7VDzm8wD A7+sHJlg==; Received: from [193.180.18.160] (port=38216 helo=[10.137.0.14]) by office.oderland.com with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.94.2) (envelope-from ) id 1nAATN-00ApIr-9e; Wed, 19 Jan 2022 13:53:57 +0100 Message-ID: Date: Wed, 19 Jan 2022 13:53:55 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:97.0) Gecko/20100101 Thunderbird/97.0 Content-Language: en-US To: Proxmox VE development discussion , Fabian Ebner , =?UTF-8?Q?Fabian_Gr=c3=bcnbichler?= References: <20211222135257.3242938-1-f.gruenbichler@proxmox.com> <20211222135257.3242938-7-f.gruenbichler@proxmox.com> <1642582327.0fpxeqo696.astroid@nora.none> <8735485c-517c-e70f-02f7-3a970b27f575@proxmox.com> From: Josef Johansson In-Reply-To: <8735485c-517c-e70f-02f7-3a970b27f575@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - office.oderland.com X-AntiAbuse: Original Domain - lists.proxmox.com X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - oderland.se X-Get-Message-Sender-Via: office.oderland.com: authenticated_id: josjoh@oderland.se X-Authenticated-Sender: office.oderland.com: josjoh@oderland.se X-SPAM-LEVEL: Spam detection results: 0 AWL -0.280 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DKIM_SIGNED 0.1 Message has a DKIM or DK signature, not necessarily valid DKIM_VALID -0.1 Message has at least one valid DKIM or DK signature DKIM_VALID_AU -0.1 Message has a valid DKIM or DK signature from author's domain DKIM_VALID_EF -0.1 Message has a valid DKIM or DK signature from envelope-from domain KAM_LOTSOFHASH 0.25 Emails with lots of hash-like gibberish NICE_REPLY_A -0.001 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH v3 proxmox-websocket-tunnel 3/4] add fingerprint validation X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 19 Jan 2022 12:54:05 -0000 On 1/19/22 13:16, Fabian Ebner wrote: > Am 19.01.22 um 11:34 schrieb Fabian Grünbichler: >> On January 4, 2022 12:37 pm, Fabian Ebner wrote: >>> Am 22.12.21 um 14:52 schrieb Fabian Grünbichler: >>>> in case we have no explicit fingerprint, we use openssl's regular >>>> "PEER" >>>> verification. if we have a fingerprint, we ignore openssl >>>> altogether and >>>> just verify the fingerprint of the presented leaf certificate. >>>> >>>> Signed-off-by: Fabian Grünbichler >>>> --- >>>> >>>> Notes: >>>>       v3: switch to using hex instead of no-longer-existing >>>> digest_to_hex >>>>       v2: new >>>> >>>>    src/main.rs | 47 ++++++++++++++++++++++++++++++++++++++++++++--- >>>>    1 file changed, 44 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/src/main.rs b/src/main.rs >>>> index 582214c..49d6ffe 100644 >>>> --- a/src/main.rs >>>> +++ b/src/main.rs >>>> @@ -134,9 +134,50 @@ impl CtrlTunnel { >>>>            } >>>>               let mut ssl_connector_builder = >>>> SslConnector::builder(SslMethod::tls())?; >>>> -        if fingerprint.is_some() { >>>> -            // FIXME actually verify fingerprint via callback! >>>> -            >>>> ssl_connector_builder.set_verify(openssl::ssl::SslVerifyMode::NONE); >>>> +        if let Some(expected) = fingerprint { >>>> +            ssl_connector_builder.set_verify_callback( >>>> +                openssl::ssl::SslVerifyMode::NONE, >>>> +                move |_valid, ctx| { >>>> +                    let cert = match ctx.current_cert() { >>>> +                        Some(cert) => cert, >>>> +                        None => { >>>> +                            eprintln!("SSL context lacks current >>>> certificate."); >>>> +                            return false; >>>> +                        } >>>> +                    }; >>>> + >>>> +                    let depth = ctx.error_depth(); >>>> +                    if depth != 0 { >>>> +                        return true; >>>> +                    } >>> >>> Sorry about my ignorance. Does using SslVerifyMode::NONE imply that >>> there is an error? At depth 0? Why is it fine to return true if not? >> >> this is a bit.. tricky (did I mention I really really dislike openssl's >> API? ;)) >> >> basically what we do in this branch (if we have a pinned fingerprint to >> check - the regular 'connect iff trusted by system' is the else branch >> below) we set our own callback that gets called for each cert along the >> chain (starting at the top, ending with the leaf/end certificate, but >> the order is not relevant since a single failed callback fails the whole >> verification). >> >> for each cert (== element of the chain == depth value) we get the result >> of openssl's check (`_valid`) and the X509 store context (ctx). >> >> the context (among other things ;)) contains information about where >> (depth) in the chain we currently are: >> - depth 0 == peer certificate (the one we are interested in) >> - depth 1 == CA certificate (signer of peer cert, not interesting) >> - depth 2 == higher CA certificate (signer of CA at 1, not interesting) >> - depth X == higher CA certificate (signer of CA at X-1, not >>    interesting) >> >> all but the peer certificate are optional (peer could give us just a >> self-signed certificate, or an incomplete chain). >> >> that the methods here are all referring to 'error' is an OpenSSL >> peculiarity - it basically gives us a cert store with the current cert >> and error depth set to values that are valid if we fail (error) the >> verification. >> >> for each cert/call we do the following: >> >> - ensure there is a current cert in the context or fail verification >> - continue verification with next element of the chain if we are not >>    (yet) at the peer certificate (depth != 0) >> - calculate fingerprint for current (== peer) cert, or fail >> - compare fingerprint with pinned/expected one, fail if not expected >> >> since the verification fails as soon as single callback fails, we need >> to >> - return false if we fail some assumption (like ctx having a current >>    cert, or being able to calculate a cert's FP) >> - return true if the current call is at a depth we are not interested in >>    verifying >> - return true/false depending on result of FP check if current call >> is at >> a depth we are interested in >> >> I'll add a comment to the depth part that it is for skipping the CA >> certs! also verify mode should technically be PEER, so I'll fix that up >> as well. >> > > Thanks for the thorough explanation! I think I get it now (especially > knowing that the callback can be called multiple times helps a lot). > >>> >>>> + >>>> +                    let fp = match >>>> cert.digest(openssl::hash::MessageDigest::sha256()) { >>>> +                        Ok(fp) => fp, >>>> +                        Err(err) => { >>>> +                            // should not happen >>>> +                            eprintln!("failed to calculate >>>> certificate FP - {}", err); >>>> +                            return false; >>>> +                        } >>>> +                    }; >>>> +                    let fp_string = hex::encode(&fp); >>>> +                    let fp_string = fp_string >>>> +                        .as_bytes() >>>> +                        .chunks(2) >>>> +                        .map(|v| std::str::from_utf8(v).unwrap()) >>>> +                        .collect::>() >>>> +                        .join(":"); >>>> + >>>> +                    let expected = expected.to_lowercase(); >>>> +                    if expected == fp_string { >>>> +                        true >>>> +                    } else { >>>> +                        eprintln!("certificate fingerprint does >>>> not match expected fingerprint!"); >>>> +                        eprintln!("expected:    {}", expected); >>>> +                        eprintln!("encountered: {}", fp_string); >>>> +                        false >>>> +                    } >>>> +                }, >>>> +            ); >>>>            } else { >>>>                >>>> ssl_connector_builder.set_verify(openssl::ssl::SslVerifyMode::PEER); >>>>            } >>> > > > _______________________________________________ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel Hi, Just as a datapoint, I tested the step client (https://github.com/smallstep/cli), and saw that it had a pretty sweet system for verifying remote certificates. { "version": 3, "serial_number": "347801840814762006190637676907327486230196", "signature_algorithm": { "name": "SHA256-RSA", "oid": "1.2.840.113549.1.1.11" }, "issuer": { "common_name": [ "R3" ], "country": [ "US" ], "organization": [ "Let's Encrypt" ] }, "issuer_dn": "C=US, O=Let's Encrypt, CN=R3", "validity": { "start": "2022-01-15T22:05:15Z", "end": "2022-04-15T22:05:14Z", "length": 7775999 }, "subject": { "common_name": [ "www.proxmox.com" ] }, "subject_dn": "CN=www.proxmox.com", "subject_key_info": { "key_algorithm": { "name": "RSA" }, "rsa_public_key": { "exponent": 65537, "modulus": "52/d0AcsH5pbdbTETwqKzKnNNaZqGt35Qs3ciLg3h+wkwqEZhxiSp7tmD4zyyywP84dDvUB+WxDngTsoGnkxhBNibFN+htDrhRiDIwXVqW42MRHOxnfGfyILcjmEPRcw0ej2e9LfR45Kd9o7QWL7L5ano8LgkG5rxoBHeEfnjyRyQR8q28z1EyKvR0/jE5fGRtEdtUIJn3Bu7+SsUBqQXcbKVDDcQFyEbICYxBTofXi2fxf1DVGT3ZGy9X4ja0WtPU8o6pjyfzUFW45mgY4W6UwhAWUtbDC0np+K7BiZjZcQTyuszHdcN0BY9skO9kH679DLzqD275+xkU89sb2xMQ==", "length": 2048 }, "fingerprint_sha256": "e832ec8ba68141627a1d3b58fde78150553084b14623689cd0be655aafdc5a5d" }, "extensions": { "key_usage": { "digital_signature": true, "key_encipherment": true, "value": 5 }, "basic_constraints": { "is_ca": false }, "subject_alt_name": { "dns_names": [ "proxmox.com", "www.proxmox.com" ] }, "authority_key_id": "142eb317b75856cbae500940e61faf9d8b14c2c6", "subject_key_id": "078f859fc1821a3f838c5b86ce319b8a70e962fc", "extended_key_usage": { "client_auth": true, "server_auth": true }, "certificate_policies": [ { "id": "2.23.140.1.2.1" }, { "id": "1.3.6.1.4.1.44947.1.1.1", "cps": [ "http://cps.letsencrypt.org" ] } ], "authority_info_access": { "ocsp_urls": [ "http://r3.o.lencr.org" ], "issuer_urls": [ "http://r3.i.lencr.org/" ] }, "signed_certificate_timestamps": [ { "version": 0, "log_id": "QcjKsd8iRkoQxqE6CUKHXk4xixsD6+tLx2jwkGKWBvY=", "timestamp": 1642287915, "signature": "BAMASDBGAiEA6oCYHTwiAUo6xBAknN5K+IedikfwIETRFFjzVaX/d1ICIQC5QGnOlcA9J1kNNDUjwW8MyZp/kj8U3E7Vm1f8Dr6B4A==" }, { "version": 0, "log_id": "KXm+8J45OSHwVnOfY6V35b5XfZxgCvj5TV0mXCVdx4Q=", "timestamp": 1642287915, "signature": "BAMASDBGAiEAtZrDq7wbgxUHzp2VKVDvuIYSJRVP72naI4oS3nS0rMYCIQDHCGH4TPVygVPE73P8jFzX++7ZhamkZbK2IFZgIVBdcg==" } ] }, "signature": { "signature_algorithm": { "name": "SHA256-RSA", "oid": "1.2.840.113549.1.1.11" }, "value": "iTb8dmJ1JvjG200/hmJ6mPmD3w7YYUR0XlmK8hu7YJTbPYxQtgiqew3ulrfx5z98fe7gbPmnR7fCtdSPqwz4HKbiGLS6kmqEiQn6eDkhbi2pIXLZTJ6qPpKkJJAUyT1b3rgIRF49PhVJZ0sbHTHeyBH41Lbkvs+mDWHBubhsCI6coN1f0+ZmZgctaK2cE9jKn8t1vRmMFW8MeM7SSX951EcRxO4LRrx/yNE9v8reMsLAeUbuQ6fboEh60+ZIK/6+PFupGYsINtp/76rn1AOcDCGeP1enzP7/T8qbFChNN5CnewSYEdzJYkn07y8H1nt+ioj60TsIHdW8JBd0tEajgg==", "valid": false, "self_signed": false }, "fingerprint_md5": "9102932ce81f268eedddc6b9ff59e6fe", "fingerprint_sha1": "19e4425b6321236f4dfd5ee26a592438fec4bcbd", "fingerprint_sha256": "3b21f35ed0b6bbb5e7cd1c176038c077c86bd723c10587746ffd10dc9f87ba9b", "tbs_noct_fingerprint": "0024bc21edbd4539da91a0a029fd7f3ed7f327336fd90dc150ff2c1f92e45d4a", "spki_subject_fingerprint": "8b9da0a8a08a165b399932a91add980e63d2a59299fd1517ad8e70cbb360637a", "tbs_fingerprint": "be6bf119f73c655983772411fc88e0142926ab3d5bd7adc14b017fc9c7d2f1a6", "validation_level": "DV", "names": [ "www.proxmox.com", "proxmox.com" ], "redacted": false }