crypto/x509: add certificate support.

crypto/x509: add certificate support.

[rsc, 2009-11-18 20:46:57]

http://codereview.appspot.com/156054/diff/5/1007
File src/pkg/crypto/x509/x509.go (right):

http://codereview.appspot.com/156054/diff/5/1007#newcode77
src/pkg/crypto/x509/x509.go:77: TBSCertificate		tbsCertificate;
why is this upper case?
if you make it just

tbsCertificate;

you should be able to drop a lot of .TBSCertificate. below

http://codereview.appspot.com/156054/diff/5/1007#newcode130
src/pkg/crypto/x509/x509.go:130: type SignatureAlgorithm int
doc comment

http://codereview.appspot.com/156054/diff/5/1007#newcode132
src/pkg/crypto/x509/x509.go:132: const (
doc comment

http://codereview.appspot.com/156054/diff/5/1007#newcode142
src/pkg/crypto/x509/x509.go:142: type PublicKeyAlgorithm int
doc comment

http://codereview.appspot.com/156054/diff/5/1007#newcode144
src/pkg/crypto/x509/x509.go:144: const (
doc comment

http://codereview.appspot.com/156054/diff/5/1007#newcode149
src/pkg/crypto/x509/x509.go:149: type Name struct {
doc comment

http://codereview.appspot.com/156054/diff/5/1007#newcode226
src/pkg/crypto/x509/x509.go:226: type KeyUsage int
doc comment

http://codereview.appspot.com/156054/diff/5/1007#newcode228
src/pkg/crypto/x509/x509.go:228: const (
doc comment

http://codereview.appspot.com/156054/diff/5/1007#newcode268
src/pkg/crypto/x509/x509.go:268: type UnsupportedAlgorithmError struct{}
doc comment

http://codereview.appspot.com/156054/diff/5/1007#newcode271
src/pkg/crypto/x509/x509.go:271: return "signature could not be verified because
of unimplemented algorithms"
It would be nice if these were shorter. 

"cannot verify signature: algorithm unimplemented"

http://codereview.appspot.com/156054/diff/5/1007#newcode274
src/pkg/crypto/x509/x509.go:274: type ConstraintViolationError struct{}
doc comment

http://codereview.appspot.com/156054/diff/5/1007#newcode277
src/pkg/crypto/x509/x509.go:277: return "signature is invalid because the parent
certificate is not authorized to sign such certificates."
"invalid signature: parent certificate cannot sign this kind of certificate"

http://codereview.appspot.com/156054/diff/5/1007#newcode281
src/pkg/crypto/x509/x509.go:281: // from parent. It indicates success by
returning nil.
you can drop the sentence about nil.
it's implied.

http://codereview.appspot.com/156054/diff/5/1007#newcode349
src/pkg/crypto/x509/x509.go:349: // IsValidForHostname returns true iff c is a
valid certificate for the given
s/name//

http://codereview.appspot.com/156054/diff/5/1007#newcode403
src/pkg/crypto/x509/x509.go:403: return nil, nil;	// 6g bug
It's not a 6g bug, it's in the language spec.
If you use
panic("unreachable")
then I'll find it when we fix the spec.

http://codereview.appspot.com/156054/diff/5/1007#newcode547
src/pkg/crypto/x509/x509.go:547: func ParseCertificate(asn1Data []byte)
(*Certificate, os.Error) {
doc comment

http://codereview.appspot.com/156054/diff/5/1007#newcode560
src/pkg/crypto/x509/x509.go:560: func ParseCertificates(asn1Data []byte)
([]*Certificate, os.Error) {
doc comment

http://codereview.appspot.com/156054/diff/5/1008
File src/pkg/crypto/x509/x509_test.go (right):

http://codereview.appspot.com/156054/diff/5/1008#newcode103
src/pkg/crypto/x509/x509_test.go:103: var certBytes =
"308203223082028ba00302010202106edf0d9499fd4533dd1297fc42a93be1300d06092a864886f70d0101050500304c310b3009060355040613025a4131253023060355040a131c54686177746520436f6e73756c74696e67202850747929204c74642e311630140603550403130d54686177746520534743204341301e170d3039303332353136343932395a170d3130303332353136343932395a3069310b3009060355040613025553311330110603550408130a43616c69666f726e6961311630140603550407130d4d6f756e7461696e205669657731133011060355040a130a476f6f676c6520496e63311830160603550403130f6d61696c2e676f6f676c652e636f6d30819f300d06092a864886f70d010101050003818d0030818902818100c5d6f892fccaf5614b064149e80a2c9581a218ef41ec35bd7a58125ae76f9ea54ddc893abbeb029f6b73616bf0ffd868791fba7af9c4aebf3706ba3eeaeed27435b4ddcfb157c05f351d66aa87fee0de072d66d773affbd36ab78bef090e0cc861a903ac90dd98b51c9c41566c017f0beec3bff391051ffba0f5cc6850ad2a590203010001a381e73081e430280603551d250421301f06082b0601050507030106082b0601050507030206096086480186f842040130360603551d1f042f302d302ba029a0278625687474703a2f2f63726c2e7468617774652e636f6d2f54686177746553474343412e63726c307206082b0601050507010104663064302206082b060105050730018616687474703a2f2f6f6373702e7468617774652e636f6d303e06082b060105050730028632687474703a2f2f7777772e7468617774652e636f6d2f7265706f7369746f72792f5468617774655f5347435f43412e637274300c0603551d130101ff04023000300d06092a864886f70d01010505000381810062f1f3050ebc105e497c7aedf87e24d2f4a986bb3b837bd19b91ebcad98b065992f6bd2b49b7d6d3cb2e427a99d606c7b1d46352527fac39e6a8b6726de5bf70212a52cba07634a5e332011bd1868e78eb5e3c93cf03072276786f207494feaa0ed9d53b2110a76571f90209cdae884385c882587030ee15f33d761e2e45a6bc308203233082028ca003020102020430000002300d06092a864886f70d0101050500305f310b300906035504061302555331173015060355040a130e566572695369676e2c20496e632e31373035060355040b132e436c6173732033205075626c6963205072696d6172792043657274696669636174696f6e20417574686f72697479301e170d3034303531333030303030305a170d3134303531323233353935395a304c310b3009060355040613025a4131253023060355040a131c54686177746520436f6e73756c74696e67202850747929204c74642e311630140603550403130d5468617774652053474320434130819f300d06092a864886f70d010101050003818d0030818902818100d4d367d08d157faecd31fe7d1d91a13f0b713cacccc864fb63fc324b0794bd6f80ba2fe10493c033fc093323e90b742b71c403c6d2cde22ff50963cdff48a500bfe0e7f388b72d32de9836e60aad007bc4644a3b847503f270927d0e62f521ab693684317590f8bfc76c881b06957cc9e5a8de75a12c7a68dfd5ca1c875860190203010001a381fe3081fb30120603551d130101ff040830060101ff020100300b0603551d0f040403020106301106096086480186f842010104040302010630280603551d110421301fa41d301b3119301706035504031310507269766174654c6162656c332d313530310603551d1f042a30283026a024a0228620687474703a2f2f63726c2e766572697369676e2e636f6d2f706361332e63726c303206082b0601050507010104263024302206082b060105050730018616687474703a2f2f6f6373702e7468617774652e636f6d30340603551d25042d302b06082b0601050507030106082b0601050507030206096086480186f8420401060a6086480186f845010801300d06092a864886f70d01010505000381810055ac63eadea1ddd2905f9f0bce76be13518f93d9052bc81b774bad6950a1eededcfddb07e9e83994dcab72792f06bfab8170c4a8edea5334edef1e53d906c7562bd15cf4d18a8eb42bb1379048084225c53e8acb7feb6f04d16dc574a2f7a27c7b603c77cd0ece48027f012fb69b37e02a2a36dcd585d6ace53f546f961e05af"
please split this line

[agl1, 2009-11-18 21:09:38]

http://codereview.appspot.com/156054/diff/5/1007
File src/pkg/crypto/x509/x509.go (right):

http://codereview.appspot.com/156054/diff/5/1007#newcode130
src/pkg/crypto/x509/x509.go:130: type SignatureAlgorithm int
On 2009/11/18 20:46:57, rsc wrote:
> doc comment

The comment would be something like:
"A signature algorithm represents an algorithm used for signing."

That just feels like I'm writing Java.

http://codereview.appspot.com/156054/diff/5/1007#newcode132
src/pkg/crypto/x509/x509.go:132: const (
On 2009/11/18 20:46:57, rsc wrote:
> doc comment

ditto.

http://codereview.appspot.com/156054/diff/5/1007#newcode142
src/pkg/crypto/x509/x509.go:142: type PublicKeyAlgorithm int
On 2009/11/18 20:46:57, rsc wrote:
> doc comment

ditto.

http://codereview.appspot.com/156054/diff/5/1007#newcode144
src/pkg/crypto/x509/x509.go:144: const (
On 2009/11/18 20:46:57, rsc wrote:
> doc comment

Ditto.

http://codereview.appspot.com/156054/diff/5/1007#newcode149
src/pkg/crypto/x509/x509.go:149: type Name struct {
On 2009/11/18 20:46:57, rsc wrote:
> doc comment
> 

Done.

http://codereview.appspot.com/156054/diff/5/1007#newcode226
src/pkg/crypto/x509/x509.go:226: type KeyUsage int
On 2009/11/18 20:46:57, rsc wrote:
> doc comment
> 

Done.

http://codereview.appspot.com/156054/diff/5/1007#newcode228
src/pkg/crypto/x509/x509.go:228: const (
On 2009/11/18 20:46:57, rsc wrote:
> doc comment

Seems redundant given a comment on KeyUsage.

http://codereview.appspot.com/156054/diff/5/1007#newcode268
src/pkg/crypto/x509/x509.go:268: type UnsupportedAlgorithmError struct{}
On 2009/11/18 20:46:57, rsc wrote:
> doc comment
> 

Done.

http://codereview.appspot.com/156054/diff/5/1007#newcode271
src/pkg/crypto/x509/x509.go:271: return "signature could not be verified because
of unimplemented algorithms"
On 2009/11/18 20:46:57, rsc wrote:
> It would be nice if these were shorter. 
> 
> "cannot verify signature: algorithm unimplemented"
> 

Done.

http://codereview.appspot.com/156054/diff/5/1007#newcode274
src/pkg/crypto/x509/x509.go:274: type ConstraintViolationError struct{}
On 2009/11/18 20:46:57, rsc wrote:
> doc comment
> 

Done.

http://codereview.appspot.com/156054/diff/5/1007#newcode277
src/pkg/crypto/x509/x509.go:277: return "signature is invalid because the parent
certificate is not authorized to sign such certificates."
On 2009/11/18 20:46:57, rsc wrote:
> "invalid signature: parent certificate cannot sign this kind of certificate"
> 
> 
> 

Done.

http://codereview.appspot.com/156054/diff/5/1007#newcode281
src/pkg/crypto/x509/x509.go:281: // from parent. It indicates success by
returning nil.
On 2009/11/18 20:46:57, rsc wrote:
> you can drop the sentence about nil.
> it's implied.
> 

Done.

http://codereview.appspot.com/156054/diff/5/1007#newcode349
src/pkg/crypto/x509/x509.go:349: // IsValidForHostname returns true iff c is a
valid certificate for the given
On 2009/11/18 20:46:57, rsc wrote:
> s/name//
> 

Done.

http://codereview.appspot.com/156054/diff/5/1007#newcode403
src/pkg/crypto/x509/x509.go:403: return nil, nil;	// 6g bug
On 2009/11/18 20:46:57, rsc wrote:
> It's not a 6g bug, it's in the language spec.
> If you use
> panic("unreachable")
> then I'll find it when we fix the spec.
> 

Done.

http://codereview.appspot.com/156054/diff/5/1007#newcode547
src/pkg/crypto/x509/x509.go:547: func ParseCertificate(asn1Data []byte)
(*Certificate, os.Error) {
On 2009/11/18 20:46:57, rsc wrote:
> doc comment
> 

Done.

http://codereview.appspot.com/156054/diff/5/1007#newcode560
src/pkg/crypto/x509/x509.go:560: func ParseCertificates(asn1Data []byte)
([]*Certificate, os.Error) {
On 2009/11/18 20:46:57, rsc wrote:
> doc comment
> 

Done.

[rsc, 2009-11-18 21:41:55]

LGTM

http://codereview.appspot.com/156054/diff/5/1007
File src/pkg/crypto/x509/x509.go (right):

http://codereview.appspot.com/156054/diff/5/1007#newcode130
src/pkg/crypto/x509/x509.go:130: type SignatureAlgorithm int
ok fine.
usually there is more to say like what the SignatureAlgorithm gets used for
and when.

http://codereview.appspot.com/156054/diff/10/1011#newcode567
src/pkg/crypto/x509/x509.go:567: if err != nil {
s/appended/concatenated/

[agl1, 2009-11-19 00:08:26]

*** Submitted as http://code.google.com/p/go/source/detail?r=facde8540428 ***

crypto/x509: add certificate support.

R=rsc
CC=golang-dev
http://codereview.appspot.com/156054