add HTTPS support to owner onboarding server #578
Conversation
This commit adds TLS support to owner-onboarding-server. It also adds a new module ov_management that handles enpoints for ov management api. Signed-off-by: Rupanshi Jain <rupanshijain45678@gmail.com>
Signed-off-by: Rupanshi Jain <rupanshijain45678@gmail.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| let ov_list: [&str; 1] = ["89cb17fd-95e7-4de8-a36a-686926a7f88f"]; | ||
| let delete_ov = client | ||
| .post(format!( | ||
| "http://localhost:{}/management/v1/ownership_voucher/delete", |
Check notice
Code scanning / devskim
Accessing localhost could indicate debug code, or could hinder scaling. Note test
|
|
||
| let add_ov = client | ||
| .post(format!( | ||
| "https://localhost:{}/management/v1/ownership_voucher", //DevSkim: ignore DS137138 |
Check notice
Code scanning / devskim
Accessing localhost could indicate debug code, or could hinder scaling. Note test
| user_data: Arc<OwnerServiceUD>, | ||
| ) -> core::result::Result<Box<dyn warp::reply::Reply>, warp::Rejection> { | ||
| // check for ov type | ||
| let content_type = content_type.to_str().unwrap(); |
There was a problem hiding this comment.
generally, we cannot bail on a server, we should return an error and continue listening to requests
| let mut reader = body.reader(); | ||
| //max limit of ov size is 1024 * 32 bytes | ||
| let mut dst = [0; 32768]; | ||
| let _ = reader.read(&mut dst).unwrap(); |
There was a problem hiding this comment.
I'm not familiar with how warp handles possible DoS attacks, what happens when they send us 2GB, 20GB, 200GB of data? do we read all of it?
There was a problem hiding this comment.
there's a content length limit filter, so any request exceeding 1024*16 bytes should be should be a no-go and get rejected by the server
|
|
||
| let ovs = match content_type { | ||
| "application/x-pem-file" => OwnershipVoucher::many_from_pem(&dst[..]), | ||
| "application/cbor" => OwnershipVoucher::deserialize_many_from_reader(&dst[..]), |
There was a problem hiding this comment.
this add_ovfunction only adds a single OV, so you should use from_pem and deserialize_from_reader instead
There was a problem hiding this comment.
right, reading things again, we do want to add multiple vouchers at the same time, so the lines that say:
//max limit of ov size is 1024 * 32 bytes
let mut dst = [0; 32768];
are misleading.
Do you mean to say that the max body size is 1024 * 32 bytes instead?
There was a problem hiding this comment.
yes my bad, this is the max body size. Also, it seems like we are capping all requests at 1024*16 bytes for body size, will make this change.
| } | ||
|
|
||
| // check-empty | ||
| let ovs = ovs.unwrap(); |
There was a problem hiding this comment.
you can do a match over here to handle err or the result
| ) -> core::result::Result<Box<dyn warp::reply::Reply>, warp::Rejection> { | ||
| let mut unknown_device = Vec::new(); | ||
| for uuid in &uuids { | ||
| if Guid::from_str(uuid).is_err() { |
There was a problem hiding this comment.
since you'll be needing the unwrapped uuid also below, you can let uuid = match [...] and do the error handling of the uuid first.
| Ok(None) => { | ||
| unknown_device.push(uuid.to_string()); | ||
| } | ||
| Err(_) => {} |
There was a problem hiding this comment.
why don't we do anything when there is an error trying to find an OV?
| NoOwnershipVoucherFound, | ||
| #[error("Unable to parse UUID")] | ||
| UuidParseError, | ||
| #[error("Pem format error")] |
There was a problem hiding this comment.
do we always know that it is a PEM format error when we are parsing stuff?
| } | ||
| bail!("Some tests failed"); | ||
| } | ||
| } |
There was a problem hiding this comment.
you also need to test when a proper OV is sent in PEM format, when a proper OV is sent in cbor format, when multiple OVs are sent in both formats, when the formats are mixed up and when the actual OV count does not match the sent OV count.
other changes include: - read cert and key from config - grammar fixes Signed-off-by: Rupanshi Jain <rupanshijain45678@gmail.com>
To-Do: