GlobalPlatform / WebApis-for-SE

Open source Apis to access a Secure Element from a Web application
Apache License 2.0
18 stars 5 forks source link

Input validation added to example or addressed explicitly separately #59

Closed hchavers closed 8 years ago

hchavers commented 8 years ago

Comments from Trusted Computing Group - Item # 40

Section: EXAMPLE 1

Comment: Re: “For simplicity and readability reasons, the code above omits the error handling that would have to be implemented in a real application.”

Proposed Resolution: This is understandable, but as things like input validation are so critical to APIs, perhaps they should be called out, or addressed explicitly somewhere?

serianox commented 8 years ago

Updated (untested) with some error management, and more ES6.

// my application identifier
var myAppId = new Uint8Array(…);
// my application command
var myAppCmd = new SECommand(…);

// register sepresent event handler
navigator.secureElementManager.onsepresent = (event) => {

  // open session
  event.reader.openSession()

  .then( (session) => session.openBasicChannel(myAppID)

    // send APDU to my application
    .then( (channel) => channel.transmit(myAppCmd))

    .then( (response) => {
      if (!response.isStatus(0x90, 0x00) {
        // this might be an error
        // ...
      }
    })

    .catch(SESecurityException, e =>
      // exception handler
    )

    .catch(SEIoException, e =>
      // exception handler
    )

    .finally( _ => session.close() );
  );
});
opoto commented 8 years ago

Above proposal looks good, it provides minimum error handling

opoto commented 8 years ago

After a few testings and reworks, here is the new proposed sample:

// my application identifier
var myAppId = new Uint8Array([0xA0, 0x00, 0x00, 0x00, 0x18, 0x0C, 0x00, 0x00, 0x01, 0x63, 0x42, 0x00]);
// my application command
var myAppCmd = new SECommand(0x00, 0xCA, 0x9F, 0x7F, undefined, 0x2A);

// register sepresent event handler
navigator.secureElementManager.onsepresent = (event) => {

  var session;

  // open session
  event.reader.openSession()

  .then( _session => {
    session = _session;
    return session.openBasicChannel(myAppId);
  })

  .then( _channel => {
    return _channel.transmit(myAppCmd);
  })

  .then( _response => {
    if (_response.isStatus(0x90, 0x00)) {
      // process success response
      mySuccessHandler(_response);
    } else {
      // process unexpected response
      myFailureHandler(_response);
    }
  })

  .then( _ => {
    session.close();
  })

  .catch( error => {
    // exception handler
    myErrorHandler(error);
    if (session) {
      session.close();
    }
  })

}
serianox commented 8 years ago

Optimized real-world example reading a NFC tag.

// NDEF Application Identifier
let NDEFAppID = new Uint8Array([0xD2, 0x76, 0x00, 0x00, 0x85, 0x01, 0x01]);

// Select Card Capabilities file
let SelectCC = new SECommand(0x00, 0xA4, 0x00, 0x0C, new Uint8Array([0xE1, 0x03]));

// Read Card Capabilities file
let ReadBinaryCC = new SECommand(0x00, 0xB0, 0x00, 0x00, undefined, 0x0F);

// register sepresent event handler
navigator.secureElementManager.onsepresent = event => {

  // open session
  event.reader.openSession()

  // open channel and select NDEF application
  .then( session => session.openBasicChannel(NDEFAppID)

    // select Card Capabilities file and read its content
    .then( channel => channel.transmit(SelectCC)

      .then( response => response.isStatus(0x90, 0x00)? channel.transmit(ReadBinaryCC): Promise.reject(response.raw) )

      .then( response => response.isStatus(0x90, 0x00)? Promise.resolve(response.data): Promise.reject(response.raw) )

    )

    .then(successHandler)

    .catch(failureHandler)

    .then( _ => session.close() )
  );
};
opoto commented 8 years ago

This example is nice, but possibly too "smart" for the objective here: by using non obvious EcmaScript shortcuts (implicit returns) and multi-level promises it may confuse readers and distract them from the Web API for SE usage. However the final .then to close session is good and simple, I integrated it in commit 7842794.