Repository: pion/webrtc
Pull request #1439 (2020-09-21 20:48:42)

Add GetCertificateStats to StatsReport
Add an API to get basic stats around certificate

Relates to #610

#### Description

#### Reference issue
Fixes #...


codecov[bot]: # [Codecov](https://codecov.io/gh/pion/webrtc/pull/1439?src=pr&el=h1) Report
> Merging [#1439](https://codecov.io/gh/pion/webrtc/pull/1439?src=pr&el=desc) into [master](https://codecov.io/gh/pion/webrtc/commit/062e55d7693a8b77c9c7fe47c695c505d7e294ce?el=desc) will **decrease** coverage by `2.24%`.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/pion/webrtc/pull/1439/graphs/tree.svg?width=650&height=150&src=pr&token=XOOuQo07I2)](https://codecov.io/gh/pion/webrtc/pull/1439?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1439      +/-   ##
==========================================
- Coverage   73.94%   71.70%   -2.25%     
==========================================
  Files          73       53      -20     
  Lines        5197     1947    -3250     
==========================================
- Hits         3843     1396    -2447     
+ Misses        999      453     -546     
+ Partials      355       98     -257     
```

| Flag | Coverage Δ | |
|---|---|---|
| #go | `?` | |
| #wasm | `71.70% <ø> (ø)` | |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/pion/webrtc/pull/1439?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [icetcp.go](https://codecov.io/gh/pion/webrtc/pull/1439/diff?src=pr&el=tree#diff-aWNldGNwLmdv) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [sdpsemantics.go](https://codecov.io/gh/pion/webrtc/pull/1439/diff?src=pr&el=tree#diff-c2Rwc2VtYW50aWNzLmdv) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [icecandidatepair.go](https://codecov.io/gh/pion/webrtc/pull/1439/diff?src=pr&el=tree#diff-aWNlY2FuZGlkYXRlcGFpci5nbw==) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [stats.go](https://codecov.io/gh/pion/webrtc/pull/1439/diff?src=pr&el=tree#diff-c3RhdHMuZ28=) | `0.00% <0.00%> (-52.18%)` | :arrow_down: |
| [gathering\_complete\_promise.go](https://codecov.io/gh/pion/webrtc/pull/1439/diff?src=pr&el=tree#diff-Z2F0aGVyaW5nX2NvbXBsZXRlX3Byb21pc2UuZ28=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [dtlsrole.go](https://codecov.io/gh/pion/webrtc/pull/1439/diff?src=pr&el=tree#diff-ZHRsc3JvbGUuZ28=) | `68.96% <0.00%> (-31.04%)` | :arrow_down: |
| [networktype.go](https://codecov.io/gh/pion/webrtc/pull/1439/diff?src=pr&el=tree#diff-bmV0d29ya3R5cGUuZ28=) | `46.80% <0.00%> (-19.15%)` | :arrow_down: |
| [icegathererstate.go](https://codecov.io/gh/pion/webrtc/pull/1439/diff?src=pr&el=tree#diff-aWNlZ2F0aGVyZXJzdGF0ZS5nbw==) | `84.61% <0.00%> (-15.39%)` | :arrow_down: |
| [icecandidatetype.go](https://codecov.io/gh/pion/webrtc/pull/1439/diff?src=pr&el=tree#diff-aWNlY2FuZGlkYXRldHlwZS5nbw==) | `61.11% <0.00%> (-13.89%)` | :arrow_down: |
| [operations.go](https://codecov.io/gh/pion/webrtc/pull/1439/diff?src=pr&el=tree#diff-b3BlcmF0aW9ucy5nbw==) | `83.33% <0.00%> (-11.12%)` | :arrow_down: |
| ... and [20 more](https://codecov.io/gh/pion/webrtc/pull/1439/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pion/webrtc/pull/1439?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pion/webrtc/pull/1439?src=pr&el=footer). Last update [062e55d...6c2d7b7](https://codecov.io/gh/pion/webrtc/pull/1439?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

tarrencev: @obasajujoshua31 if you can, add a simple test to get codecov/patch to pass. otherwise this looks good!

Patch:
diff --git a/certificate.go b/certificate.go
index 88610d33..d5441758 100644
--- a/certificate.go
+++ b/certificate.go
@@ -9,6 +9,7 @@ import (
 	"crypto/rsa"
 	"crypto/x509"
 	"crypto/x509/pkix"
+	"encoding/base64"
 	"encoding/hex"
 	"fmt"
 	"math/big"
@@ -22,6 +23,7 @@ import (
 type Certificate struct {
 	privateKey crypto.PrivateKey
 	x509Cert   *x509.Certificate
+	statsID    string
 }
 
 // NewCertificate generates a new x509 compliant Certificate to be used
@@ -55,7 +57,7 @@ func NewCertificate(key crypto.PrivateKey, tpl x509.Certificate) (*Certificate,
 		return nil, &rtcerr.UnknownError{Err: err}
 	}
 
-	return &Certificate{privateKey: key, x509Cert: cert}, nil
+	return &Certificate{privateKey: key, x509Cert: cert, statsID: fmt.Sprintf("certificate-%d", time.Now().UnixNano())}, nil
 }
 
 // Equals determines if two certificates are identical by comparing both the
@@ -155,5 +157,29 @@ func GenerateCertificate(secretKey crypto.PrivateKey) (*Certificate, error) {
 //
 // This can be used if you want to share a certificate across multiple PeerConnections
 func CertificateFromX509(privateKey crypto.PrivateKey, certificate *x509.Certificate) Certificate {
-	return Certificate{privateKey, certificate}
+	return Certificate{privateKey, certificate, fmt.Sprintf("certificate-%d", time.Now().UnixNano())}
+}
+
+func (c Certificate) collectStats(report *statsReportCollector) error {
+	report.Collecting()
+
+	fingerPrintAlgo, err := c.GetFingerprints()
+	if err  != nil {
+		return err
+	}
+
+	base64Certificate := base64.RawURLEncoding.EncodeToString(c.x509Cert.Raw)
+
+	stats := CertificateStats{
+		Timestamp:            statsTimestampFrom(time.Now()),
+		Type:                 StatsTypeCertificate,
+		ID:                   c.statsID,
+		Fingerprint:          fingerPrintAlgo[0].Value,
+		FingerprintAlgorithm: fingerPrintAlgo[0].Algorithm,
+		Base64Certificate:    base64Certificate,
+		IssuerCertificateID:  c.x509Cert.Issuer.String(),
+	}
+
+	report.Collect(stats.ID, stats)
+	return nil
 }
diff --git a/peerconnection.go b/peerconnection.go
index 09493d33..a4041fe6 100644
--- a/peerconnection.go
+++ b/peerconnection.go
@@ -1947,9 +1947,17 @@ func (pc *PeerConnection) GetStats() StatsReport {
 		DataChannelsOpened:    dataChannelsOpened,
 		DataChannelsRequested: dataChannelsRequested,
 	}
-	pc.mu.Unlock()
 
 	statsCollector.Collect(stats.ID, stats)
+
+	certificates := pc.configuration.Certificates
+	for _, certificate := range certificates {
+		if err := certificate.collectStats(statsCollector); err != nil {
+			continue
+		}
+	}
+	pc.mu.Unlock()
+
 	return statsCollector.Ready()
 }
 
diff --git a/stats_go.go b/stats_go.go
index 9d92ddfe..f1b783e7 100644
--- a/stats_go.go
+++ b/stats_go.go
@@ -61,3 +61,18 @@ func (r StatsReport) GetICECandidatePairStats(c *ICECandidatePair) (ICECandidate
 	}
 	return candidateStats, true
 }
+
+// GetCertificateStats is a helper method to return the associated stats for a given Certificate
+func (r StatsReport) GetCertificateStats(c *Certificate) (CertificateStats, bool) {
+	statsID := c.statsID
+	stats, ok := r[statsID]
+	if !ok {
+		return CertificateStats{}, false
+	}
+
+	certificateStats, ok := stats.(CertificateStats)
+	if !ok {
+		return CertificateStats{}, false
+	}
+	return certificateStats, true
+}


Test patch:
diff --git a/certificate_test.go b/certificate_test.go
index 5e7b431d..4168d08c 100644
--- a/certificate_test.go
+++ b/certificate_test.go
@@ -65,6 +65,9 @@ func TestGenerateCertificateEqual(t *testing.T) {
 	sk1, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
 	assert.Nil(t, err)
 
+	sk3, err := rsa.GenerateKey(rand.Reader, 2048)
+	assert.NoError(t, err)
+
 	cert1, err := GenerateCertificate(sk1)
 	assert.Nil(t, err)
 
@@ -74,8 +77,12 @@ func TestGenerateCertificateEqual(t *testing.T) {
 	cert2, err := GenerateCertificate(sk2)
 	assert.Nil(t, err)
 
+	cert3, err := GenerateCertificate(sk3)
+	assert.NoError(t, err)
+
 	assert.True(t, cert1.Equals(*cert1))
 	assert.False(t, cert1.Equals(*cert2))
+	assert.True(t, cert3.Equals(*cert3))
 }
 
 func TestGenerateCertificateExpires(t *testing.T) {
@@ -87,4 +94,8 @@ func TestGenerateCertificateExpires(t *testing.T) {
 
 	now := time.Now()
 	assert.False(t, cert.Expires().IsZero() || now.After(cert.Expires()))
+
+	x509Cert := CertificateFromX509(sk, &x509.Certificate{})
+	assert.NotNil(t, x509Cert)
+	assert.Contains(t, x509Cert.statsID, "certificate")
 }
diff --git a/stats_go_test.go b/stats_go_test.go
index 8d421cbf..9f36298d 100644
--- a/stats_go_test.go
+++ b/stats_go_test.go
@@ -107,6 +107,13 @@ func getTransportStats(t *testing.T, report StatsReport, statsID string) Transpo
 	return transportStats
 }
 
+func getCertificateStats(t *testing.T, report StatsReport, certificate *Certificate) CertificateStats {
+	certificateStats, ok := report.GetCertificateStats(certificate)
+	assert.True(t, ok)
+	assert.Equal(t, certificateStats.Type, StatsTypeCertificate)
+	return certificateStats
+}
+
 func findLocalCandidateStats(report StatsReport) []ICECandidateStats {
 	result := []ICECandidateStats{}
 	for _, s := range report {
@@ -295,6 +302,13 @@ func TestPeerConnection_GetStats(t *testing.T) {
 	assert.GreaterOrEqual(t, offerSCTPTransportStats.BytesSent, answerSCTPTransportStats.BytesReceived)
 	assert.GreaterOrEqual(t, answerSCTPTransportStats.BytesSent, offerSCTPTransportStats.BytesReceived)
 
+	certificates := offerPC.configuration.Certificates
+
+	for _, certificate := range certificates {
+		certificateStats := getCertificateStats(t, reportPCOffer, &certificate)
+		assert.NotEmpty(t, certificateStats)
+	}
+
 	assert.NoError(t, offerPC.Close())
 	assert.NoError(t, answerPC.Close())
 }