Tcl Library Source Code

View Ticket
Login
Ticket UUID: bbdff172a399a771485ddbe9606ce9e2738a5d8c
Title: ::pki::verify always returns false when "algo" argument is provided
Type: Bug Version: 0.10
Submitter: RP. Created on: 2020-02-12 11:14:15
Subsystem: pki Assigned To: rkeene
Priority: 5 Medium Severity: Important
Status: Open Last Modified: 2020-02-19 08:11:30
Resolution: None Closed By: nobody
    Closed on:
Description:

As in title, when algo argument is provided ::pki::verify always returns false. As I found out problem is that when default algorithm is used plaintext value is stripped and converted to octetstring, but when explicit algo is provided final comparision is between plain-text and binary.
Fix that I've made is to always convert plaintext to octet-string digest (before if clause):

proc ::pki::verify {signedmessage checkmessage keylist {algo default}} {
	package require asn

	if {[catch {
		set plaintext [::pki::decrypt -binary -unpad -pub -- $signedmessage $keylist]
	}]} {
		return false
	}

	# RP - always convert plain text to extracted octet-string digest (original $plaintext is not valid for final comparison with binary hash)
	set digest ""
	catch {
		::asn::asnGetSequence plaintext message
		::asn::asnGetSequence message digestInfo
		::asn::asnGetOctetString message digest
	}

	if {$algo eq "default"} {
		set algoId "unknown"

		catch {
			::asn::asnGetObjectIdentifier digestInfo algoId
			set algoId [::pki::_oid_number_to_name $algoId]
		}
	} else {
		set algoId $algo
	}

	switch -- $algoId {
		"md5" - "md5WithRSAEncryption" {
			set checkdigest [md5::md5 $checkmessage]
		}
		"sha1" - "sha1WithRSAEncryption" {
			set checkdigest [sha1::sha1 -bin $checkmessage]
		}
		"sha256" - "sha256WithRSAEncryption" {
			set checkdigest [sha2::sha256 -bin $checkmessage]
		}
		default {
			return -code error "Unknown hashing algorithm: $algoId"
		}
	}

	return [expr {$checkdigest eq $digest}]
}

User Comments: RP. added on 2020-02-19 08:11:30:
Thank you for your response. In my case message isn't signed by me, so I wanted additional security to make sure that only specific algorithm was used (force signee to use it). But maybe in fact it was not that necessary.
Anyway I think that "algo" argument is misleading and as it works as you described it should be something like "is_raw" since any other option than "raw" makes no sense.

rkeene added on 2020-02-18 20:33:25:

This looks like a duplicate of [f8e09da9dc22e29ca64e7862c19b573f1d522d4c], which has been resolved by updating the documentation.

Additionally the fix is probably a bad idea, because if the algorithm is provided then there's no reason a PKCS#1v1.5 structured object would be passed in. If a PKCS#1v1.5 structure is passed in, then there's no need to specify the algorithm.

Stripping off the PKCS#1v1.5 structure then validating with a DIFFERENT hashing algorithm is probably a bad idea.

What are you hoping to accomplish with this change ?


aku added on 2020-02-18 19:38:02:
Assigning to Roy (author pki)