Fixed all parts according to agl's advise.
https://codereview.appspot.com/9438043/diff/5001/src/pkg/crypto/rsa/pss.goFile src/pkg/crypto/rsa/pss.go (right):
https://codereview.appspot.com/9438043/diff/5001/src/pkg/crypto/rsa/pss.go#newcode2src/pkg/crypto/rsa/pss.go:2:
On 2013/05/17 15:18:55, agl1 wrote:
Should have a file-level comment describing what the functions in this file do
and referencing
http://www.rsa.com/rsalabs/pkcs/files/h11300-wp-pkcs-1v2-2-rsa-cryptography-standard.pdfDone.
https://codereview.appspot.com/9438043/diff/5001/src/pkg/crypto/rsa/pss.go#newcode11src/pkg/crypto/rsa/pss.go:11: func emsaPSSEncode(mHash []byte, emBits
int, salt []byte, hash hash.Hash) ([]byte, error) {
On 2013/05/17 15:18:55, agl1 wrote:
// See [1], section 9.1.1.
Done.
https://codereview.appspot.com/9438043/diff/5001/src/pkg/crypto/rsa/pss.go#newcode31src/pkg/crypto/rsa/pss.go:31: if emLen < hLen+sLen+2 {
On 2013/05/17 15:18:55, agl1 wrote:
since this check is after all the slicing, above, I suspect that the code will
crash first.
Done.
https://codereview.appspot.com/9438043/diff/5001/src/pkg/crypto/rsa/pss.go#newcode46src/pkg/crypto/rsa/pss.go:46: prefix := []byte{0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00}
On 2013/05/17 15:18:55, agl1 wrote:
var prefix [8]byte
Done.
https://codereview.appspot.com/9438043/diff/5001/src/pkg/crypto/rsa/pss.go#newcode48src/pkg/crypto/rsa/pss.go:48: hash.Write(prefix[0:8])
On 2013/05/17 15:18:55, agl1 wrote:
prefix[:]
Done.
https://codereview.appspot.com/9438043/diff/5001/src/pkg/crypto/rsa/pss.go#newcode70src/pkg/crypto/rsa/pss.go:70: // 11. Set the leftmost 8emLen - emBits
bits of the leftmost octet in
On 2013/05/17 15:18:55, agl1 wrote:
s/8emLen/8*emLen/
Done.
https://codereview.appspot.com/9438043/diff/5001/src/pkg/crypto/rsa/pss.go#newcode82src/pkg/crypto/rsa/pss.go:82: func emsaPSSVerify(mHash []byte, em
[]byte, emBits, sLen int, hash hash.Hash) error {
On 2013/05/17 15:18:55, agl1 wrote:
drop the first []byte
Done.
https://codereview.appspot.com/9438043/diff/5001/src/pkg/crypto/rsa/pss.go#newcode83src/pkg/crypto/rsa/pss.go:83: // 1. If the length of M is greater than
the input limitation for the
On 2013/05/17 15:18:55, agl1 wrote:
// See [1], section 9.1.2.
Done.
https://codereview.appspot.com/9438043/diff/5001/src/pkg/crypto/rsa/pss.go#newcode110src/pkg/crypto/rsa/pss.go:110: // 6. If the leftmost 8emLen - emBits
bits of the leftmost octet in
On 2013/05/17 15:18:55, agl1 wrote:
s/8emLen/8*emLen/
Done.
https://codereview.appspot.com/9438043/diff/5001/src/pkg/crypto/rsa/pss.go#newcode113src/pkg/crypto/rsa/pss.go:113: if em[0]&(0xFF<<uint(8-(8*emLen-emBits)))
!= 0 {
On 2013/05/17 15:18:55, agl1 wrote:
s/8emLen/8*emLen/
Done.
https://codereview.appspot.com/9438043/diff/5001/src/pkg/crypto/rsa/pss.go#newcode122src/pkg/crypto/rsa/pss.go:122: // 9. Set the leftmost 8emLen - emBits
bits of the leftmost octet in DB
On 2013/05/17 15:18:55, agl1 wrote:
s/8emLen/8*emLen/
Done.
https://codereview.appspot.com/9438043/diff/5001/src/pkg/crypto/rsa/pss.go#newcode148src/pkg/crypto/rsa/pss.go:148: prefix := []byte{0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00}
On 2013/05/17 15:18:55, agl1 wrote:
var prefix [8]byte
Done.
https://codereview.appspot.com/9438043/diff/5001/src/pkg/crypto/rsa/pss.go#newcode149src/pkg/crypto/rsa/pss.go:149: hash.Write(prefix)
On 2013/05/17 15:18:55, agl1 wrote:
prefix[:]
Done.
https://codereview.appspot.com/9438043/diff/5001/src/pkg/crypto/rsa/pss.go#newcode154src/pkg/crypto/rsa/pss.go:154: h0 = hash.Sum(h0[:0])
On 2013/05/17 15:18:55, agl1 wrote:
delete the previous line and do
h0 := hash.Sum(nil)
Done.
https://codereview.appspot.com/9438043/diff/5001/src/pkg/crypto/rsa/pss.go#newcode157src/pkg/crypto/rsa/pss.go:157: for i, e := range h0 {
On 2013/05/17 15:18:55, agl1 wrote:
use bytes.Equal
Done.
https://codereview.appspot.com/9438043/diff/5001/src/pkg/crypto/rsa/pss.go#newcode168src/pkg/crypto/rsa/pss.go:168: func SignPSS(rand io.Reader, priv
*PrivateKey, hash crypto.Hash, hashed []byte, salt []byte) (s []byte,
err error) {
Since the test vectors provided by RSA contains the salt, I put the salt
here. I think it would be better to it an internal function, say
signPSSWithSalt() and define SignPSS to call signPSSWithSalt(). In test
cases, we test signPSSWithSalt() so that we can keep using test cases
from RSA.
https://codereview.appspot.com/9438043/diff/5001/src/pkg/crypto/rsa/pss.go#newcode168src/pkg/crypto/rsa/pss.go:168: func SignPSS(rand io.Reader, priv
*PrivateKey, hash crypto.Hash, hashed []byte, salt []byte) (s []byte,
err error) {
On 2013/05/17 15:18:55, agl1 wrote:
Why is the salt passed in rather than generated internally? How are users
expected to pick a good value for the salt length? It might be best to fix it in
this package.
Done.
https://codereview.appspot.com/9438043/diff/5001/src/pkg/crypto/rsa/pss.go#newcode168src/pkg/crypto/rsa/pss.go:168: func SignPSS(rand io.Reader, priv
*PrivateKey, hash crypto.Hash, hashed []byte, salt []byte) (s []byte,
err error) {
On 2013/05/17 15:18:55, agl1 wrote:
First line in the function should reference [1], section 8.1.1. (Where [1] will
be defined in the file level comment and point to the PKCS#1 spec
PDF.)
Done.
https://codereview.appspot.com/9438043/diff/5001/src/pkg/crypto/rsa/pss.go#newcode169src/pkg/crypto/rsa/pss.go:169: em, err := emsaPSSEncode(hashed,
priv.N.BitLen()-1, salt, hash.New())
On 2013/05/17 15:18:55, agl1 wrote:
BitLen is a somewhat expensive operation. Worth doing
nBits := priv.N.BitLen()
at the top of this function.
Done.
https://codereview.appspot.com/9438043/diff/5001/src/pkg/crypto/rsa/pss.go#newcode186src/pkg/crypto/rsa/pss.go:186: // sLen is number of bytes of the salt
used to sign the message.
On 2013/05/17 15:18:55, agl1 wrote:
s/sLen/saltLen/
Done.
https://codereview.appspot.com/9438043/diff/5001/src/pkg/crypto/rsa/pss.go#newcode187src/pkg/crypto/rsa/pss.go:187: func VerifyPSS(pub *PublicKey, hash
crypto.Hash, hashed []byte, sig []byte, sLen int) error {
On 2013/05/17 15:18:55, agl1 wrote:
// See [1], section 8.1.2.
nBits := pub.N.BitLen()
if len(sig) != (nBits+7)/8 {
return ErrVerification
}
Done.
https://codereview.appspot.com/9438043/diff/5001/src/pkg/crypto/rsa/pss.go#newcode197src/pkg/crypto/rsa/pss.go:197: err := emsaPSSVerify(hashed, em,
pub.N.BitLen()-1, sLen, hash.New())
On 2013/05/17 15:18:55, agl1 wrote:
merge with next line.
Done.
https://codereview.appspot.com/9438043/--
---
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
For more options, visit
https://groups.google.com/groups/opt_out.
