[Cronet-iOS] Public-Key-Pinning Tests

components/cronet/ios/Cronet.h

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import <Foundation/Foundation.h>
 
 #include "bidirectional_stream_c.h"
 
 // TODO(mef): Remove this header after transition to bidirectional_stream_c.h
 // See crbug.com/650462 for details.
@@ skipping 55 matching lines @@
 // If |partial| is set to NO, then |userAgent| value is complete value sent to
 // the remote. For Example: "Foo/3.0.0.0" is sent as "Foo/3.0.0.0".
 //
 // This method only has any effect before |start| is called.
 + (void)setUserAgent:(NSString*)userAgent partial:(BOOL)partial;
 
 // Sets SSLKEYLogFileName to export SSL key for Wireshark decryption of packet
 // captures. This method only has any effect before |start| is called.
 + (void)setSslKeyLogFileName:(NSString*)sslKeyLogFileName;
 
+// <p>

[lilyhoughton, 2017/06/09 16:51:57]

Can the html be replaced with something more like markdown?  Whatever the
effects on doc formatting, it makes the source code itself slightly less
readable, I think.

[kapishnikov, 2017/06/09 21:27:01]

Done.

+// Pins a set of public keys for a given host. This method only has any effect
+// before |start| is called. By pinning a set of public keys,
+// |pinsSha256|, communication with |hostName| is required to
+// authenticate with a certificate with a public key from the set of pinned
+// ones. An app can pin the public key of the root certificate, any of the
+// intermediate certificates or the end-entry certificate. Authentication will
+// fail and secure communication will not be established if none of the public
+// keys is present in the host's certificate chain, even if the host attempts to
+// authenticate with a certificate allowed by the device's trusted store of
+// certificates.
+// </p>
+// <p>
+// Calling this method multiple times with the same host name overrides the
+// previously set pins for the host.
+// </p>
+// <p>
+// More information about the public key pinning can be found in
+// <a href="https://tools.ietf.org/html/rfc7469">RFC 7469</a>.
+// </p>
+//
+// @param hostName name of the host to which the public keys should be pinned. A

[lilyhoughton, 2017/06/09 16:51:57]

We also don't have @params for any of the other functions - should we?

[kapishnikov, 2017/06/09 21:27:01]

I think we should.

The correct way of documenting a method is actually looks like this:
/**
 <#Description#>

 @param host <#host description#>
 @param pinHashes <#pinHashes description#>
 @param includeSubdomains <#includeSubdomains description#>
 @param expirationDate <#expirationDate description#>
 */

It is also an acceptable documentation style according to the google style
guide.

Using three slashes is also possible but it usually used for a single line
documentation.
/// <#Description#>
///
/// @param host <#host description#>
/// @param pinHashes <#pinHashes description#>
/// @param includeSubdomains <#includeSubdomains description#>
/// @param expirationDate <#expirationDate description#>

Double slashes that we currently use are not considered documentation comments.

If a method has a return value, it should be documented like this:
/**
 <#Description#>

 @param fileName <#fileName description#>
 @param logBytes <#logBytes description#>
 @return <#return value description#>
 */
+ (BOOL)startNetLogToFile:(NSString*)fileName logBytes:(BOOL)logBytes;

In order to check if a method is documented correctly, you can alt+click on it.

Maybe we should revisit the documentation of other methods in a separate CL.

+// host that
+//                 consists only of digits and the dot character is treated as
+//                 invalid.
+// @param pinsSha256 a set of pins. Each pin is the SHA-256 cryptographic
+//                   hash of the DER-encoded ASN.1 representation of the Subject
+//                   Public Key Info (SPKI) of the host's X.509 certificate.
+//                   Although, the method does not mandate the presence of the
+//                   backup pin that can be used if the control of the primary
+//                   private key has been lost, it is highly recommended to
+//                   supply one.
+// @param includeSubdomains indicates whether the pinning policy should be
+// applied to
+//                          subdomains of {@code hostName}.

[lilyhoughton, 2017/06/09 16:51:57]

is {@code hostName} different from |hostName|?

[kapishnikov, 2017/06/09 21:27:01]

This is the result of copy-paste. Fixed.

+// @param expirationDate specifies the expiration date for the pins.
++ (void)addPublicKeyPinsForHost:(NSString*)host
+                      pinHashes:(NSSet<NSData*>*)pinHashes
+              includeSubdomains:(BOOL)includeSubdomains
+                 expirationDate:(NSDate*)expirationDate;
+
 // Sets the block used to determine whether or not Cronet should handle the
 // request. If the block is not set, Cronet will handle all requests. Cronet
 // retains strong reference to the block, which can be released by calling this
 // method with nil block.
 + (void)setRequestFilterBlock:(RequestFilterBlock)block;
 
 // Starts CronetEngine. It is recommended to call this method on the application
 // main thread. If the method is called on any thread other than the main one,
 // the method will internally try to execute synchronously using the main GCD
 // queue. Please make sure that the main thread is not blocked by a job
@@ skipping 51 matching lines @@
 
 // Sets Host Resolver Rules for testing.
 // This method must be called after |start| has been called.
 + (void)setHostResolverRulesForTesting:(NSString*)hostResolverRulesForTesting;
 
 // Enables TestCertVerifier which accepts all certificates for testing.
 // This method only has any effect before |start| is called.
 + (void)enableTestCertVerifierForTesting;
 
 @end